From 39e22b03d0ef24590fa2d3e39dd348cc4808ca76 Mon Sep 17 00:00:00 2001 From: Ashwin Date: Wed, 5 Aug 2026 22:37:50 +0530 Subject: [PATCH] Implement board creation API and enhance currency handling across frontend components --- backend/opportunity/serializer.py | 23 +++ .../tests/test_opportunities_api.py | 132 +++++++++++++ frontend/src/lib/server/v2/board.js | 22 +++ .../src/routes/(app)/pipeline/+page.server.js | 13 +- .../src/routes/(app)/pipeline/+page.svelte | 10 +- .../routes/(app)/pipeline/[id]/+page.svelte | 13 +- .../(app)/pipeline/[id]/edit/+page.svelte | 5 +- .../routes/(app)/pipeline/new/+page.server.js | 9 +- .../routes/(app)/pipeline/new/+page.svelte | 4 +- .../routes/(app)/tasks/board/+page.server.js | 27 ++- .../src/routes/(app)/tasks/board/+page.svelte | 48 ++++- mobile/lib/services/api_service.dart | 66 ++++++- .../test/unit/api_service_refresh_test.dart | 177 ++++++++++++++++++ 13 files changed, 522 insertions(+), 27 deletions(-) create mode 100644 mobile/test/unit/api_service_refresh_test.dart diff --git a/backend/opportunity/serializer.py b/backend/opportunity/serializer.py index 68b51538..789b8ced 100644 --- a/backend/opportunity/serializer.py +++ b/backend/opportunity/serializer.py @@ -1,3 +1,4 @@ +from django.db.models import Sum from drf_spectacular.utils import extend_schema_field from rest_framework import serializers @@ -263,6 +264,28 @@ class OpportunityCreateSerializer(serializers.ModelSerializer): if stage in AMOUNT_REQUIRED_STAGES and not self._resolved(data, "amount"): errors["amount"] = "A won deal has to record what it was worth." + # `amount` stops being the client's field once the deal has line items. + # `OpportunityLineItem.save()` calls `recalculate_amount()`, which sets + # `amount` from the lines and stamps `amount_source = "CALCULATED"`. + # Nothing recomputed it on the deal's own update path, so a request + # carrying a different figure overwrote the total while leaving the + # source still claiming the number came from the lines. The record then + # asserted two incompatible things about the same money. + # + # An unchanged value is accepted rather than refused, because clients + # that send the whole record back on every edit (the mobile deal form + # does) must still be able to move the stage or the close date. Only an + # actual attempt to change the figure is an error. + if self.instance is not None and "amount" in data: + line_items = self.instance.line_items + if line_items.exists(): + line_total = line_items.aggregate(total=Sum("total"))["total"] or 0 + if data["amount"] != line_total: + errors["amount"] = ( + "This deal's amount is the sum of its line items. " + "Edit the line items to change it." + ) + if errors: raise serializers.ValidationError(errors) return data diff --git a/backend/opportunity/tests/test_opportunities_api.py b/backend/opportunity/tests/test_opportunities_api.py index 7f00c7b9..1d2fe3c5 100644 --- a/backend/opportunity/tests/test_opportunities_api.py +++ b/backend/opportunity/tests/test_opportunities_api.py @@ -1867,3 +1867,135 @@ class TestOpportunityNonAdminAssigned: format="json", ) assert response.status_code == status.HTTP_200_OK + + +class TestAmountIsOwnedByLineItems: + """A deal with line items owns its own `amount`. + + `OpportunityLineItem.save()` calls `recalculate_amount()`, which writes + `amount` from the lines and stamps `amount_source = "CALCULATED"`. Nothing + recomputed it on the deal's own update path, so an ordinary PUT or PATCH + could set a different figure and leave the source still claiming the number + came from the lines. + + The web edit form disables the amount input when line items exist, but a UI + guard is not a guard: the mobile form has no equivalent and neither does + curl. + """ + + @pytest.fixture() + def opportunity(self, admin_user, org_a): + _set_rls(org_a) + return Opportunity.objects.create( + name="Line Item Owned Deal", + stage="QUALIFICATION", + amount=Decimal("100.00"), + org=org_a, + created_by=admin_user, + ) + + @pytest.fixture() + def priced(self, opportunity, org_a): + """One line item of 2 x 250, so the deal is worth 500 and knows it.""" + _set_rls(org_a) + OpportunityLineItem.objects.create( + opportunity=opportunity, + name="Widget", + quantity=2, + unit_price=Decimal("250.00"), + org=org_a, + ) + opportunity.refresh_from_db() + return opportunity + + def test_line_items_take_ownership_of_the_amount(self, priced): + """The premise of every test below.""" + assert priced.amount == Decimal("500.00") + assert priced.amount_source == "CALCULATED" + + def test_patching_a_different_amount_is_refused(self, admin_client, priced): + response = admin_client.patch( + _detail_url(priced.pk), {"amount": "9.99"}, format="json" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "amount" in response.data["errors"] + priced.refresh_from_db() + assert priced.amount == Decimal("500.00") + assert priced.amount_source == "CALCULATED" + + def test_putting_a_different_amount_is_refused(self, admin_client, priced): + """PUT is the verb the mobile edit form uses.""" + response = admin_client.put( + _detail_url(priced.pk), + { + "name": priced.name, + "stage": "QUALIFICATION", + "amount": "9.99", + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + priced.refresh_from_db() + assert priced.amount == Decimal("500.00") + + def test_clearing_the_amount_is_refused(self, admin_client, priced): + response = admin_client.patch( + _detail_url(priced.pk), {"amount": None}, format="json" + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + priced.refresh_from_db() + assert priced.amount == Decimal("500.00") + + def test_echoing_the_computed_amount_is_allowed(self, admin_client, priced): + """A client that sends the whole record back must still be able to edit. + + This is the case that stops the rule from breaking the mobile form, + which posts every field on every save. + """ + response = admin_client.put( + _detail_url(priced.pk), + { + "name": priced.name, + "stage": "PROPOSAL", + "amount": "500.00", + }, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + priced.refresh_from_db() + assert priced.stage == "PROPOSAL" + assert priced.amount == Decimal("500.00") + + def test_an_edit_that_omits_the_amount_is_allowed(self, admin_client, priced): + response = admin_client.patch( + _detail_url(priced.pk), {"description": "Still on track"}, format="json" + ) + assert response.status_code == status.HTTP_200_OK + priced.refresh_from_db() + assert priced.amount == Decimal("500.00") + + def test_a_deal_without_line_items_still_owns_its_amount( + self, admin_client, opportunity + ): + """The rule must not leak onto ordinary deals, which are typed by hand.""" + response = admin_client.patch( + _detail_url(opportunity.pk), {"amount": "1234.00"}, format="json" + ) + assert response.status_code == status.HTTP_200_OK + opportunity.refresh_from_db() + assert opportunity.amount == Decimal("1234.00") + + def test_deleting_the_last_line_item_hands_the_amount_back( + self, admin_client, priced, org_a + ): + _set_rls(org_a) + priced.line_items.first().delete() + priced.refresh_from_db() + assert priced.amount_source == "MANUAL" + + response = admin_client.patch( + _detail_url(priced.pk), {"amount": "42.00"}, format="json" + ) + assert response.status_code == status.HTTP_200_OK + priced.refresh_from_db() + assert priced.amount == Decimal("42.00") diff --git a/frontend/src/lib/server/v2/board.js b/frontend/src/lib/server/v2/board.js index 37ee253b..3c4426ef 100644 --- a/frontend/src/lib/server/v2/board.js +++ b/frontend/src/lib/server/v2/board.js @@ -109,6 +109,28 @@ export async function createColumn({ cookies }, boardId, { name, color, order }) return apiRequest(`/boards/${boardId}/columns/`, { method: 'POST', body }, { cookies }); } +/** + * Create a board. + * + * `BoardListCreateView.post` derives `org`, `owner` and `created_by` from the + * JWT, enrols the caller as the owner member, and, unless told otherwise, + * seeds the three default columns. So the whole write surface is a name and an + * optional description, and a fresh board is usable the moment it exists. + * + * Until this existed no client could create one. The page said to use the + * mobile app, which has no board feature at all, so an org that had never been + * given a board through the API could not get one. + * + * @param {{ cookies: import('@sveltejs/kit').Cookies }} event + * @param {{ name: string, description?: string }} board + */ +export async function createBoard({ cookies }, { name, description }) { + /** @type {Record} */ + const body = { name }; + if (description) body.description = description; + return apiRequest('/boards/', { method: 'POST', body }, { cookies }); +} + /** * Add a card to a column. Any board member may; the backend derives the card's * `column`, `org` and `created_by` from the URL and the JWT. The write surface diff --git a/frontend/src/routes/(app)/pipeline/+page.server.js b/frontend/src/routes/(app)/pipeline/+page.server.js index e8d92c0d..a7315d9d 100644 --- a/frontend/src/routes/(app)/pipeline/+page.server.js +++ b/frontend/src/routes/(app)/pipeline/+page.server.js @@ -67,7 +67,18 @@ export async function load(event) { // from this `.server.js` module (SvelteKit forbids that, the same // protection as `$lib/server/`); the toggle just needs to know which of // the CURRENT params survive the switch. - boardFields: BOARD_FIELDS + boardFields: BOARD_FIELDS, + // The currency for figures that are sums rather than one deal: the header + // totals and the board lane subtotals. A per-deal currency cannot label a + // sum, and `money()` would otherwise fall back to a hardcoded USD and + // print a dollar sign over a euro figure. + // + // This does not make a mixed-currency sum correct. `get_totals` in + // `opportunity_views.py` is `SUM(amount)` over every row regardless of + // currency, so on an org running more than one the total is 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. + orgCurrency: /** @type {any} */ (locals).org_settings?.default_currency || 'USD' }; if (boardMode) { diff --git a/frontend/src/routes/(app)/pipeline/+page.svelte b/frontend/src/routes/(app)/pipeline/+page.svelte index 6bd778d3..c514d364 100644 --- a/frontend/src/routes/(app)/pipeline/+page.svelte +++ b/frontend/src/routes/(app)/pipeline/+page.svelte @@ -74,8 +74,8 @@ that was only ever true under the old hardcoded ?open=true would lie here as soon as somebody switched presets. --> {count(totals.count)} deals · - {money(totals.amount_sum)} · - {money(totals.weighted_sum)} weighted · + {money(totals.amount_sum, data.orgCurrency)} · + {money(totals.weighted_sum, data.orgCurrency)} weighted · {totals.stalled_count} stalled {/snippet} {#snippet actions()} @@ -119,7 +119,7 @@
{STAGE_LABEL[lane.stage]} - {count(lane.count)} · {money(lane.sum)} + {count(lane.count)} · {money(lane.sum, data.orgCurrency)}
{#if lane.truncated} @@ -143,7 +143,7 @@
- {money(d.amount)} + {money(d.amount, d.currency)} {shortDate(d.closed_on)} @@ -197,7 +197,7 @@ {AGING_LABEL[d.aging_status]} - {money(d.amount)} + {money(d.amount, d.currency)} {shortDate(d.closed_on)} - {li.discount_amount > 0 ? `−${money(li.discount_amount)}` : ''} + {li.discount_amount > 0 ? `−${money(li.discount_amount, deal.currency)}` : ''} - {money(li.total)} + {money(li.total, deal.currency)} {/each} @@ -116,12 +116,13 @@ > {#if discount > 0} Subtotal - {money(subtotal)} + {money(subtotal, deal.currency)} Discounts - −{money(discount)} + −{money(discount, deal.currency)} {/if} Total - {money(deal.amount)} + {money(deal.amount, deal.currency)}
{/if} diff --git a/frontend/src/routes/(app)/pipeline/[id]/edit/+page.svelte b/frontend/src/routes/(app)/pipeline/[id]/edit/+page.svelte index fe06cdb6..5b63067f 100644 --- a/frontend/src/routes/(app)/pipeline/[id]/edit/+page.svelte +++ b/frontend/src/routes/(app)/pipeline/[id]/edit/+page.svelte @@ -302,8 +302,9 @@ {:else if amountIsCalculated}

{server.line_item_count} line items add up to - {money(server.line_item_total)}. The server recalculates - this on every save, so typing over it would not last. Edit the line items instead. + {money(server.line_item_total, deal.currency)}. The server + refuses a different figure on a deal with line items, so this cannot be typed over. Edit + the line items instead.

{:else}

diff --git a/frontend/src/routes/(app)/pipeline/new/+page.server.js b/frontend/src/routes/(app)/pipeline/new/+page.server.js index 06070205..2d052349 100644 --- a/frontend/src/routes/(app)/pipeline/new/+page.server.js +++ b/frontend/src/routes/(app)/pipeline/new/+page.server.js @@ -3,7 +3,14 @@ import { EDITABLE_FIELDS, createDeal, getDealFormOptions } from '$lib/server/v2/ /** @type {import('./$types').PageServerLoad} */ export async function load(event) { - return await getDealFormOptions(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' + }; } /** @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 56b6b69f..6f8002cf 100644 --- a/frontend/src/routes/(app)/pipeline/new/+page.svelte +++ b/frontend/src/routes/(app)/pipeline/new/+page.svelte @@ -210,7 +210,9 @@

{errors.amount}

{:else}

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

{/if} diff --git a/frontend/src/routes/(app)/tasks/board/+page.server.js b/frontend/src/routes/(app)/tasks/board/+page.server.js index 03795ee9..64735028 100644 --- a/frontend/src/routes/(app)/tasks/board/+page.server.js +++ b/frontend/src/routes/(app)/tasks/board/+page.server.js @@ -1,5 +1,11 @@ import { fail } from '@sveltejs/kit'; -import { createCard, createColumn, getBoard, moveBoardTask } from '$lib/server/v2/board.js'; +import { + createBoard, + createCard, + createColumn, + getBoard, + moveBoardTask +} from '$lib/server/v2/board.js'; import { readableError } from '$lib/server/v2/form-errors.js'; /** Map an api-helpers error to a form `fail`, preserving a 403 as a 403. */ @@ -33,6 +39,25 @@ export const actions = { } }, + // Create the org's first board, or another one. + // + // The empty state used to send people to the mobile app for this. The mobile + // app has no board feature, and no client called `POST /boards/` at all, so + // an org that had never been handed a board through the API had no way to + // get one. The backend seeds the three default columns, so the board is + // usable as soon as it lands. + create: async ({ request, cookies }) => { + const form = await request.formData(); + const name = String(form.get('name') || '').trim(); + if (!name) return fail(400, { error: 'A board needs a name.' }); + try { + await createBoard({ cookies }, { name }); + return { added: 'board' }; + } catch (err) { + return toFail(err, 'Could not create the board.'); + } + }, + // Add a card to a column. Any board member may; the backend is the boundary. addCard: async ({ request, cookies }) => { const form = await request.formData(); diff --git a/frontend/src/routes/(app)/tasks/board/+page.svelte b/frontend/src/routes/(app)/tasks/board/+page.svelte index 91c4b201..87e6433c 100644 --- a/frontend/src/routes/(app)/tasks/board/+page.svelte +++ b/frontend/src/routes/(app)/tasks/board/+page.svelte @@ -72,6 +72,10 @@ let addColumnBusy = $state(false); let addColumnError = $state(''); + // Only ever seen on the empty state, where there is no board to add to yet. + let createBoardBusy = $state(false); + let createBoardError = $state(''); + function openCardForm(/** @type {string} */ columnId) { addingCardTo = columnId; addCardError = ''; @@ -181,14 +185,54 @@ {#if !data.board} +

No boards yet

- A board organises work into columns you drag cards between. Create one from the mobile app - to see it here. + A board organises work into columns you drag cards between. It starts with To Do, In + Progress and Done, and you can rename them later.

+
{ + createBoardBusy = true; + return async ({ result }) => { + createBoardBusy = false; + if (result.type === 'success') { + createBoardError = ''; + await invalidateAll(); + } else if (result.type === 'failure') { + createBoardError = + /** @type {any} */ (result.data)?.error || 'Could not create the board.'; + } else if (result.type === 'error') { + createBoardError = 'Could not create the board.'; + } + }; + }} + > + + + +
+ {#if createBoardError} +

{createBoardError}

+ {/if}
{:else} diff --git a/mobile/lib/services/api_service.dart b/mobile/lib/services/api_service.dart index 9d914ca7..0807c328 100644 --- a/mobile/lib/services/api_service.dart +++ b/mobile/lib/services/api_service.dart @@ -1,11 +1,13 @@ import 'dart:convert'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; +import 'package:jwt_decoder/jwt_decoder.dart'; import '../config/api_config.dart'; -/// Callback invoked when an authenticated request gets a 401, should hit the -/// refresh endpoint and update the ApiService's access token, returning true -/// if the new token is ready and the original request can be retried. +/// Callback invoked when the access token has aged out (or, as a fallback, when +/// a request comes back 401). Should hit the refresh endpoint and update the +/// ApiService's access token, returning true if the new token is ready and the +/// original request can be sent. typedef RefreshTokenCallback = Future Function(); /// API response wrapper @@ -36,7 +38,14 @@ class ApiService { factory ApiService() => _instance; ApiService._internal(); - final http.Client _client = http.Client(); + http.Client _client = http.Client(); + + /// Swap the HTTP client. Tests only: the app shares one client for the + /// process lifetime, and `ApiService` is a singleton. + @visibleForTesting + void setClientForTesting(http.Client client) { + _client = client; + } // Token and org getters - will be set by AuthService String? _accessToken; @@ -88,16 +97,55 @@ class ApiService { } } - /// Send an HTTP request and transparently retry once on 401 after refreshing - /// the access token. The `send` closure must be safe to invoke twice, every - /// caller below builds a fresh request inside it. + /// How close to expiry counts as expired. + /// + /// Covers a token that would age out while the request is in flight, and + /// small clock differences between the phone and the server. + static const Duration _expiryGrace = Duration(seconds: 30); + + /// True when the access token is at or near its `exp`. + /// + /// A token that will not decode returns false: refreshing cannot repair a + /// malformed credential, so the request carries it and the server answers. + bool get _accessTokenNeedsRefresh { + final token = _accessToken; + if (token == null) return false; + try { + return JwtDecoder.getExpirationDate( + token, + ).isBefore(DateTime.now().add(_expiryGrace)); + } catch (_) { + return false; + } + } + + /// Send an HTTP request, refreshing the access token first when it has aged + /// out. The `send` closure must be safe to invoke twice, every caller below + /// builds a fresh request inside it. Future _sendWithRetry({ required bool requiresAuth, required Future Function(Map headers) send, }) async { + // Refresh BEFORE sending, not after a rejection. + // + // This API does not answer 401 for an expired credential. `HasOrgContext` + // (backend `common/permissions.py`) denies with 403 before DRF's auth layer + // would emit a 401, and it answers the same way for a missing, malformed or + // expired token. So the 401 branch below never fires on expiry, and a + // session that aged out while the app was open stayed broken until the next + // cold start, where `AuthService.initialize()` does this same check once. + // + // The web client refreshes the same way, off the `exp` claim on every + // request (`frontend/src/hooks.server.js`), rather than off a status code. + if (requiresAuth && _accessTokenNeedsRefresh && _refreshCallback != null) { + await _refreshAccessToken(); + } + var response = await send(_buildHeaders(requiresAuth: requiresAuth)) .timeout(ApiConfig.connectTimeout); + // Kept as a fallback for a token revoked or rotated out from under us + // before its `exp`, which the check above cannot see. if (requiresAuth && response.statusCode == 401 && _accessToken != null && @@ -267,8 +315,10 @@ class ApiService { bool requiresAuth = true, }) async { try { + // The URL only. A request body is never safe to log here: the refresh + // call carries the refresh token, sign-in carries the Google id token, + // and an ordinary create carries a customer's name, email and phone. debugPrint('POST $url'); - debugPrint('Body: ${jsonEncode(body)}'); final uri = Uri.parse(url); final encoded = jsonEncode(body); diff --git a/mobile/test/unit/api_service_refresh_test.dart b/mobile/test/unit/api_service_refresh_test.dart new file mode 100644 index 00000000..0f68aab8 --- /dev/null +++ b/mobile/test/unit/api_service_refresh_test.dart @@ -0,0 +1,177 @@ +import 'dart:convert'; + +import 'package:bottle_crm/config/api_config.dart'; +import 'package:bottle_crm/services/api_service.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; + +/// A JWT that decodes but is not signed. `JwtDecoder` reads the claims without +/// verifying, which is all the expiry check does. +String jwtExpiring(Duration fromNow) { + String seg(Map m) => + base64Url.encode(utf8.encode(jsonEncode(m))).replaceAll('=', ''); + final exp = DateTime.now().add(fromNow).millisecondsSinceEpoch ~/ 1000; + return '${seg({'alg': 'HS256', 'typ': 'JWT'})}.${seg({'exp': exp})}.sig'; +} + +void main() { + // The singleton carries state between tests, so every test sets what it + // needs and clears afterwards. + tearDown(() { + ApiService().clearAuth(); + ApiService().setRefreshCallback(null); + ApiService().setClientForTesting(http.Client()); + }); + + group('access token refresh', () { + test('refreshes an expired token before sending, even though the API ' + 'answers 403 rather than 401', () async { + final api = ApiService(); + final sentTokens = []; + var refreshCalls = 0; + + api.setAccessToken(jwtExpiring(const Duration(minutes: -5))); + api.setRefreshCallback(() async { + refreshCalls++; + api.setAccessToken(jwtExpiring(const Duration(hours: 1))); + return true; + }); + api.setClientForTesting( + MockClient((request) async { + sentTokens.add(request.headers['Authorization']); + // What the real API returns for a credential it will not accept: + // HasOrgContext denies before DRF's auth layer emits a 401. + return http.Response( + jsonEncode({'detail': 'Organization context is required.'}), + 403, + ); + }), + ); + + await api.get(ApiConfig.leads); + + expect(refreshCalls, 1, reason: 'expiry must trigger exactly one refresh'); + // One send, not two: the token was renewed on the way out, so there is + // nothing to retry. Before this fix the count was one send and zero + // refreshes, because the 401 branch never saw a 401. + expect(sentTokens, hasLength(1)); + }); + + test('the request carries the refreshed token, not the expired one', + () async { + final api = ApiService(); + final expired = jwtExpiring(const Duration(minutes: -5)); + final fresh = jwtExpiring(const Duration(hours: 1)); + String? sent; + + api.setAccessToken(expired); + api.setRefreshCallback(() async { + api.setAccessToken(fresh); + return true; + }); + api.setClientForTesting( + MockClient((request) async { + sent = request.headers['Authorization']; + return http.Response('{}', 200); + }), + ); + + await api.get(ApiConfig.leads); + + expect(sent, 'Bearer $fresh'); + expect(sent, isNot('Bearer $expired')); + }); + + test('a live token is sent as-is, with no refresh round-trip', () async { + final api = ApiService(); + final live = jwtExpiring(const Duration(hours: 1)); + var refreshCalls = 0; + String? sent; + + api.setAccessToken(live); + api.setRefreshCallback(() async { + refreshCalls++; + return true; + }); + api.setClientForTesting( + MockClient((request) async { + sent = request.headers['Authorization']; + return http.Response('{}', 200); + }), + ); + + await api.get(ApiConfig.leads); + + expect(refreshCalls, 0); + expect(sent, 'Bearer $live'); + }); + + test('a token inside the grace window counts as expired', () async { + final api = ApiService(); + var refreshCalls = 0; + + // Valid for another 5 seconds, which is less than the 30-second grace: + // it would age out mid-flight. + api.setAccessToken(jwtExpiring(const Duration(seconds: 5))); + api.setRefreshCallback(() async { + refreshCalls++; + api.setAccessToken(jwtExpiring(const Duration(hours: 1))); + return true; + }); + api.setClientForTesting( + MockClient((_) async => http.Response('{}', 200)), + ); + + await api.get(ApiConfig.leads); + + expect(refreshCalls, 1); + }); + + test('an unauthenticated call never refreshes', () async { + final api = ApiService(); + var refreshCalls = 0; + + api.setAccessToken(jwtExpiring(const Duration(minutes: -5))); + api.setRefreshCallback(() async { + refreshCalls++; + return true; + }); + api.setClientForTesting( + MockClient((_) async => http.Response('{}', 200)), + ); + + // The refresh endpoint itself posts with requiresAuth: false. Without + // this exemption the expired token would trigger a refresh, whose own + // request would trigger another, and `_refreshInFlight` would return the + // future that is waiting on it. + await api.post(ApiConfig.refreshToken, {'refresh': 'x'}, + requiresAuth: false); + + expect(refreshCalls, 0); + }); + + test('a token that will not decode is sent as-is', () async { + final api = ApiService(); + var refreshCalls = 0; + String? sent; + + api.setAccessToken('not-a-jwt'); + api.setRefreshCallback(() async { + refreshCalls++; + return true; + }); + api.setClientForTesting( + MockClient((request) async { + sent = request.headers['Authorization']; + return http.Response('{}', 403); + }), + ); + + await api.get(ApiConfig.leads); + + expect(refreshCalls, 0, reason: 'refreshing cannot repair a bad token'); + expect(sent, 'Bearer not-a-jwt'); + }); + }); +}