Merge pull request #3828 from rommapp/fix/v2-clear-selection-after-delete

fix(v2): clear gallery selection after deleting ROMs

Author: gantoine

backend/endpoints/firmware.py

@@ -284,14 +284,14 @@ async def delete_firmware(
     assert_can(perms, PermEntity.FIRMWARE, PermAction.DELETE)
 
     successful_items = 0
-    failed_items = 0
+    failed_ids = []
     errors = []
 
     for id in firmware:
         fw = db_firmware_handler.get_firmware(id)
         # Treat firmware on a hidden platform as non-existent for this caller.
         if not fw or not perms.can_see_platform(fw.platform_id):
-            failed_items += 1
+            failed_ids.append(id)
             errors.append(f"Firmware with ID {id} not found")
             continue
 
@@ -308,16 +308,16 @@ async def delete_firmware(
                     error = f"Firmware file {hl(fw.file_name)} not found for platform {hl(fw.platform.slug)}"
                     log.error(error)
                     errors.append(error)
-                    failed_items += 1
+                    failed_ids.append(id)
                     continue
 
             successful_items += 1
         except Exception as e:
-            failed_items += 1
+            failed_ids.append(id)
             errors.append(f"Failed to delete firmware {id}: {str(e)}")
 
     return {
         "successful_items": successful_items,
-        "failed_items": failed_items,
+        "failed_ids": failed_ids,
         "errors": errors,
     }

backend/endpoints/responses/__init__.py

@@ -149,5 +149,5 @@ class GenericTaskStatusResponse(BaseTaskStatusResponse):
 
 class BulkOperationResponse(TypedDict):
     successful_items: int
-    failed_items: int
+    failed_ids: list[int]
     errors: list[str]

backend/endpoints/roms/__init__.py

@@ -1896,15 +1896,15 @@ async def delete_roms(
     assert_can(perms, PermEntity.ROMS, PermAction.DELETE)
 
     successful_items = 0
-    failed_items = 0
+    failed_ids = []
     errors = []
 
     for id in roms:
         rom = db_rom_handler.get_rom(id)
 
         # Hidden roms are masked as not-found rather than reported deletable.
         if not rom or not perms.can_see_rom(rom.id, rom.platform_id):
-            failed_items += 1
+            failed_ids.append(id)
             errors.append(f"ROM with ID {id} not found")
             continue
 
@@ -1952,15 +1952,15 @@ async def delete_roms(
 
             successful_items += 1
         except Exception as e:
-            failed_items += 1
+            failed_ids.append(id)
             errors.append(f"Failed to delete ROM {id}: {str(e)}")
 
     if successful_items:
         db_rom_handler.invalidate_filter_values_cache()
 
     return {
         "successful_items": successful_items,
-        "failed_items": failed_items,
+        "failed_ids": failed_ids,
         "errors": errors,
     }
 

backend/tests/endpoints/roms/test_rom.py

@@ -570,6 +570,23 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom):
     assert body["successful_items"] == 1
 
 
+def test_delete_roms_reports_failed_ids(
+    client: TestClient, access_token: str, rom: Rom
+):
+    missing_id = rom.id + 999999
+    response = client.post(
+        "/api/roms/delete",
+        headers={"Authorization": f"Bearer {access_token}"},
+        json={"roms": [rom.id, missing_id], "delete_from_fs": []},
+    )
+    assert response.status_code == status.HTTP_200_OK
+
+    body = response.json()
+    assert body["successful_items"] == 1
+    # The failed id stays reported so the client can keep it selected for retry.
+    assert body["failed_ids"] == [missing_id]
+
+
 @patch(
     "endpoints.roms.fs_rom_handler.remove_directory",
     new_callable=AsyncMock,
@@ -607,7 +624,7 @@ def test_delete_roms_from_fs_flat(
 
     body = response.json()
     assert body["successful_items"] == 1
-    assert body["failed_items"] == 0
+    assert body["failed_ids"] == []
     mock_remove_file.assert_called_once()
     mock_remove_directory.assert_not_called()
 
@@ -651,7 +668,7 @@ def test_delete_roms_from_fs_flat_cleans_empty_parent(
 
     body = response.json()
     assert body["successful_items"] == 1
-    assert body["failed_items"] == 0
+    assert body["failed_ids"] == []
     mock_remove_file.assert_called_once()
     # remove_directory should be called to clean up the empty parent dir
     mock_remove_directory.assert_called_once()
@@ -703,7 +720,7 @@ def test_delete_roms_from_fs_nested(
 
     body = response.json()
     assert body["successful_items"] == 1
-    assert body["failed_items"] == 0
+    assert body["failed_ids"] == []
     mock_remove_directory.assert_called_once()
 
 
@@ -728,7 +745,7 @@ def test_delete_roms_from_fs_missing_file_still_deletes_db_entry(
 
     body = response.json()
     assert body["successful_items"] == 1
-    assert body["failed_items"] == 0
+    assert body["failed_ids"] == []
     assert body["errors"] == []
     assert db_rom_handler.get_rom(rom.id) is None
 

frontend/src/__generated__/models/BulkOperationResponse.ts

@@ -15,6 +15,7 @@ import storeRoms, { type SimpleRom } from "@/stores/roms";
 import type { Events } from "@/types/emitter";
 import { useSnackbar } from "@/v2/composables/useSnackbar";
 import storeGalleryRoms from "@/v2/stores/galleryRoms";
+import storeGallerySelection from "@/v2/stores/gallerySelection";
 
 defineOptions({ inheritAttrs: false });
 
@@ -24,6 +25,7 @@ const route = useRoute();
 const show = ref(false);
 const romsStore = storeRoms();
 const galleryRomsStore = storeGalleryRoms();
+const gallerySelectionStore = storeGallerySelection();
 const roms = ref<SimpleRom[]>([]);
 const romsToDeleteFromFs = ref<number[]>([]);
 const excludeOnDelete = ref(false);
@@ -73,13 +75,27 @@ async function deleteRoms() {
   if (deleting.value) return;
   deleting.value = true;
 
+  // Snapshot the dialog state up front: the dialog is a singleton, so a
+  // fresh `showDeleteRomDialog` event could replace these refs while the
+  // request is in flight. Acting on the snapshot keeps the response tied
+  // to the ROMs it actually processed.
+  const targetRoms = roms.value;
+  const targetPlatformId = platformId.value;
+  const deleteFromFs = romsToDeleteFromFs.value;
+  const exclude = excludeOnDelete.value;
+
   try {
     const response = await romApi.deleteRoms({
-      roms: roms.value,
-      deleteFromFs: romsToDeleteFromFs.value,
+      roms: targetRoms,
+      deleteFromFs,
     });
+    // The backend deletes per-ROM and can partially fail; only prune the
+    // ROMs it actually removed so a failed subset stays visible and
+    // selected for the user to retry.
+    const failedIds = new Set(response.data.failed_ids);
+    const deletedRoms = targetRoms.filter((rom) => !failedIds.has(rom.id));
     snackbar.success(
-      fsCount.value > 0
+      deleteFromFs.length > 0
         ? t("rom.deleted-from-filesystem", {
             count: response.data.successful_items,
           })
@@ -88,8 +104,8 @@ async function deleteRoms() {
           }),
       { icon: "mdi-check-bold" },
     );
-    if (excludeOnDelete.value) {
-      for (const rom of roms.value) {
+    if (exclude) {
+      for (const rom of deletedRoms) {
         const type = rom.has_simple_single_file
           ? "EXCLUDED_SINGLE_FILES"
           : "EXCLUDED_MULTI_FILES";
@@ -101,24 +117,27 @@ async function deleteRoms() {
       }
     }
     romsStore.resetSelection();
-    romsStore.remove(roms.value);
-    galleryRomsStore.remove(roms.value);
+    // Drop the deleted ROMs from the gallery selection
+    gallerySelectionStore.removeIds(deletedRoms.map((rom) => rom.id));
+    romsStore.remove(deletedRoms);
+    galleryRomsStore.remove(deletedRoms);
     romsStore.setRecentRoms(
       romsStore.recentRoms.filter(
-        (r) => !roms.value.some((rom) => rom.id === r.id),
+        (r) => !deletedRoms.some((rom) => rom.id === r.id),
       ),
     );
     romsStore.setContinuePlayingRoms(
       romsStore.continuePlayingRoms.filter(
-        (r) => !roms.value.some((rom) => rom.id === r.id),
+        (r) => !deletedRoms.some((rom) => rom.id === r.id),
       ),
     );
     emitter?.emit("refreshDrawer", null);
     closeDialog();
-    if (route.name === "rom") {
+    // Only leave the single-ROM route when that ROM was actually deleted.
+    if (route.name === "rom" && deletedRoms.length > 0) {
       router.push({
         name: ROUTES.PLATFORM,
-        params: { platform: platformId.value },
+        params: { platform: targetPlatformId },
       });
     }
   } catch (error: unknown) {