From abbd2a94ce6804b0fc1a1c13c18cc2eaf1050b37 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:30:13 +0200 Subject: [PATCH] fix(admin): a failed "Load more" in the desktop release history keeps the list and can be retried On the desktop admin GitHub panel, when "Load more" failed (for example GitHub rate limiting), the whole release history was replaced by the error card for the rest of the session: no list, no Load more button, no way to retry. Had the user found a way to load again, the next load would also have skipped the page that failed. Cause: in its one page per load mode useGithubReleases advanced the page counter whether the request worked or not and never cleared the error, and GitHubPanel showed the error card whenever an error was set. The phone (fillPages mode) already held the page, cleared the error on the next good load and drew the error inline under its timeline. Fix: the one page mode now moves the page on only after a good load and clears the error then, like fillPages. GitHubPanel shows the error card only when nothing has loaded yet; after that it keeps the timeline, shows the error as a line above Load more and leaves the button there. The page walk difference behind fillPages stays as it was. Red tests: - FE-COMP-GHREL-HOOK-012 (new, useGithubReleases.test.ts) failed with "AssertionError: expected [ 1, 2, 3 ] to deeply equal [ 1, 2, 2 ]". - FE-ADMIN-GH-017 (new, GitHubPanel.test.tsx) failed with "Unable to find an element with the text: /v1\.\d\.0/" after the failed load. Changed assertions in src/components/Admin/useGithubReleases.test.ts: - FE-COMP-GHREL-HOOK-004 (renamed from "sets the error and still moves the page on" to "sets the error and holds the page"): requested pages were [1, 2, 3], now [1, 2, 2]. --- .../src/components/Admin/GitHubPanel.test.tsx | 29 +++++++++++++++- client/src/components/Admin/GitHubPanel.tsx | 11 +++++-- .../Admin/useGithubReleases.test.ts | 33 +++++++++++++++++-- .../src/components/Admin/useGithubReleases.ts | 13 ++++---- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/client/src/components/Admin/GitHubPanel.test.tsx b/client/src/components/Admin/GitHubPanel.test.tsx index c1f20094e..7bc8bb211 100644 --- a/client/src/components/Admin/GitHubPanel.test.tsx +++ b/client/src/components/Admin/GitHubPanel.test.tsx @@ -1,4 +1,4 @@ -// FE-ADMIN-GH-001 to FE-ADMIN-GH-016 +// FE-ADMIN-GH-001 to FE-ADMIN-GH-017 import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; @@ -333,4 +333,31 @@ describe('GitHubPanel', () => { // Load more should be hidden (PAGE_2 < 10) expect(screen.queryByText('Load more')).not.toBeInTheDocument(); }); + + it('FE-ADMIN-GH-017: a failed "Load more" keeps the list and the retry loads the page', async () => { + let failPage2 = true; + server.use( + http.get('/api/admin/github-releases', ({ request }) => { + const page = new URL(request.url).searchParams.get('page'); + if (page === '2') { + if (failPage2) return HttpResponse.json({ error: 'rate limited' }, { status: 500 }); + return HttpResponse.json(PAGE_2); + } + return HttpResponse.json(PAGE_1); + }), + ); + const user = userEvent.setup(); + render(); + await screen.findByText('v1.0.0'); + + await user.click(screen.getByText('Load more')); + await screen.findByText(/Failed to load releases/); + expect(screen.getAllByText(/v1\.\d\.0/).length).toBe(10); + + failPage2 = false; + await user.click(screen.getByText('Load more')); + await screen.findByText('v0.0.0'); + expect(screen.getAllByText(/^v[01]\.\d\.0$/).length).toBe(15); + expect(screen.queryByText(/Failed to load releases/)).not.toBeInTheDocument(); + }); }); diff --git a/client/src/components/Admin/GitHubPanel.tsx b/client/src/components/Admin/GitHubPanel.tsx index 9d64967ed..2657f5d34 100644 --- a/client/src/components/Admin/GitHubPanel.tsx +++ b/client/src/components/Admin/GitHubPanel.tsx @@ -69,7 +69,7 @@ function SupportCard({ link }: { link: SupportLink }) { export default function GitHubPanel({ isPrerelease = false }: { isPrerelease?: boolean }) { const { t } = useTranslation(); const { - shownReleases: shown, loading, error, expanded, toggleExpand, hasMore, loadingMore, handleLoadMore, formatDate, + releases, shownReleases: shown, loading, error, expanded, toggleExpand, hasMore, loadingMore, handleLoadMore, formatDate, } = useGithubReleases({ isPrerelease }); // Simple markdown-to-html for release notes (handles headers, bold, lists, links) @@ -183,7 +183,7 @@ export default function GitHubPanel({ isPrerelease = false }: { isPrerelease?: b ); - } else if (error) { + } else if (error && releases.length === 0) { releasesCard = ; } else { releasesCard = ( @@ -276,6 +276,13 @@ export default function GitHubPanel({ isPrerelease = false }: { isPrerelease?: b + {/* A failed "Load more" keeps the timeline and stays retryable */} + {error && ( +

+ {t('admin.github.error')}: {error} +

+ )} + {/* Load more */} {hasMore && (
diff --git a/client/src/components/Admin/useGithubReleases.test.ts b/client/src/components/Admin/useGithubReleases.test.ts index 32b9ce2b8..09abd05c2 100644 --- a/client/src/components/Admin/useGithubReleases.test.ts +++ b/client/src/components/Admin/useGithubReleases.test.ts @@ -1,4 +1,4 @@ -// FE-COMP-GHREL-HOOK-001 to -011: the release history behind both admin shells. +// FE-COMP-GHREL-HOOK-001 to -012: the release history behind both admin shells. import { act, renderHook, waitFor } from '@testing-library/react'; import apiClient from '../../api/client'; @@ -76,7 +76,7 @@ describe('useGithubReleases, one page per load', () => { expect(result.current.loadingMore).toBe(false); }); - it('FE-COMP-GHREL-HOOK-004: a failed load more sets the error and still moves the page on', async () => { + it('FE-COMP-GHREL-HOOK-004: a failed load more sets the error and holds the page', async () => { const spy = serve({ 1: fullPage(1), 2: new Error('rate limited'), 3: [release(21)] }); const { result } = renderHook(() => useGithubReleases({ isPrerelease: false })); await waitFor(() => expect(result.current.loading).toBe(false)); @@ -87,10 +87,37 @@ describe('useGithubReleases, one page per load', () => { await act(async () => { await result.current.handleLoadMore(); }); - expect(requestedPages(spy)).toEqual([1, 2, 3]); + expect(requestedPages(spy)).toEqual([1, 2, 2]); expect(result.current.error).toBe('rate limited'); }); + it('FE-COMP-GHREL-HOOK-012: a failed load more keeps the list and the retry clears the error', async () => { + let failPage2 = true; + const spy = vi + .spyOn(apiClient, 'get') + .mockImplementation(async (_url: string, config?: { params?: { page?: number } }) => { + const page = config?.params?.page; + if (page === 1) return { data: fullPage(1) }; + if (failPage2) throw new Error('offline'); + return { data: [release(11)] }; + }); + const { result } = renderHook(() => useGithubReleases({ isPrerelease: false })); + await waitFor(() => expect(result.current.loading).toBe(false)); + await act(async () => { + await result.current.handleLoadMore(); + }); + expect(result.current.error).toBe('offline'); + expect(result.current.releases).toHaveLength(10); + failPage2 = false; + await act(async () => { + await result.current.handleLoadMore(); + }); + expect(requestedPages(spy)).toEqual([1, 2, 2]); + expect(result.current.error).toBeNull(); + expect(result.current.releases).toHaveLength(11); + expect(result.current.hasMore).toBe(false); + }); + it('FE-COMP-GHREL-HOOK-005: a first page of only prereleases is all a stable install gets', async () => { const spy = serve({ 1: fullPage(1, true), 2: [release(11)] }); const { result } = renderHook(() => useGithubReleases({ isPrerelease: false })); diff --git a/client/src/components/Admin/useGithubReleases.ts b/client/src/components/Admin/useGithubReleases.ts index 9f8ff82a8..b83c0f36a 100644 --- a/client/src/components/Admin/useGithubReleases.ts +++ b/client/src/components/Admin/useGithubReleases.ts @@ -23,9 +23,9 @@ interface UseGithubReleasesOptions { isPrerelease: boolean; /** * Keep pulling pages until at least one release survives the prerelease filter, up to - * MAX_PAGES_PER_LOAD per load. A failed page then keeps what is already shown and - * leaves the page counter where it was, and the next good load clears the error. - * Without it each load reads exactly one page and the page counter moves on regardless. + * MAX_PAGES_PER_LOAD per load. Without it each load reads exactly one page. Either way + * a failed page keeps what is already shown and leaves the page counter where it was, + * and the next good load clears the error. */ fillPages?: boolean; } @@ -59,9 +59,11 @@ export function useGithubReleases({ isPrerelease, fillPages = false }: UseGithub const fetchReleases = async (pageNum = 1, append = false) => { const data = await fetchPage(pageNum); - if (!data) return; + if (!data) return false; setReleases((prev) => (append ? [...prev, ...data] : data)); setHasMore(data.length === PER_PAGE); + setError(null); + return true; }; // Keep pulling pages until at least one release survives the prerelease filter, @@ -102,8 +104,7 @@ export function useGithubReleases({ isPrerelease, fillPages = false }: UseGithub } const next = page + 1; setLoadingMore(true); - await fetchReleases(next, true); - setPage(next); + if (await fetchReleases(next, true)) setPage(next); setLoadingMore(false); };