Merge pull request #4138 from Spinnich/fix/random-pick-non-gallery-route

fix(v2): drop a random pick that lands after leaving the gallery

Author: gantoine

frontend/src/v2/composables/useIsAlive/index.test.ts

@@ -0,0 +1,25 @@
+import { describe, expect, it } from "vitest";
+import { effectScope } from "vue";
+import { useIsAlive } from "./index";
+
+describe("useIsAlive", () => {
+  it("starts alive and flips on scope disposal", () => {
+    const scope = effectScope();
+    const alive = scope.run(() => useIsAlive())!;
+
+    expect(alive.value).toBe(true);
+    scope.stop();
+    expect(alive.value).toBe(false);
+  });
+
+  it("keeps sibling scopes independent", () => {
+    const a = effectScope();
+    const b = effectScope();
+    const aliveA = a.run(() => useIsAlive())!;
+    const aliveB = b.run(() => useIsAlive())!;
+
+    a.stop();
+    expect(aliveA.value).toBe(false);
+    expect(aliveB.value).toBe(true);
+  });
+});

frontend/src/v2/composables/useIsAlive/index.ts

@@ -0,0 +1,11 @@
+// useIsAlive — tracks whether the owning effect scope is still active, so an
+// async handler that resolves late can tell it's talking to a dead component.
+import { onScopeDispose, shallowRef, type ShallowRef } from "vue";
+
+export function useIsAlive(): ShallowRef<boolean> {
+  const alive = shallowRef(true);
+  onScopeDispose(() => {
+    alive.value = false;
+  });
+  return alive;
+}

frontend/src/v2/views/Gallery/Collection.test.ts

@@ -268,6 +268,48 @@ describe("Collection view random rom", () => {
     expect(push).toHaveBeenCalledWith({ name: "rom", params: { rom: 7 } });
   });
 
+  // Issue #4114: leaving for a route that is not another gallery leaves
+  // `currentCollection` set, so the id check alone cannot tell that the user
+  // walked away.
+  it("drops a pick that lands after the user left the gallery", async () => {
+    let resolvePick: (value: { data: SimpleRom }) => void = () => {};
+    getRandomRom.mockReturnValueOnce(
+      new Promise((resolve) => {
+        resolvePick = resolve;
+      }),
+    );
+
+    const wrapper = await mountView();
+    await wrapper.get("button.random").trigger("click");
+
+    wrapper.unmount();
+
+    resolvePick({ data: rom(42) });
+    await flushPromises();
+
+    expect(push).not.toHaveBeenCalled();
+  });
+
+  it("stays quiet when a pick fails after the user left the gallery", async () => {
+    let failPick: (reason: Error) => void = () => {};
+    getRandomRom.mockReturnValueOnce(
+      new Promise((_resolve, reject) => {
+        failPick = reject;
+      }),
+    );
+
+    const wrapper = await mountView();
+    await wrapper.get("button.random").trigger("click");
+
+    wrapper.unmount();
+
+    failPick(new Error("boom"));
+    await flushPromises();
+
+    expect(snackbarError).not.toHaveBeenCalled();
+    expect(push).not.toHaveBeenCalled();
+  });
+
   it("ignores a click while a pick is in flight", async () => {
     let resolvePick: (value: { data: SimpleRom }) => void = () => {};
     getRandomRom.mockReturnValue(

frontend/src/v2/views/Gallery/Collection.vue

@@ -33,6 +33,7 @@ import CollectionSettingsTab from "@/v2/components/Gallery/CollectionSettingsTab
 import GalleryShell from "@/v2/components/Gallery/GalleryShell.vue";
 import { useCan } from "@/v2/composables/useCan";
 import { useConfirm } from "@/v2/composables/useConfirm";
+import { useIsAlive } from "@/v2/composables/useIsAlive";
 import { usePageTitle } from "@/v2/composables/usePageTitle";
 import { useSnackbar } from "@/v2/composables/useSnackbar";
 import { useWebpSupport } from "@/v2/composables/useWebpSupport";
@@ -281,6 +282,10 @@ function randomScope(): {
   return { collectionId: Number(c.id) };
 }
 
+// Leaving for anything that isn't another gallery keeps `currentCollection`
+// in place, so the id check in `onRandomGame` can't see the user walked away.
+const alive = useIsAlive();
+
 // `/roms/random` samples the pick server-side, so one request resolves it
 // whatever the collection holds. `null` means the collection holds no roms.
 async function onRandomGame() {
@@ -289,7 +294,7 @@ async function onRandomGame() {
   randomLoading.value = true;
   const scopeId = c.id;
   // A pick from the collection the user just left leads nowhere useful.
-  const stale = () => currentCollection.value?.id !== scopeId;
+  const stale = () => !alive.value || currentCollection.value?.id !== scopeId;
   try {
     const { data } = await romApi.getRandomRom(randomScope());
     if (stale()) return;

frontend/src/v2/views/Gallery/Platform.test.ts

@@ -278,6 +278,38 @@ describe("Platform view random rom", () => {
     expect(push).toHaveBeenCalledWith({ name: "rom", params: { rom: 42 } });
   });
 
+  // Issue #4114: leaving for a route that is not another gallery never runs
+  // `resetGallery`, so the store still holds this platform and the id check
+  // alone cannot tell that the user walked away.
+  it("drops a pick that lands after the user left the gallery", async () => {
+    const resolvePick = deferRandomRom();
+
+    const wrapper = await mountView();
+    await wrapper.get("button.random").trigger("click");
+
+    wrapper.unmount();
+
+    resolvePick(rom(42));
+    await flushPromises();
+
+    expect(push).not.toHaveBeenCalled();
+  });
+
+  it("stays quiet when a pick fails after the user left the gallery", async () => {
+    const failPick = deferRandomRomFailure();
+
+    const wrapper = await mountView();
+    await wrapper.get("button.random").trigger("click");
+
+    wrapper.unmount();
+
+    failPick();
+    await flushPromises();
+
+    expect(snackbarError).not.toHaveBeenCalled();
+    expect(push).not.toHaveBeenCalled();
+  });
+
   it("ignores a click while a pick is in flight", async () => {
     let resolvePick: (value: { data: SimpleRom }) => void = () => {};
     getRandomRom.mockReturnValue(

frontend/src/v2/views/Gallery/Platform.vue

@@ -33,6 +33,7 @@ import ScanPlatformDialog from "@/v2/components/Gallery/ScanPlatformDialog.vue";
 import SettingsTab from "@/v2/components/Gallery/SettingsTab.vue";
 import { useCan } from "@/v2/composables/useCan";
 import { useConfirm } from "@/v2/composables/useConfirm";
+import { useIsAlive } from "@/v2/composables/useIsAlive";
 import { usePageTitle } from "@/v2/composables/usePageTitle";
 import { useSnackbar } from "@/v2/composables/useSnackbar";
 import storeGalleryRoms from "@/v2/stores/galleryRoms";
@@ -325,6 +326,10 @@ function onScan() {
   scanOpen.value = true;
 }
 
+// Leaving for anything that isn't another gallery keeps the store's platform
+// in place, so the id check in `onRandomGame` can't see the user walked away.
+const alive = useIsAlive();
+
 // Random ROM — pick one game from this platform and jump to its
 // details. Mirrors the Home RandomPickWidget: `/roms/random` samples the
 // pick server-side, so one request resolves it whatever the platform
@@ -337,7 +342,7 @@ async function onRandomGame() {
   // The pick belongs to the platform that was on screen when the button was
   // clicked; following it after the user moved on would drop them into a
   // game from a gallery they already left.
-  const stale = () => currentPlatform.value?.id !== scopeId;
+  const stale = () => !alive.value || currentPlatform.value?.id !== scopeId;
   try {
     const { data } = await romApi.getRandomRom({ platformIds: [scopeId] });
     if (stale()) return;