diff --git a/backend/common/tests/test_auth.py b/backend/common/tests/test_auth.py index fd508cfc..79749b3a 100644 --- a/backend/common/tests/test_auth.py +++ b/backend/common/tests/test_auth.py @@ -909,3 +909,75 @@ class TestGoogleOrgListExcludesDeactivated: assert response.status_code == status.HTTP_200_OK returned = {o["id"] for o in response.data["organizations"]} assert returned == {str(org_a.id)} + + +@pytest.mark.django_db +class TestOrgPayloadCarriesCurrency: + """Sign-in and org switch must say what currency an org keeps its books in. + + Both clients parse `default_currency` and `currency_symbol` off the org + records these endpoints return, and no endpoint was sending either. The + mobile deal and lead forms therefore defaulted every new record to USD, + and the mobile dashboard printed a dollar sign over an org's totals + whatever currency they were actually in. + """ + + @patch("google.oauth2.id_token.verify_oauth2_token") + @patch("google.auth.transport.requests.Request") + def test_google_sign_in_lists_the_currency( + self, + mock_request_cls, + mock_verify, + unauthenticated_client, + admin_user, + admin_profile, + org_a, + ): + org_a.default_currency = "EUR" + org_a.save(update_fields=["default_currency"]) + mock_verify.return_value = { + "email": admin_user.email, + "email_verified": True, + "picture": "", + } + + response = unauthenticated_client.post( + "/api/auth/google/", {"idToken": "valid-token"}, format="json" + ) + + assert response.status_code == status.HTTP_200_OK + listed = response.data["organizations"][0] + assert listed["default_currency"] == "EUR" + assert listed["currency_symbol"] == "€" + + def test_org_switch_returns_the_target_org_currency( + self, admin_client, admin_user, org_b + ): + org_b.default_currency = "INR" + org_b.save(update_fields=["default_currency"]) + Profile.objects.create(user=admin_user, org=org_b, role="ADMIN") + + response = admin_client.post( + "/api/auth/switch-org/", {"org_id": str(org_b.id)}, format="json" + ) + + assert response.status_code == status.HTTP_200_OK + assert response.data["current_org"]["default_currency"] == "INR" + assert response.data["current_org"]["currency_symbol"] == "₹" + + def test_an_org_with_no_currency_set_falls_back_to_dollars( + self, admin_client, admin_user, org_b + ): + """`default_currency` is nullable in practice, so the symbol lookup + has to answer for an empty value rather than raise or return None.""" + org_b.default_currency = "" + org_b.save(update_fields=["default_currency"]) + Profile.objects.create(user=admin_user, org=org_b, role="ADMIN") + + response = admin_client.post( + "/api/auth/switch-org/", {"org_id": str(org_b.id)}, format="json" + ) + + assert response.status_code == status.HTTP_200_OK + assert response.data["current_org"]["default_currency"] == "USD" + assert response.data["current_org"]["currency_symbol"] == "$" diff --git a/backend/common/tests/test_magic_link.py b/backend/common/tests/test_magic_link.py index 85455e5e..7d4b3fd4 100644 --- a/backend/common/tests/test_magic_link.py +++ b/backend/common/tests/test_magic_link.py @@ -495,6 +495,21 @@ class TestVerifyCodeOrgSelection: assert response.data["current_org"]["id"] == str(org_a.id) assert [o["id"] for o in response.data["organizations"]] == [str(org_a.id)] + def test_the_listed_org_says_which_currency_it_uses( + self, unauthenticated_client, admin_user, admin_profile, org_a + ): + """The client defaults a new deal's currency from this record, so an + org list without a currency made every org look like a dollar org.""" + org_a.default_currency = "GBP" + org_a.save(update_fields=["default_currency"]) + self._code_token(admin_user.email) + + response = self._verify(unauthenticated_client, admin_user.email) + + assert response.status_code == status.HTTP_200_OK + assert response.data["organizations"][0]["default_currency"] == "GBP" + assert response.data["organizations"][0]["currency_symbol"] == "£" + def test_multi_org_user_is_offered_the_choice_instead_of_one_being_made( self, unauthenticated_client, admin_user, admin_profile, org_a, org_b ): diff --git a/backend/common/views/auth_views.py b/backend/common/views/auth_views.py index 79076467..d119411a 100644 --- a/backend/common/views/auth_views.py +++ b/backend/common/views/auth_views.py @@ -18,10 +18,36 @@ from rest_framework_simplejwt.authentication import JWTAuthentication from common import serializer from common.models import Org, Profile, User from common.serializer import OrgAwareRefreshToken +from common.utils import CURRENCY_SYMBOLS logger = logging.getLogger(__name__) +def _org_payload(org, role=None): + """The org record every auth response hands a client. + + Currency belongs here because a client picks it up at sign-in and holds it + for the whole session: the mobile deal and lead forms default a new + record's currency from it, and the dashboard prices its totals with it. + All three read fields that no endpoint was sending, so an org keeping its + books in euros still created dollar deals and drew a dollar dashboard. + + ``default_currency`` is normalised rather than passed through, so a blank + column answers with the same "USD" the rest of the codebase assumes. + """ + currency = org.default_currency or "USD" + payload = { + "id": str(org.id), + "name": org.name, + "default_currency": currency, + "currency_symbol": CURRENCY_SYMBOLS.get(currency, "$"), + "default_country": org.default_country, + } + if role is not None: + payload["role"] = role + return payload + + def _google_email_is_verified(claims): """Whether Google asserts the caller controls the email in ``claims``. @@ -296,14 +322,7 @@ class GoogleIdTokenView(APIView): profiles = Profile.objects.filter(user=user, is_active=True).select_related( "org" ) - organizations = [ - { - "id": str(p.org.id), - "name": p.org.name, - "role": p.role, - } - for p in profiles - ] + organizations = [_org_payload(p.org, role=p.role) for p in profiles] # Generate JWT token token = OrgAwareRefreshToken.for_user_and_org(user, None) @@ -576,7 +595,7 @@ class OrgSwitchView(APIView): { "access_token": str(token.access_token), "refresh_token": str(token), - "current_org": {"id": str(profile.org.id), "name": profile.org.name}, + "current_org": _org_payload(profile.org), "profile": { "id": str(profile.id), "role": profile.role, @@ -764,10 +783,7 @@ class MagicLinkVerifyView(APIView): } if default_org: - response_data["current_org"] = { - "id": str(default_org.id), - "name": default_org.name, - } + response_data["current_org"] = _org_payload(default_org) return Response(response_data, status=status.HTTP_200_OK) @@ -882,9 +898,7 @@ class MagicLinkVerifyCodeView(APIView): ) # Same shape the Google flow returns, so a client can offer the same # picker whichever way the user signed in. - organizations = [ - {"id": str(p.org.id), "name": p.org.name, "role": p.role} for p in profiles - ] + organizations = [_org_payload(p.org, role=p.role) for p in profiles] # Bind the session to an org only when there is no choice to make. # Picking `profiles.first()` out of several was an arbitrary answer to @@ -910,8 +924,5 @@ class MagicLinkVerifyCodeView(APIView): "organizations": organizations, } if default_org: - response_data["current_org"] = { - "id": str(default_org.id), - "name": default_org.name, - } + response_data["current_org"] = _org_payload(default_org) return Response(response_data, status=status.HTTP_200_OK) diff --git a/frontend/src/lib/server/v2/accounts.js b/frontend/src/lib/server/v2/accounts.js index 824bd7b3..06662774 100644 --- a/frontend/src/lib/server/v2/accounts.js +++ b/frontend/src/lib/server/v2/accounts.js @@ -256,6 +256,10 @@ export async function getAccount({ cookies }, id) { name: deal.name ?? '', stage: deal.stage, amount: num(deal.amount) ?? 0, + // Each deal is priced in its own currency; the account's rollups above + // are sums and take the org's. Dropping this printed every deal on the + // page as dollars. + currency: deal.currency || 'USD', closed_on: deal.closed_on ?? null, // Real fields, from the stage-aging chain the deal module already owns. days_in_current_stage: deal.days_in_stage ?? 0, @@ -283,6 +287,7 @@ export async function getAccount({ cookies }, id) { status: invoice.status, total_amount: num(invoice.total_amount) ?? 0, amount_due: num(invoice.amount_due) ?? 0, + currency: invoice.currency || 'USD', due_date: invoice.due_date ?? null, past_due: isPastDue(invoice) })), diff --git a/frontend/src/lib/server/v2/contacts.js b/frontend/src/lib/server/v2/contacts.js index 13317a6e..39161fac 100644 --- a/frontend/src/lib/server/v2/contacts.js +++ b/frontend/src/lib/server/v2/contacts.js @@ -194,6 +194,9 @@ export async function getContact({ cookies }, id) { name: deal.name ?? '', stage: deal.stage, amount: num(deal.amount) ?? 0, + // Priced in the deal's own currency, unlike the open-pipeline sum beside + // it, which is an addition across deals and takes the org's. + currency: deal.currency || 'USD', closed_on: deal.closed_on ?? null })), tickets: (response.cases ?? []).map((/** @type {any} */ ticket) => ({ diff --git a/frontend/src/lib/server/v2/leads.js b/frontend/src/lib/server/v2/leads.js index 63ff2786..e03dfb35 100644 --- a/frontend/src/lib/server/v2/leads.js +++ b/frontend/src/lib/server/v2/leads.js @@ -84,6 +84,11 @@ function toRow(lead) { status: lead.status ?? 'assigned', source: lead.source ?? '', opportunity_amount: lead.opportunity_amount ? Number(lead.opportunity_amount) : null, + // A lead's amount is stored in the lead's own currency, which the + // serializer stamps from the org default at create time and the user can + // change afterwards. Dropping it here left every lead amount printing as + // dollars, the same defect the deal list had. + currency: lead.currency || 'USD', assigned_to: ownerName(lead), // Null means never contacted, and the column says so rather than // substituting `updated_at`. An edit is not a conversation, and a list diff --git a/frontend/src/routes/(app)/+layout.server.js b/frontend/src/routes/(app)/+layout.server.js index 46a988cc..2be5864a 100644 --- a/frontend/src/routes/(app)/+layout.server.js +++ b/frontend/src/routes/(app)/+layout.server.js @@ -121,7 +121,21 @@ export async function load(event) { counts: /** @type {Record} */ ({}), org: { name: event.locals.org?.name || 'BottleCRM', - terminology: /** @type {Record | undefined} */ (undefined) + terminology: /** @type {Record | undefined} */ (undefined), + // The currency for figures that are sums rather than one record: pipeline + // totals, invoice ageing, goal progress. A per-record currency cannot + // label a sum, and `money()` falls back to a hardcoded USD when it is not + // told, which printed a dollar sign over a euro figure on every page that + // forgot. It lives here rather than in each page loader because 17 of + // them needed it and copying the same line 17 times is how the first one + // fell out of step. + // + // This does not make a mixed-currency sum correct: the API adds `amount` + // across rows regardless of currency, so an org running more than one + // gets an addition that should not have happened, and no symbol repairs + // it. Read from the JWT's org settings, which is also where a new deal's + // currency comes from. + currency: /** @type {any} */ (event.locals).org_settings?.default_currency || 'USD' }, // Server-derived from the JWT (never the client). Display-only: it lets the // shell hide destinations a member can only reach to be turned away. The diff --git a/frontend/src/routes/(app)/+page.svelte b/frontend/src/routes/(app)/+page.svelte index bc627b34..ba04b946 100644 --- a/frontend/src/routes/(app)/+page.svelte +++ b/frontend/src/routes/(app)/+page.svelte @@ -28,7 +28,7 @@ : summary.quiet_deals === 0 ? `${plural(summary.count, 'thing wants', 'things want')} you today.` : `${plural(summary.count, 'thing wants', 'things want')} you today. ` + - `${plural(summary.quiet_deals, 'deal', 'deals')} worth ${money(summary.quiet_value)} ` + + `${plural(summary.quiet_deals, 'deal', 'deals')} worth ${money(summary.quiet_value, data.org.currency)} ` + `${summary.quiet_deals === 1 ? 'has' : 'have'} gone quiet. Those are first.` ); diff --git a/frontend/src/routes/(app)/accounts/+page.svelte b/frontend/src/routes/(app)/accounts/+page.svelte index f3e930fc..0150e9c7 100644 --- a/frontend/src/routes/(app)/accounts/+page.svelte +++ b/frontend/src/routes/(app)/accounts/+page.svelte @@ -103,19 +103,19 @@ - {a.won_amount ? money(a.won_amount) : '—'} + {a.won_amount ? money(a.won_amount, data.org.currency) : '—'} - {a.open_pipeline ? money(a.open_pipeline) : '—'} + {a.open_pipeline ? money(a.open_pipeline, data.org.currency) : '—'} - {a.overdue_amount ? money(a.overdue_amount) : '—'} + {a.overdue_amount ? money(a.overdue_amount, data.org.currency) : '—'} {#if a.open_tickets} diff --git a/frontend/src/routes/(app)/accounts/[id]/+page.svelte b/frontend/src/routes/(app)/accounts/[id]/+page.svelte index c9926d0c..ad58d037 100644 --- a/frontend/src/routes/(app)/accounts/[id]/+page.svelte +++ b/frontend/src/routes/(app)/accounts/[id]/+page.svelte @@ -44,7 +44,7 @@ : stalled.length ? `${stalled[0].name} has not moved in ${stalled[0].days_in_current_stage} days.` : pastDue.length - ? `${pastDue[0].invoice_number} is past due, ${money(pastDue[0].amount_due)}.` + ? `${pastDue[0].invoice_number} is past due, ${money(pastDue[0].amount_due, pastDue[0].currency)}.` : null ); @@ -79,7 +79,7 @@
i.invoice_number).join(', ') @@ -166,7 +166,9 @@ {#if d.aging_status === 'red' && !d.stage.startsWith('CLOSED_')} {d.days_in_current_stage}d {/if} - {money(d.amount)} + {money(d.amount, d.currency)} {:else}

@@ -261,7 +263,7 @@ {inv.past_due ? 'Past due' : invoiceStatusLabel(inv.status)} - {money(inv.past_due ? inv.amount_due : inv.total_amount)} + {money(inv.past_due ? inv.amount_due : inv.total_amount, inv.currency)} {:else} diff --git a/frontend/src/routes/(app)/contacts/[id]/+page.svelte b/frontend/src/routes/(app)/contacts/[id]/+page.svelte index c58b0230..b8a33c6a 100644 --- a/frontend/src/routes/(app)/contacts/[id]/+page.svelte +++ b/frontend/src/routes/(app)/contacts/[id]/+page.svelte @@ -157,7 +157,7 @@ : !contact.email && (!contact.phone || contact.do_not_call) ? `There is no way to reach ${contact.first_name} on this record: no email${contact.do_not_call ? ', and they asked not to be called' : ' and no phone'}.` : openDeals.length && !contact.owner - ? `${contact.first_name} is on ${openDeals.length === 1 ? openDeals[0].name : `${openDeals.length} open deals`} worth ${money(openPipeline)}, and nobody owns this record.` + ? `${contact.first_name} is on ${openDeals.length === 1 ? openDeals[0].name : `${openDeals.length} open deals`} worth ${money(openPipeline, data.org.currency)}, and nobody owns this record.` : overdueTasks.length ? `${overdueTasks.length === 1 ? 'A task' : `${overdueTasks.length} tasks`} naming ${contact.first_name} ${overdueTasks.length === 1 ? 'is' : 'are'} past due.` : null @@ -221,7 +221,7 @@

Deals they are named on {#if openDeals.length} - {money(openPipeline)} open + {money(openPipeline, data.org.currency)} open {/if}
@@ -243,7 +243,7 @@ : ''}
- {money(d.amount)} + {money(d.amount, d.currency)} {:else}

diff --git a/frontend/src/routes/(app)/goals/+page.svelte b/frontend/src/routes/(app)/goals/+page.svelte index b6c2edc5..5c95e889 100644 --- a/frontend/src/routes/(app)/goals/+page.svelte +++ b/frontend/src/routes/(app)/goals/+page.svelte @@ -68,13 +68,13 @@ }; /** Revenue goals are money; DEALS_CLOSED goals are a count of deals. */ - const value = (g, n) => (g.goal_type === 'REVENUE' ? money(n) : count(n)); + const value = (g, n) => (g.goal_type === 'REVENUE' ? money(n, data.org.currency) : count(n)); {#snippet sub()} - {money(totals.achieved)} of - {money(totals.target)} across + {money(totals.achieved, data.org.currency)} of + {money(totals.target, data.org.currency)} across {count(totals.active)} active goals {/snippet} {#snippet actions()} @@ -88,13 +88,13 @@

@@ -224,7 +224,7 @@
{row.user}
- {money(row.achieved)} of {money(row.target)} + {money(row.achieved, data.org.currency)} of {money(row.target, data.org.currency)}
{count(totals.count)} invoices · - {money(totals.outstanding)} outstanding + {money(totals.outstanding, data.org.currency)} outstanding {/snippet} {#snippet actions()} New invoice @@ -63,13 +63,26 @@
- - - + + +
@@ -134,7 +147,9 @@ {invoiceStatusLabel(inv.status)} - {money(inv.total_amount)} + {money(inv.total_amount, inv.currency)} {shortDate(inv.due_date)} {li.detail}{/if} {li.quantity} - {money(li.rate)} + {money(li.rate, invoice.currency)} {li.tax_rate}% - {money(li.amount)} + {money(li.amount, invoice.currency)} {:else} @@ -163,19 +163,29 @@
Subtotal
-
{money(invoice.subtotal)}
+
+ {money(invoice.subtotal, invoice.currency)} +
{#if invoice.discount_amount > 0}
Discount
-
−{money(invoice.discount_amount)}
+
+ −{money(invoice.discount_amount, invoice.currency)} +
{/if}
Tax
-
{money(invoice.tax_amount)}
+
+ {money(invoice.tax_amount, invoice.currency)} +
{#if invoice.shipping_amount > 0}
Shipping
-
{money(invoice.shipping_amount)}
+
+ {money(invoice.shipping_amount, invoice.currency)} +
{/if}
Paid
-
{money(invoice.amount_paid)}
+
+ {money(invoice.amount_paid, invoice.currency)} +
Outstanding
- {money(invoice.amount_due)} + {money(invoice.amount_due, invoice.currency)}
@@ -205,7 +215,7 @@ name="amount" class="v2-input" style="width:150px" - placeholder={money(invoice.amount_due)} + placeholder={money(invoice.amount_due, invoice.currency)} inputmode="decimal" /> @@ -213,7 +223,7 @@

- Leave the amount blank to settle the full {money(invoice.amount_due)} balance. + Leave the amount blank to settle the full {money(invoice.amount_due, invoice.currency)} balance.

{/if} diff --git a/frontend/src/routes/(app)/invoices/estimates/+page.svelte b/frontend/src/routes/(app)/invoices/estimates/+page.svelte index 0ad0b745..2e809188 100644 --- a/frontend/src/routes/(app)/invoices/estimates/+page.svelte +++ b/frontend/src/routes/(app)/invoices/estimates/+page.svelte @@ -43,7 +43,7 @@ {#snippet sub()} {count(totals.count)} estimates · - {money(totals.awaiting_reply)} awaiting a reply + {money(totals.awaiting_reply, data.org.currency)} awaiting a reply {/snippet} {#snippet actions()}
Not yet due - {money(aging.not_yet_due.amount)} + {money(aging.not_yet_due.amount, data.org.currency)} across {count(aging.not_yet_due.count)} invoices, not late, not shown above @@ -214,7 +217,9 @@ > {a.oldest_days}d - {money(a.amount)} + {money(a.amount, data.org.currency)} {/each} diff --git a/frontend/src/routes/(app)/leads/+page.svelte b/frontend/src/routes/(app)/leads/+page.svelte index 0e1cc485..3dc15d23 100644 --- a/frontend/src/routes/(app)/leads/+page.svelte +++ b/frontend/src/routes/(app)/leads/+page.svelte @@ -99,7 +99,8 @@ {l.status} {l.source} - {l.opportunity_amount ? money(l.opportunity_amount) : '—'}{l.opportunity_amount ? money(l.opportunity_amount, l.currency) : '—'} {count(totals.count)} deals · - {money(totals.amount_sum, data.orgCurrency)} · - {money(totals.weighted_sum, data.orgCurrency)} weighted · + {money(totals.amount_sum, data.org.currency)} · + {money(totals.weighted_sum, data.org.currency)} weighted · {totals.stalled_count} stalled {/snippet} {#snippet actions()} @@ -119,7 +119,7 @@
{STAGE_LABEL[lane.stage]} - {count(lane.count)} · {money(lane.sum, data.orgCurrency)} + {count(lane.count)} · {money(lane.sum, data.org.currency)}
{#if lane.truncated} diff --git a/frontend/src/routes/(app)/pipeline/new/+page.server.js b/frontend/src/routes/(app)/pipeline/new/+page.server.js index 2d052349..62707203 100644 --- a/frontend/src/routes/(app)/pipeline/new/+page.server.js +++ b/frontend/src/routes/(app)/pipeline/new/+page.server.js @@ -3,14 +3,11 @@ import { EDITABLE_FIELDS, createDeal, getDealFormOptions } from '$lib/server/v2/ /** @type {import('./$types').PageServerLoad} */ export async function load(event) { - return { - ...(await getDealFormOptions(event)), - // The currency this deal will be created in. The form has no currency - // field, so the serializer stamps the org default - // (`OpportunityCreateSerializer.create`), and the hint under the amount - // used to say "USD" whatever that default actually was. - orgCurrency: /** @type {any} */ (event.locals).org_settings?.default_currency || 'USD' - }; + // The currency hint under the amount comes from the shell (`data.org.currency` + // via `(app)/+layout.server.js`). It is the currency this deal will be created + // in: the form has no currency field, so the serializer stamps the org default + // (`OpportunityCreateSerializer.create`). + return await getDealFormOptions(event); } /** @type {import('./$types').Actions} */ diff --git a/frontend/src/routes/(app)/pipeline/new/+page.svelte b/frontend/src/routes/(app)/pipeline/new/+page.svelte index 6f8002cf..577df892 100644 --- a/frontend/src/routes/(app)/pipeline/new/+page.svelte +++ b/frontend/src/routes/(app)/pipeline/new/+page.svelte @@ -211,8 +211,8 @@ {:else}

{Number(form.amount) > 0 - ? money(Number(form.amount), data.orgCurrency) - : data.orgCurrency} + ? money(Number(form.amount), data.org.currency) + : data.org.currency}

{/if}
diff --git a/frontend/src/routes/(app)/timesheet/+page.svelte b/frontend/src/routes/(app)/timesheet/+page.svelte index c3cf1ca1..60f73963 100644 --- a/frontend/src/routes/(app)/timesheet/+page.svelte +++ b/frontend/src/routes/(app)/timesheet/+page.svelte @@ -136,7 +136,7 @@ /> diff --git a/mobile/DESIGN_SYSTEM.md b/mobile/DESIGN_SYSTEM.md index 13a4b41e..f8005d08 100644 --- a/mobile/DESIGN_SYSTEM.md +++ b/mobile/DESIGN_SYSTEM.md @@ -2,6 +2,15 @@ A comprehensive design guide for the BottleCRM mobile app, inspired by modern CRM apps like HubSpot. +> **This is the authoritative design reference for the Flutter app.** Change a +> colour or a spacing value here. There is a second, similarly named file at +> [`docs/DESIGN-SYSTEM.md`](./docs/DESIGN-SYSTEM.md): that one is the original +> pre-build spec, kept for its data models and screen inventory. The two agree +> on the values they share; where they ever diverge, this file wins. +> +> A third file, `Django-CRM/DESIGN_SYSTEM.md` at the repo root, is the **web** +> design system. It describes CSS variables and does not apply here. + --- ## Table of Contents diff --git a/mobile/docs/DESIGN-SYSTEM.md b/mobile/docs/DESIGN-SYSTEM.md index d19b8882..c47dba36 100644 --- a/mobile/docs/DESIGN-SYSTEM.md +++ b/mobile/docs/DESIGN-SYSTEM.md @@ -1,6 +1,13 @@ # CRM Mobile App Design System -A comprehensive design system document for implementing the SalesPro CRM application in Flutter. This document serves as the single source of truth for colors, typography, spacing, components, and interaction patterns. +The design system the app was originally built from, written under the working +name "SalesPro CRM". + +> **For colors, typography and spacing, [`../DESIGN_SYSTEM.md`](../DESIGN_SYSTEM.md) +> is authoritative.** The two agree on the values they share, including the +> `#2563EB` primary, so nothing here contradicts it. This file is kept for what +> the other one does not carry: data models, navigation structure, screen +> inventory, and the interaction and motion patterns below. --- diff --git a/mobile/docs/README.md b/mobile/docs/README.md index 93163f92..34870384 100644 --- a/mobile/docs/README.md +++ b/mobile/docs/README.md @@ -1,12 +1,21 @@ -# SalesPro CRM - Flutter Implementation Documentation +# BottleCRM Mobile - Original Implementation Spec -This documentation provides comprehensive specifications for implementing the SalesPro CRM mobile application in Flutter. The specifications are derived from a fully functional SvelteKit prototype. +> **This tree is the brief the app was built from, not a description of the app +> as it stands.** It was written against a SvelteKit prototype under the working +> name "SalesPro CRM", and the app has moved on: it signs in with Google and +> magic links rather than a password, so `03-forgot-password.md` and +> `04-onboarding.md` describe screens that were never built. Read it for intent +> and history. +> +> **For design tokens, [`../DESIGN_SYSTEM.md`](../DESIGN_SYSTEM.md) is +> authoritative.** It is the file the workspace `CLAUDE.md` points at, and the +> one to change when a colour or spacing value changes. --- ## Quick Start -1. **Read the Design System** - Start with [DESIGN-SYSTEM.md](./DESIGN-SYSTEM.md) for colors, typography, spacing, and overall architecture +1. **Read the Design System** - [`../DESIGN_SYSTEM.md`](../DESIGN_SYSTEM.md) carries the current colors, typography, and spacing. [DESIGN-SYSTEM.md](./DESIGN-SYSTEM.md) in this folder adds the original data models, navigation structure, and screen inventory 2. **Review Components** - Check [components/ui-components.md](./components/ui-components.md) for reusable widget specifications 3. **Implement Screens** - Follow the route documentation in sequential order @@ -18,7 +27,8 @@ This documentation provides comprehensive specifications for implementing the Sa | Document | Description | |----------|-------------| -| [DESIGN-SYSTEM.md](./DESIGN-SYSTEM.md) | **Start here** - Complete design system with colors, typography, spacing, data models, and implementation checklist | +| [../DESIGN_SYSTEM.md](../DESIGN_SYSTEM.md) | **Authoritative** - the colors, typography, spacing and component rules the app is actually built to | +| [DESIGN-SYSTEM.md](./DESIGN-SYSTEM.md) | The original design system, kept for its data models, navigation structure, and screen inventory. Where its tokens disagree with the file above, the file above wins | | [components/ui-components.md](./components/ui-components.md) | Detailed specifications for all reusable UI components | ### Authentication Screens diff --git a/mobile/lib/routes/app_router.dart b/mobile/lib/routes/app_router.dart index d6f9cf1a..c1e74175 100644 --- a/mobile/lib/routes/app_router.dart +++ b/mobile/lib/routes/app_router.dart @@ -67,8 +67,6 @@ class AppRoutes { static const String taskEdit = '/tasks/:id/edit'; static const String more = '/more'; static const String profile = '/more/profile'; - static const String notifications = '/more/notifications'; - static const String team = '/more/team'; // Knowledge base static const String solutions = '/solutions'; diff --git a/mobile/lib/screens/dashboard/dashboard_screen.dart b/mobile/lib/screens/dashboard/dashboard_screen.dart index 6b199ed0..660dcbb7 100644 --- a/mobile/lib/screens/dashboard/dashboard_screen.dart +++ b/mobile/lib/screens/dashboard/dashboard_screen.dart @@ -5,6 +5,7 @@ import 'package:lucide_icons_flutter/lucide_icons.dart'; import 'package:intl/intl.dart'; import '../../core/theme/theme.dart'; import '../../data/models/dashboard_data.dart'; +import '../../data/models/deal.dart'; import '../../providers/auth_provider.dart'; import '../../providers/dashboard_provider.dart'; import '../../routes/app_router.dart'; @@ -25,16 +26,6 @@ class _DashboardScreenState extends ConsumerState { final authState = ref.watch(authProvider); final user = authState.user; final greetingName = _friendlyFirstName(user?.name, user?.email); - final currencySymbol = - authState.selectedOrganization?.currencySymbol ?? '\$'; - final currencyFormat = NumberFormat.currency( - symbol: currencySymbol, - decimalDigits: 0, - ); - final compactCurrencyFormat = NumberFormat.compactCurrency( - symbol: currencySymbol, - decimalDigits: 1, - ); return Scaffold( backgroundColor: AppColors.surfaceDim, @@ -76,13 +67,7 @@ class _DashboardScreenState extends ConsumerState { ), // Content - SliverToBoxAdapter( - child: _buildContent( - dashboardAsync, - currencyFormat, - compactCurrencyFormat, - ), - ), + SliverToBoxAdapter(child: _buildContent(dashboardAsync)), ], ), ), @@ -90,11 +75,7 @@ class _DashboardScreenState extends ConsumerState { ); } - Widget _buildContent( - AsyncValue dashboardAsync, - NumberFormat currencyFormat, - NumberFormat compactCurrencyFormat, - ) { + Widget _buildContent(AsyncValue dashboardAsync) { // Stale-while-loading: keep showing prior data during refresh. final data = dashboardAsync.value; @@ -130,19 +111,24 @@ class _DashboardScreenState extends ConsumerState { } final resolved = data ?? const DashboardData(); - return _buildBody(resolved, currencyFormat, compactCurrencyFormat); + return _buildBody(resolved); } - Widget _buildBody( - DashboardData data, - NumberFormat currencyFormat, - NumberFormat compactCurrencyFormat, - ) { + Widget _buildBody(DashboardData data) { + // The currency belongs to the payload, not to the stored org record: no + // endpoint returns `currency_symbol`, so reading it there gave null and + // printed every amount with a dollar sign. `/api/dashboard/` reports the + // currency it priced these totals in, alongside them. + final currencyFormat = NumberFormat.compactCurrency( + symbol: Currency.fromString(data.revenueMetrics.currency).symbol, + decimalDigits: 1, + ); + return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ // KPI Cards - _buildKpiSection(data, compactCurrencyFormat), + _buildKpiSection(data, currencyFormat), const SizedBox(height: 16), @@ -154,7 +140,11 @@ class _DashboardScreenState extends ConsumerState { // Pipeline Overview if (data.pipelineByStage.isNotEmpty) ...[ - _buildPipelineSection(data.pipelineByStage, compactCurrencyFormat), + _buildPipelineSection( + data.pipelineByStage, + currencyFormat, + data.revenueMetrics.otherCurrencyCount, + ), const SizedBox(height: 16), ], @@ -243,6 +233,21 @@ class _DashboardScreenState extends ConsumerState { icon: LucideIcons.target, color: AppColors.purple500, ), + // Neither of these two summarises a list the app can open: one is a + // month-and-stage slice, the other a forecast. Like Conversion, they + // stay plain rather than link somewhere that shows something else. + _KpiCard( + title: 'Won This Month', + value: currencyFormat.format(data.revenueMetrics.wonThisMonth), + icon: LucideIcons.trophy, + color: AppColors.success500, + ), + _KpiCard( + title: 'Weighted', + value: currencyFormat.format(data.revenueMetrics.weightedPipeline), + icon: LucideIcons.scale, + color: AppColors.primary400, + ), ]; return Padding( @@ -328,10 +333,14 @@ class _DashboardScreenState extends ConsumerState { Widget _buildPipelineSection( List stages, NumberFormat currencyFormat, + int otherCurrencyCount, ) { - // Only show active pipeline stages (not closed) + // Only show active pipeline stages (not closed). Selecting on the deal + // count rather than the value, because the server prices a stage in the + // org's currency alone: a stage whose deals are all in another currency + // arrives worth zero and used to disappear from the chart entirely. final activeStages = stages - .where((s) => !s.code.contains('CLOSED') && s.value > 0) + .where((s) => !s.code.contains('CLOSED') && s.count > 0) .toList(); if (activeStages.isEmpty) { @@ -396,7 +405,13 @@ class _DashboardScreenState extends ConsumerState { SizedBox( width: 70, child: Text( - currencyFormat.format(stage.value), + // An unpriced stage reports what it does hold. The + // alternative was a confident zero next to deals that + // exist. + stage.value > 0 + ? currencyFormat.format(stage.value) + : '${stage.count} ' + '${stage.count == 1 ? 'deal' : 'deals'}', style: AppTypography.bodySmall.copyWith( fontWeight: FontWeight.w600, fontSize: 11, @@ -408,6 +423,20 @@ class _DashboardScreenState extends ConsumerState { ), ); }), + if (otherCurrencyCount > 0) ...[ + const SizedBox(height: 4), + Text( + otherCurrencyCount == 1 + ? '1 deal in another currency is not counted in these ' + 'totals' + : '$otherCurrencyCount deals in other currencies are not ' + 'counted in these totals', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + fontSize: 10, + ), + ), + ], ], ), ), diff --git a/mobile/test/screens/dashboard/dashboard_currency_test.dart b/mobile/test/screens/dashboard/dashboard_currency_test.dart new file mode 100644 index 00000000..55ea98cd --- /dev/null +++ b/mobile/test/screens/dashboard/dashboard_currency_test.dart @@ -0,0 +1,174 @@ +import 'package:bottle_crm/core/theme/theme.dart'; +import 'package:bottle_crm/data/models/auth_response.dart'; +import 'package:bottle_crm/data/models/dashboard_data.dart'; +import 'package:bottle_crm/providers/auth_provider.dart'; +import 'package:bottle_crm/providers/dashboard_provider.dart'; +import 'package:bottle_crm/screens/dashboard/dashboard_screen.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// The dashboard took its currency symbol from the stored org record. Sign-in +/// and the org switch returned `{id, name, role}` and nothing else, so +/// `currencySymbol` was always null and every amount fell back to a dollar +/// sign. Those endpoints now send the currency (see +/// `TestOrgPayloadCarriesCurrency` on the backend), but the dashboard reads it +/// from `/api/dashboard/` instead, which reports the currency it actually +/// priced these totals in. The fake org below therefore carries no symbol on +/// purpose: the screen must not need one. +/// +/// The same block reports how many deals were left out of those totals for +/// being in another currency. Ignoring it made a stage of foreign-currency +/// deals vanish from the chart, because the chart filtered on value and the +/// server had already zeroed it. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + Widget host(DashboardData data) => ProviderScope( + overrides: [ + authProvider.overrideWith(() => _FakeAuth()), + dashboardProvider.overrideWith(() => _FakeDashboard(data)), + ], + child: MaterialApp(theme: AppTheme.light, home: const DashboardScreen()), + ); + + testWidgets('amounts print the org currency, not a dollar sign', ( + tester, + ) async { + await tester.pumpWidget( + host( + const DashboardData( + revenueMetrics: RevenueMetrics( + pipelineValue: 1463538, + currency: 'EUR', + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.textContaining('€'), findsWidgets); + expect(find.textContaining(r'$'), findsNothing); + }); + + testWidgets('a stage holding only foreign-currency deals still appears', ( + tester, + ) async { + await tester.pumpWidget( + host( + const DashboardData( + pipelineByStage: [ + PipelineStage( + code: 'PROPOSAL', + label: 'Proposal', + count: 2, + value: 0, + ), + ], + revenueMetrics: RevenueMetrics(currency: 'USD'), + ), + ), + ); + await tester.pumpAndSettle(); + + // The stage is on the chart, and reports the deals it holds rather than a + // priced zero it cannot back up. + expect(find.text('Proposal'), findsOneWidget); + expect(find.text('2 deals'), findsOneWidget); + }); + + testWidgets('deals left out of the totals are named', (tester) async { + await tester.pumpWidget( + host( + const DashboardData( + pipelineByStage: [ + PipelineStage( + code: 'PROPOSAL', + label: 'Proposal', + count: 4, + value: 90000, + ), + ], + revenueMetrics: RevenueMetrics( + pipelineValue: 90000, + currency: 'USD', + otherCurrencyCount: 3, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.textContaining('3 deals in other currencies'), findsOneWidget); + }); + + testWidgets('an org with one currency gets no exclusion note', ( + tester, + ) async { + await tester.pumpWidget( + host( + const DashboardData( + pipelineByStage: [ + PipelineStage( + code: 'PROPOSAL', + label: 'Proposal', + count: 4, + value: 90000, + ), + ], + revenueMetrics: RevenueMetrics(pipelineValue: 90000), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.textContaining('other currencies'), findsNothing); + }); + + testWidgets('the month result and the weighted forecast are on the board', ( + tester, + ) async { + await tester.pumpWidget( + host( + const DashboardData( + revenueMetrics: RevenueMetrics( + pipelineValue: 200000, + weightedPipeline: 84000, + wonThisMonth: 40000, + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('Won This Month'), findsOneWidget); + expect(find.text('Weighted'), findsOneWidget); + }); +} + +class _FakeDashboard extends DashboardNotifier { + _FakeDashboard(this.data); + + final DashboardData data; + + @override + Future build() async => data; + + @override + Future refresh() async {} +} + +/// Deliberately bare. If any assertion here starts depending on the org +/// record, this is the fake that will catch it. +class _FakeAuth extends AuthNotifier { + @override + AuthState build() { + const org = Organization(id: 'org-1', name: 'Test Org'); + return AuthState( + user: const AuthUser(id: 'user-1', email: 'user@example.com'), + organizations: [org], + selectedOrganization: org, + isAuthenticated: true, + ); + } +} diff --git a/mobile/test/screens/deals/deal_form_currency_test.dart b/mobile/test/screens/deals/deal_form_currency_test.dart new file mode 100644 index 00000000..e9dd4c9d --- /dev/null +++ b/mobile/test/screens/deals/deal_form_currency_test.dart @@ -0,0 +1,61 @@ +import 'package:bottle_crm/core/theme/theme.dart'; +import 'package:bottle_crm/data/models/auth_response.dart'; +import 'package:bottle_crm/providers/auth_provider.dart'; +import 'package:bottle_crm/screens/deals/deal_form_screen.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// A new deal takes its currency from the org, and the org record the client +/// stores was arriving without one, so every deal an org booked defaulted to +/// dollars however it actually trades. The client half was already right; it +/// was reading a field no endpoint sent. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + Widget host(String? orgCurrency) => ProviderScope( + overrides: [authProvider.overrideWith(() => _FakeAuth(orgCurrency))], + child: MaterialApp(theme: AppTheme.light, home: const DealFormScreen()), + ); + + testWidgets('a new deal opens in the currency the org trades in', ( + tester, + ) async { + await tester.pumpWidget(host('EUR')); + await tester.pump(); + + expect(find.text('EUR'), findsOneWidget); + expect(find.text('€'), findsOneWidget); + }); + + testWidgets('an org that never set one still gets a usable default', ( + tester, + ) async { + await tester.pumpWidget(host(null)); + await tester.pump(); + + expect(find.text('USD'), findsOneWidget); + }); +} + +class _FakeAuth extends AuthNotifier { + _FakeAuth(this.orgCurrency); + + final String? orgCurrency; + + @override + AuthState build() { + final org = Organization( + id: 'org-1', + name: 'Test Org', + role: 'ADMIN', + defaultCurrency: orgCurrency, + ); + return AuthState( + user: const AuthUser(id: 'user-1', email: 'user@example.com'), + organizations: [org], + selectedOrganization: org, + isAuthenticated: true, + ); + } +}