From b230052272163ef54dd0580eead3f1fa74689a6b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 24 Apr 2026 22:46:03 +0200 Subject: [PATCH 1/2] feat(frontend): refresh button shows in-flight state + toasts Rename the shared freshness Refresh button to "Refreshing..." while the POST /api/recommendations/refresh call is in flight, and surface a sticky info toast on start + a 5s success toast on completion (or an error toast on failure). The freshness bar is re-rendered on success, which naturally updates the "Data from " timestamp. Adds two freshness.test.ts cases: one that holds the refresh promise open to assert the in-flight button text/disabled state + info toast, and one that asserts the error path restores the button and emits an error toast. --- frontend/src/__tests__/freshness.test.ts | 69 ++++++++++++++++++++++++ frontend/src/freshness.ts | 22 +++++++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/freshness.test.ts b/frontend/src/__tests__/freshness.test.ts index d575e60c..0bdc1d77 100644 --- a/frontend/src/__tests__/freshness.test.ts +++ b/frontend/src/__tests__/freshness.test.ts @@ -21,6 +21,7 @@ import { getRecommendationsFreshness, refreshRecommendations, } from '../api/recommendations'; +import type { RefreshRecommendationsResult } from '../api/recommendations'; const mockedGet = getRecommendationsFreshness as jest.MockedFunction; const mockedRefresh = refreshRecommendations as jest.MockedFunction; @@ -120,6 +121,74 @@ test('refresh button triggers POST /recommendations/refresh + onRefresh callback expect(onRefresh).toHaveBeenCalledTimes(1); }); +test('refresh button renames to "Refreshing..." while in flight, then emits a success toast', async () => { + const initial = new Date(Date.now() - 60 * 60_000).toISOString(); + const refreshed = new Date().toISOString(); + mockedGet + .mockResolvedValueOnce({ last_collected_at: initial, last_collection_error: null }) + .mockResolvedValueOnce({ last_collected_at: refreshed, last_collection_error: null }); + + // Hold the refresh API open so we can observe the in-flight state + // before resolving. + let resolveRefresh: ((v: RefreshRecommendationsResult) => void) | null = null; + mockedRefresh.mockImplementation( + () => new Promise((r) => { resolveRefresh = r; }), + ); + const onRefresh = jest.fn().mockResolvedValue(undefined); + + await renderFreshness('fresh', onRefresh); + + const btn = document.querySelector('#fresh-refresh-btn') as HTMLButtonElement; + btn.click(); + // let the click handler run up to the awaited refreshAPI() call + await new Promise((r) => setTimeout(r, 0)); + + expect(btn.textContent).toBe('Refreshing...'); + expect(btn.getAttribute('disabled')).toBe('true'); + + const inFlightToast = document.querySelector('#toast-container .toast-message'); + expect(inFlightToast?.textContent).toBe('Refreshing recommendations…'); + + resolveRefresh!({ recommendations: 0, total_savings: 0 }); + // drain the rest of the handler: refreshAPI → onRefresh → renderFreshness → toasts + for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0)); + + const toastMessages = Array.from( + document.querySelectorAll('#toast-container .toast-message'), + ).map((n) => n.textContent); + expect(toastMessages).toContain('Recommendations refreshed'); + + // Bar was re-rendered with the newer timestamp. + expect(document.querySelector('#fresh')!.textContent).toContain('Data from'); +}); + +test('refresh button restores original text and surfaces an error toast on failure', async () => { + mockedGet.mockResolvedValue({ + last_collected_at: new Date().toISOString(), + last_collection_error: null, + }); + mockedRefresh.mockRejectedValue(new Error('boom')); + const onRefresh = jest.fn(); + + await renderFreshness('fresh', onRefresh); + const btn = document.querySelector('#fresh-refresh-btn') as HTMLButtonElement; + // Swallow the expected console.error from the handler so the test output stays clean. + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + btn.click(); + for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0)); + + expect(btn.textContent).toBe('Refresh'); + expect(btn.hasAttribute('disabled')).toBe(false); + const toastMessages = Array.from( + document.querySelectorAll('#toast-container .toast-message'), + ).map((n) => n.textContent); + expect(toastMessages.some((m) => m?.includes('Refresh failed'))).toBe(true); + expect(onRefresh).not.toHaveBeenCalled(); + + spy.mockRestore(); +}); + test('silently no-ops when the container element is missing', async () => { document.body.replaceChildren(); // container gone await expect(renderFreshness('nonexistent', jest.fn())).resolves.toBeUndefined(); diff --git a/frontend/src/freshness.ts b/frontend/src/freshness.ts index 402611ef..c3a5228b 100644 --- a/frontend/src/freshness.ts +++ b/frontend/src/freshness.ts @@ -14,6 +14,7 @@ import { refreshRecommendations as refreshAPI, } from './api/recommendations'; import type { RecommendationsFreshness } from './api/recommendations'; +import { showToast } from './toast'; import { formatDate, formatRelativeTime } from './utils'; /** @@ -179,17 +180,36 @@ export async function renderFreshness( const bar = buildFreshnessBar(relTime, absTime, containerID, band); container.appendChild(bar); - const btn = document.getElementById(`${containerID}-refresh-btn`); + const btn = document.getElementById(`${containerID}-refresh-btn`) as HTMLButtonElement | null; btn?.addEventListener('click', () => { void (async () => { + const originalText = btn.textContent ?? 'Refresh'; btn.setAttribute('disabled', 'true'); + btn.textContent = 'Refreshing...'; + const inFlight = showToast({ + message: 'Refreshing recommendations…', + kind: 'info', + timeout: null, + }); try { await refreshAPI(); await onRefresh(); await renderFreshness(containerID, onRefresh); + inFlight.dismiss(); + showToast({ + message: 'Recommendations refreshed', + kind: 'success', + timeout: 5_000, + }); } catch (err) { console.error('Refresh failed:', err); + inFlight.dismiss(); + showToast({ + message: `Refresh failed: ${(err as Error).message ?? 'unknown error'}`, + kind: 'error', + }); btn.removeAttribute('disabled'); + btn.textContent = originalText; } })(); }); From eb342cf1c594bbde5582567076da0280ccc0a6bd Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 24 Apr 2026 23:26:33 +0200 Subject: [PATCH 2/2] refactor(frontend): address coderabbit review on refresh button handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use idiomatic `btn.disabled = true/false` instead of setAttribute/ removeAttribute (btn is typed as HTMLButtonElement). - Drop the dead `?? 'Refresh'` fallback on `btn.textContent` — Element .textContent is never null so the branch is unreachable. - Guard the error toast against non-Error throws: `(err as Error).message` would raise a TypeError if `err` were null/undefined, masking the real failure with a crash inside the error handler itself. - Update the freshness tests to assert `btn.disabled` (boolean) instead of the attribute string. --- frontend/src/__tests__/freshness.test.ts | 4 ++-- frontend/src/freshness.ts | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/frontend/src/__tests__/freshness.test.ts b/frontend/src/__tests__/freshness.test.ts index 0bdc1d77..df4d79b3 100644 --- a/frontend/src/__tests__/freshness.test.ts +++ b/frontend/src/__tests__/freshness.test.ts @@ -144,7 +144,7 @@ test('refresh button renames to "Refreshing..." while in flight, then emits a su await new Promise((r) => setTimeout(r, 0)); expect(btn.textContent).toBe('Refreshing...'); - expect(btn.getAttribute('disabled')).toBe('true'); + expect(btn.disabled).toBe(true); const inFlightToast = document.querySelector('#toast-container .toast-message'); expect(inFlightToast?.textContent).toBe('Refreshing recommendations…'); @@ -179,7 +179,7 @@ test('refresh button restores original text and surfaces an error toast on failu for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0)); expect(btn.textContent).toBe('Refresh'); - expect(btn.hasAttribute('disabled')).toBe(false); + expect(btn.disabled).toBe(false); const toastMessages = Array.from( document.querySelectorAll('#toast-container .toast-message'), ).map((n) => n.textContent); diff --git a/frontend/src/freshness.ts b/frontend/src/freshness.ts index c3a5228b..a2ba654b 100644 --- a/frontend/src/freshness.ts +++ b/frontend/src/freshness.ts @@ -183,8 +183,8 @@ export async function renderFreshness( const btn = document.getElementById(`${containerID}-refresh-btn`) as HTMLButtonElement | null; btn?.addEventListener('click', () => { void (async () => { - const originalText = btn.textContent ?? 'Refresh'; - btn.setAttribute('disabled', 'true'); + const originalText = btn.textContent; + btn.disabled = true; btn.textContent = 'Refreshing...'; const inFlight = showToast({ message: 'Refreshing recommendations…', @@ -202,13 +202,22 @@ export async function renderFreshness( timeout: 5_000, }); } catch (err) { + // Guard against non-Error throws: `(err as Error).message` would + // raise a TypeError if `err` were null/undefined, masking the real + // failure with a crash inside the error handler itself. + const message = + err instanceof Error + ? err.message + : err !== null && err !== undefined + ? String(err) + : 'unknown error'; console.error('Refresh failed:', err); inFlight.dismiss(); showToast({ - message: `Refresh failed: ${(err as Error).message ?? 'unknown error'}`, + message: `Refresh failed: ${message}`, kind: 'error', }); - btn.removeAttribute('disabled'); + btn.disabled = false; btn.textContent = originalText; } })();