diff --git a/backend/app/services/github_restore.py b/backend/app/services/github_restore.py index 8087ce430..7e1661d67 100644 --- a/backend/app/services/github_restore.py +++ b/backend/app/services/github_restore.py @@ -848,6 +848,7 @@ class GitHubRestoreService: valid_printers = set((await db.execute(select(Printer.id))).scalars().all()) unresolved = 0 + unlinked_archives = 0 for entry in usage: if not isinstance(entry, dict): @@ -887,6 +888,16 @@ class GitHubRestoreService: old_archive_id = entry.get("archive_id") archive_id = archive_id_map.get(old_archive_id) if isinstance(old_archive_id, int) else None + if archive_id is None and isinstance(old_archive_id, int): + # Restoring spools without archives leaves archive_id_map empty, + # so every "this print consumed that spool" link is dropped — the + # local archive may well exist, but its payload wasn't fetched, + # so there is no natural key here to match it on. Nor is it + # repairable by a later archives-only restore: the dedupe key + # above doesn't include archive_id, so these rows are recognised + # as already-present and skipped. Worth telling the user while + # they can still redo the run with both categories ticked. + unlinked_archives += 1 row = SpoolUsageHistory( spool_id=spool_id, @@ -908,6 +919,11 @@ class GitHubRestoreService: f"{unresolved} usage record(s) skipped — their spool is not in this backup's " "spool list, so there is nothing to attach them to." ) + if unlinked_archives: + tally.note( + f"{unlinked_archives} usage record(s) restored without their print-history link — " + "select Print archives alongside Spool inventory to keep it." + ) async def _restore_settings( self, diff --git a/backend/tests/unit/test_github_restore.py b/backend/tests/unit/test_github_restore.py index 61068211f..472c03f7e 100644 --- a/backend/tests/unit/test_github_restore.py +++ b/backend/tests/unit/test_github_restore.py @@ -439,6 +439,51 @@ class TestRestoreSpools: rows = (await db_session.execute(select(SpoolUsageHistory))).scalars().all() assert len(rows) == 1 + @pytest.mark.asyncio + async def test_dropped_archive_link_is_explained(self, db_session): + """Spools without archives nulls every usage -> archive link, silently.""" + tally = _CategoryTally() + inventory = {"spools": [self._spool_entry(id=41)]} + usage = { + "usage_history": [ + {"spool_id": 41, "archive_id": 7, "weight_used": 1.0, "created_at": "2026-02-01 09:00:00"}, + {"spool_id": 41, "archive_id": 8, "weight_used": 2.0, "created_at": "2026-02-01 10:00:00"}, + {"spool_id": 41, "weight_used": 3.0, "created_at": "2026-02-01 11:00:00"}, + ] + } + + # Empty archive_id_map: the archives category wasn't selected, so its + # payload was never fetched and there is nothing to match against. + await _service()._restore_spools(db_session, inventory, usage, False, tally, {}) + await db_session.commit() + + rows = (await db_session.execute(select(SpoolUsageHistory))).scalars().all() + assert len(rows) == 3 + assert all(row.archive_id is None for row in rows) + # Only the two that had a link to lose are counted. + assert any("2 usage record(s) restored without their print-history link" in n for n in tally.notes) + assert any("select Print archives alongside" in n for n in tally.notes) + + @pytest.mark.asyncio + async def test_no_note_when_every_archive_link_resolves(self, db_session): + tally = _CategoryTally() + inventory = {"spools": [self._spool_entry(id=41)]} + usage = { + "usage_history": [ + {"spool_id": 41, "archive_id": 7, "weight_used": 1.0, "created_at": "2026-02-01 09:00:00"} + ] + } + archive = PrintArchive(filename="linked.3mf", file_path="", file_size=1) + db_session.add(archive) + await db_session.flush() + + await _service()._restore_spools(db_session, inventory, usage, False, tally, {7: archive.id}) + await db_session.commit() + + row = (await db_session.execute(select(SpoolUsageHistory))).scalar_one() + assert row.archive_id == archive.id + assert not any("print-history link" in note for note in tally.notes) + @pytest.mark.asyncio async def test_dangling_printer_id_is_cleared(self, db_session): tally = _CategoryTally() diff --git a/frontend/src/__tests__/components/GitHubRestoreModal.test.tsx b/frontend/src/__tests__/components/GitHubRestoreModal.test.tsx index 77fa0e924..4b39ce15f 100644 --- a/frontend/src/__tests__/components/GitHubRestoreModal.test.tsx +++ b/frontend/src/__tests__/components/GitHubRestoreModal.test.tsx @@ -5,7 +5,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { http, HttpResponse } from 'msw'; +import { delay, http, HttpResponse } from 'msw'; import { QueryClient, useQuery, useQueryClient } from '@tanstack/react-query'; import { render } from '../utils'; import { server } from '../mocks/server'; @@ -198,6 +198,34 @@ describe('GitHubRestoreModal', () => { expect(screen.getByText('1 usage record(s) skipped')).toBeInTheDocument(); }); + it('drops the selection while a newly-picked commit is still being inspected', async () => { + // Switching commits keeps `selected` (it is only pruned once the new preview + // lands), so the footer must not keep counting it: the categories belong to + // the commit that was switched away from, and the user has not seen an item + // count for the new one. + let previewCalls = 0; + server.use( + http.get('/api/v1/github-backup/restore/preview', async () => { + previewCalls += 1; + // The second commit's preview never resolves, holding the modal in the + // in-flight state the assertions below describe. + if (previewCalls > 1) await delay('infinite'); + return HttpResponse.json(mockPreview as unknown as JsonBody); + }) + ); + render(); + + const checkboxes = await waitFor(() => screen.getAllByRole('checkbox') as HTMLInputElement[]); + await userEvent.click(checkboxes[1]); + await waitFor(() => expect(screen.getByText('1 selected')).toBeInTheDocument()); + + await userEvent.selectOptions(screen.getByLabelText('Backup commit'), mockCommits.commits[1].sha); + + await waitFor(() => expect(screen.getByText('Reading backup contents...')).toBeInTheDocument()); + expect(screen.getByText('0 selected')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Restore$/ })).toBeDisabled(); + }); + it('sends overwrite_existing when the toggle is on', async () => { let body: Record | null = null; server.use( diff --git a/frontend/src/components/GitHubRestoreModal.tsx b/frontend/src/components/GitHubRestoreModal.tsx index ef8079d95..e88ab6995 100644 --- a/frontend/src/components/GitHubRestoreModal.tsx +++ b/frontend/src/components/GitHubRestoreModal.tsx @@ -74,11 +74,35 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) { // the one whose contents the user just approved. const resolvedRef = previewQuery.data?.success ? previewQuery.data.ref : selectedRef; + const availability = useMemo(() => { + const map: Record = {}; + previewQuery.data?.categories?.forEach((c) => { + map[c.category] = { available: c.available, itemCount: c.item_count, detail: c.detail }; + }); + return map; + }, [previewQuery.data]); + + // What a Restore click would actually send. `selected` on its own is not that: + // it survives a commit switch by design (the pruning effect below only runs + // once the new preview lands), so between picking a commit and its preview + // resolving, `selected` still describes the *previous* commit while the + // checkbox list is replaced by a spinner. Counting it raw put "2 selected" + // and an enabled Restore button under that spinner, and clicking restored the + // new commit with the old commit's categories — none of which the user had + // seen an item count for. Gating on availability, exactly as the checkboxes + // do, empties the list until the preview says otherwise, which also disables + // the button. + const selectedCategories = useMemo( + () => CATEGORIES.filter((c) => selected[c.id] && availability[c.id]?.available).map((c) => c.id), + [selected, availability] + ); + const selectedCount = selectedCategories.length; + const restoreMutation = useMutation({ mutationFn: () => api.restoreFromGitHub({ ref: resolvedRef, - categories: CATEGORIES.filter((c) => selected[c.id]).map((c) => c.id), + categories: selectedCategories, overwrite_existing: overwriteExisting, }), onSuccess: (data) => { @@ -177,14 +201,6 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) { return () => window.removeEventListener('beforeunload', handler); }, [isRestoring]); - const availability = useMemo(() => { - const map: Record = {}; - previewQuery.data?.categories?.forEach((c) => { - map[c.category] = { available: c.available, itemCount: c.item_count, detail: c.detail }; - }); - return map; - }, [previewQuery.data]); - // Selecting a category that isn't in the newly-picked commit would send a // request the backend rejects, so drop those whenever the preview changes. useEffect(() => { @@ -198,7 +214,6 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) { }); }, [previewQuery.data, availability]); - const selectedCount = CATEGORIES.filter((c) => selected[c.id]).length; const commits = commitsQuery.data?.commits ?? []; const formatCommitLabel = (sha: string, message: string, date: string) => {