ソースを参照

fix(backup): don't count a stale selection, and say when archive links are dropped (#2656)

Two smaller restore-path fixes from the same review.

The modal's footer counted `selected` raw while the checkboxes rendered
`selected && isAvailable`. Switching commits keeps `selected` on purpose (it is
only pruned once the new preview lands), so for as long as the new commit's
preview was in flight — with the category list replaced by its spinner — the
footer still read "2 selected" over an enabled Restore button, and clicking it
restored the newly-picked commit with the previous commit's categories, none of
which the user had seen an item count for. The count and the POST body now come
from one `selectedCategories` memo gated on availability, exactly as the
checkboxes are, so both go empty until the preview lands.

Restoring Spool inventory without Print archives leaves archive_id_map empty,
so every usage -> archive link is nulled even where the archive exists locally.
It can't be resolved here (the archives payload isn't fetched for a category
that wasn't selected) and a later archives-only restore won't repair it either,
since the usage dedupe key doesn't include archive_id and those rows read as
already-present. So it gets a note naming the remedy while the user can still
redo the run with both categories ticked.

Carries the rebuilt bundle (index-C2LOlVCR.js -> index-C16HJNOV.js).
jmoore-skild 1 ヶ月 前
コミット
07244b6a43

+ 16 - 0
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,

+ 45 - 0
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()

+ 29 - 1
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(<GitHubRestoreModal onClose={vi.fn()} />);
+
+    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<string, unknown> | null = null;
     server.use(

+ 25 - 10
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<string, { available: boolean; itemCount: number; detail: string | null }> = {};
+    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<string, { available: boolean; itemCount: number; detail: string | null }> = {};
-    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) => {