Fixed the display of actionable proposals page when a single Sns fails (

#5074) # Motivation Currently, when loading actionable proposals for a single Sns fails, the actionable proposals page displays a loading state. This happens because the loading store only compares successfully loaded Snses and ignores failures. This PR addresses the issue by modifying the `actionableProposalsLoadedStore` to account for failed responses. Note: the proposals page for a failed SNS remains unchanged and displays the loading state indefinitely. # Changes - New `failedActionableSnsesStore`. - Use it in `actionableSnsProposalsByUniverseStore` to calculate the loading state. # Tests - `failedActionableSnsesStore` - `actionableProposalsLoadedStore` uses `failedActionableSnsesStore`. - `loadActionableSnsProposals` updates `failedActionableSnsesStore`. Tested locally by simulating the failing call with adding `throw new Error()` inside the [queryProposals](https://github.com/dfinity/nns-dapp/blob/02ceec03e01235b6926004a619eed1a342882bb9/frontend/src/lib/api/sns-governance.api.ts#L550). # Todos - [x] Add entry to changelog (if necessary). # Screenshots ## Before <img width="1095" alt="image" src="https://github.com/dfinity/nns-dapp/assets/98811342/bb2bd128-0e1e-4fa9-acca-bb7b36d8188d"> ## After <img width="1099" alt="image" src="https://github.com/dfinity/nns-dapp/assets/98811342/904fdd0c-cc0f-4330-b28a-3db4a82b35cf"> ## Failed Sns proposals page (w/o changes) <img width="905" alt="image" src="https://github.com/dfinity/nns-dapp/assets/98811342/eecb458b-2993-4e30-8b72-616ed3157665">
dfinity · Jun 20, 2024 · d65b905 · d65b905
1 parent 8330987
commit d65b905
Show file tree

Hide file tree

Showing 7 changed files with 202 additions and 14 deletions.
diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md
@@ -27,6 +27,8 @@ proposal is successful, the changes it released will be moved from this file to
 
 #### Fixed
 
+* Fixed the display of actionable proposals page when a single SNS fails.
+
 #### Security
 
 #### Not Published

diff --git a/frontend/src/lib/derived/actionable-proposals.derived.ts b/frontend/src/lib/derived/actionable-proposals.derived.ts
@@ -6,7 +6,10 @@ import { selectableUniversesStore } from "$lib/derived/selectable-universes.deri
 import { snsProjectsCommittedStore } from "$lib/derived/sns/sns-projects.derived";
 import { actionableNnsProposalsStore } from "$lib/stores/actionable-nns-proposals.store";
 import { actionableProposalsSegmentStore } from "$lib/stores/actionable-proposals-segment.store";
-import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
+import {
+  actionableSnsProposalsStore,
+  failedActionableSnsesStore,
+} from "$lib/stores/actionable-sns-proposals.store";
 import type { ProposalsNavigationId } from "$lib/types/proposals";
 import type { Universe } from "$lib/types/universe";
 import { isSelectedPath } from "$lib/utils/navigation.utils";
@@ -121,12 +124,14 @@ export const actionableProposalsLoadedStore: Readable<boolean> = derived(
     actionableNnsProposalsStore,
     actionableSnsProposalsStore,
     snsProjectsCommittedStore,
+    failedActionableSnsesStore,
   ],
-  ([nnsProposals, snsProposals, committedSnsProjects]) =>
+  ([nnsProposals, snsProposals, committedSnsProjects, failedSnses]) =>
     nonNullish(nnsProposals.proposals) &&
     // It is expected to have at least one SNS to cover when the projects have not yet been loaded.
     committedSnsProjects.length > 0 &&
-    committedSnsProjects.length === Object.keys(snsProposals).length
+    committedSnsProjects.length ===
+      Object.keys(snsProposals).length + failedSnses.length
 );
 
 // Generate list of ProposalsNavigationId using universes to provide correct order

diff --git a/frontend/src/lib/services/actionable-sns-proposals.services.ts b/frontend/src/lib/services/actionable-sns-proposals.services.ts
@@ -3,7 +3,10 @@ import { MAX_ACTIONABLE_REQUEST_COUNT } from "$lib/constants/constants";
 import { DEFAULT_SNS_PROPOSALS_PAGE_SIZE } from "$lib/constants/sns-proposals.constants";
 import { snsProjectsCommittedStore } from "$lib/derived/sns/sns-projects.derived";
 import { getAuthenticatedIdentity } from "$lib/services/auth.services";
-import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
+import {
+  actionableSnsProposalsStore,
+  failedActionableSnsesStore,
+} from "$lib/stores/actionable-sns-proposals.store";
 import { snsNeuronsStore } from "$lib/stores/sns-neurons.store";
 import { votableSnsNeurons } from "$lib/utils/sns-neuron.utils";
 import {
@@ -42,6 +45,8 @@ export const loadActionableProposalsForSns = async (
         identity,
       });
 
+    failedActionableSnsesStore.remove(rootCanisterIdText);
+
     if (!includeBallotsByCaller) {
       // No need to fetch neurons if there are no actionable proposals support.
       actionableSnsProposalsStore.set({
@@ -80,6 +85,9 @@ export const loadActionableProposalsForSns = async (
     });
   } catch (err) {
     console.error(err);
+
+    // Store the failed root canister ID to provide the correct loading state.
+    failedActionableSnsesStore.add(rootCanisterId.toText());
   }
 };
 

diff --git a/frontend/src/lib/stores/actionable-sns-proposals.store.ts b/frontend/src/lib/stores/actionable-sns-proposals.store.ts
@@ -24,6 +24,12 @@ export interface ActionableSnsProposalsStore
   resetForTesting: () => void;
 }
 
+interface FailedActionableSnsesStore extends Readable<string[]> {
+  add: (rootCanisterId: string) => void;
+  remove: (rootCanisterId: string) => void;
+  resetForTesting: () => void;
+}
+
 /**
  * A store that contains sns proposals that can be voted on by the user (ballots w/ state 0).
  */
@@ -68,4 +74,32 @@ const initActionableSnsProposalsStore = (): ActionableSnsProposalsStore => {
   };
 };
 
+/**
+ * A store that contains root canister ids of SNS projects that failed to load actionable proposals.
+ */
+const initFailedActionableSnsesStore = (): FailedActionableSnsesStore => {
+  const { subscribe, update, set } = writable<string[]>([]);
+
+  return {
+    subscribe,
+
+    add(rootCanisterId: string) {
+      update((currentState: string[]) =>
+        Array.from(new Set([...currentState, rootCanisterId]))
+      );
+    },
+
+    remove(rootCanisterId: string) {
+      update((currentState: string[]) =>
+        currentState.filter((id) => id !== rootCanisterId)
+      );
+    },
+
+    resetForTesting(): void {
+      set([]);
+    },
+  };
+};
+
 export const actionableSnsProposalsStore = initActionableSnsProposalsStore();
+export const failedActionableSnsesStore = initFailedActionableSnsesStore();
diff --git a/frontend/src/tests/lib/derived/actionable-proposals.derived.spec.ts b/frontend/src/tests/lib/derived/actionable-proposals.derived.spec.ts
@@ -13,7 +13,10 @@ import {
 } from "$lib/derived/actionable-proposals.derived";
 import { actionableNnsProposalsStore } from "$lib/stores/actionable-nns-proposals.store";
 import { actionableProposalsSegmentStore } from "$lib/stores/actionable-proposals-segment.store";
-import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
+import {
+  actionableSnsProposalsStore,
+  failedActionableSnsesStore,
+} from "$lib/stores/actionable-sns-proposals.store";
 import { page } from "$mocks/$app/stores";
 import { resetIdentity, setNoIdentity } from "$tests/mocks/auth.store.mock";
 import { mockProposalInfo } from "$tests/mocks/proposal.mock";
@@ -50,6 +53,7 @@ describe("actionable proposals derived stores", () => {
     resetSnsProjects();
     actionableNnsProposalsStore.reset();
     actionableSnsProposalsStore.resetForTesting();
+    failedActionableSnsesStore.resetForTesting();
   });
 
   describe("actionableProposalIndicationEnabledStore", () => {
@@ -398,6 +402,32 @@ describe("actionable proposals derived stores", () => {
 
       expect(get(actionableProposalsLoadedStore)).toEqual(true);
     });
+
+    it("should consider failed snses as loaded", async () => {
+      expect(get(actionableProposalsLoadedStore)).toEqual(false);
+      setSnsProjects([
+        {
+          lifecycle: SnsSwapLifecycle.Committed,
+          rootCanisterId: principal0,
+        },
+        {
+          lifecycle: SnsSwapLifecycle.Committed,
+          rootCanisterId: principal1,
+        },
+      ]);
+      actionableSnsProposalsStore.set({
+        rootCanisterId: principal0,
+        proposals: [],
+        includeBallotsByCaller: false,
+      });
+      actionableNnsProposalsStore.setProposals([mockProposalInfo]);
+
+      expect(get(actionableProposalsLoadedStore)).toEqual(false);
+
+      failedActionableSnsesStore.add(principal1.toText());
+
+      expect(get(actionableProposalsLoadedStore)).toEqual(true);
+    });
   });
 
   describe("actionableProposalsNavigationIdsStore", () => {

diff --git a/frontend/src/tests/lib/services/actionable-sns-proposals.services.spec.ts b/frontend/src/tests/lib/services/actionable-sns-proposals.services.spec.ts
@@ -1,7 +1,10 @@
 import * as api from "$lib/api/sns-governance.api";
 import { snsProjectsCommittedStore } from "$lib/derived/sns/sns-projects.derived";
 import { loadActionableSnsProposals } from "$lib/services/actionable-sns-proposals.services";
-import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
+import {
+  actionableSnsProposalsStore,
+  failedActionableSnsesStore,
+} from "$lib/stores/actionable-sns-proposals.store";
 import { authStore } from "$lib/stores/auth.store";
 import { enumValues } from "$lib/utils/enum.utils";
 import { getSnsNeuronIdAsHexString } from "$lib/utils/sns-neuron.utils";
@@ -29,11 +32,13 @@ import {
   type SnsNeuronId,
   type SnsProposalData,
 } from "@dfinity/sns";
+import type { SpyInstance } from "@vitest/spy";
 import { get } from "svelte/store";
 
 describe("actionable-sns-proposals.services", () => {
   beforeEach(() => {
     vi.restoreAllMocks();
+    failedActionableSnsesStore.resetForTesting();
   });
 
   describe("loadActionableProposalsForSns", () => {
@@ -115,8 +120,14 @@ describe("actionable-sns-proposals.services", () => {
         proposals,
         include_ballots_by_caller: [true],
       }) as SnsListProposalsResponse;
+    const expectedFilterParams = {
+      includeRewardStatus: [
+        SnsProposalRewardStatus.PROPOSAL_REWARD_STATUS_ACCEPT_VOTES,
+      ],
+      limit: 20,
+    };
 
-    let spyQuerySnsProposals;
+    let spyQuerySnsProposals: SpyInstance;
     let spyQuerySnsNeurons;
     let spyConsoleError;
     let includeBallotsByCaller = true;
@@ -177,12 +188,6 @@ describe("actionable-sns-proposals.services", () => {
       await loadActionableSnsProposals();
 
       expect(spyQuerySnsProposals).toHaveBeenCalledTimes(2);
-      const expectedFilterParams = {
-        includeRewardStatus: [
-          SnsProposalRewardStatus.PROPOSAL_REWARD_STATUS_ACCEPT_VOTES,
-        ],
-        limit: 20,
-      };
       expect(spyQuerySnsProposals).toHaveBeenCalledWith({
         identity: mockIdentity,
         rootCanisterId: rootCanisterId1,
@@ -197,6 +202,74 @@ describe("actionable-sns-proposals.services", () => {
       });
     });
 
+    it("should save failed canister IDs", async () => {
+      const failRootCanisterId = principal(13);
+      const snsQueryError = new Error("sns query proposals test fail");
+      mockSnsProjectsCommittedStore([
+        failRootCanisterId,
+        rootCanisterId1,
+        rootCanisterId2,
+      ]);
+      spyQuerySnsProposals = vi
+        .spyOn(api, "queryProposals")
+        .mockRejectedValueOnce(snsQueryError)
+        .mockImplementation(async () => ({
+          proposals: [],
+          include_ballots_by_caller: undefined,
+        }));
+      spyConsoleError = silentConsoleErrors();
+
+      expect(spyQuerySnsProposals).not.toHaveBeenCalled();
+
+      await loadActionableSnsProposals();
+
+      expect(spyQuerySnsProposals).toHaveBeenCalledTimes(3);
+      expect(spyQuerySnsProposals).toHaveBeenCalledWith({
+        identity: mockIdentity,
+        rootCanisterId: failRootCanisterId,
+        certified: false,
+        params: expectedFilterParams,
+      });
+      expect(spyQuerySnsProposals).toHaveBeenCalledWith({
+        identity: mockIdentity,
+        rootCanisterId: rootCanisterId1,
+        certified: false,
+        params: expectedFilterParams,
+      });
+      expect(spyQuerySnsProposals).toHaveBeenCalledWith({
+        identity: mockIdentity,
+        rootCanisterId: rootCanisterId2,
+        certified: false,
+        params: expectedFilterParams,
+      });
+
+      // expect a single error to be logged
+      expect(spyConsoleError).toHaveBeenCalledTimes(1);
+      expect(spyConsoleError).toBeCalledWith(snsQueryError);
+
+      expect(get(failedActionableSnsesStore)).toEqual([
+        failRootCanisterId.toText(),
+      ]);
+    });
+
+    it("should remove failed canister IDs", async () => {
+      mockSnsProjectsCommittedStore([rootCanisterId1]);
+      spyQuerySnsProposals = vi
+        .spyOn(api, "queryProposals")
+        .mockImplementation(async () => ({
+          proposals: [],
+          include_ballots_by_caller: undefined,
+        }));
+      failedActionableSnsesStore.add(rootCanisterId1.toText());
+
+      expect(spyQuerySnsProposals).not.toHaveBeenCalled();
+
+      await loadActionableSnsProposals();
+
+      expect(spyQuerySnsProposals).toHaveBeenCalledTimes(1);
+      expect(get(failedActionableSnsesStore)).toEqual([]);
+    });
+
     it("should query list proposals using multiple calls", async () => {
       mockSnsProjectsCommittedStore([rootCanisterId1]);
       const firstResponse = hundredProposals.slice(0, 20);

diff --git a/frontend/src/tests/lib/stores/actionable-sns-proposals.store.spec.ts b/frontend/src/tests/lib/stores/actionable-sns-proposals.store.spec.ts
@@ -1,4 +1,7 @@
-import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
+import {
+  actionableSnsProposalsStore,
+  failedActionableSnsesStore,
+} from "$lib/stores/actionable-sns-proposals.store";
 import { principal } from "$tests/mocks/sns-projects.mock";
 import { mockSnsProposal } from "$tests/mocks/sns-proposals.mock";
 import type { SnsProposalData } from "@dfinity/sns";
@@ -72,3 +75,36 @@ describe("actionableSnsProposalsStore", () => {
     });
   });
 });
+
+describe("failedActionableSnsesStore", () => {
+  beforeEach(() => {
+    failedActionableSnsesStore.resetForTesting();
+  });
+
+  it("should store root canister ids", () => {
+    expect(get(failedActionableSnsesStore)).toEqual([]);
+    failedActionableSnsesStore.add("1");
+    failedActionableSnsesStore.add("2");
+    expect(get(failedActionableSnsesStore)).toEqual(["1", "2"]);
+  });
+
+  it("should store only unique ids", () => {
+    expect(get(failedActionableSnsesStore)).toEqual([]);
+    failedActionableSnsesStore.add("1");
+    failedActionableSnsesStore.add("2");
+    failedActionableSnsesStore.add("1");
+    expect(get(failedActionableSnsesStore)).toEqual(["1", "2"]);
+  });
+
+  it("should remove ids", () => {
+    expect(get(failedActionableSnsesStore)).toEqual([]);
+    failedActionableSnsesStore.add("1");
+    failedActionableSnsesStore.add("2");
+    failedActionableSnsesStore.add("3");
+    expect(get(failedActionableSnsesStore)).toEqual(["1", "2", "3"]);
+    failedActionableSnsesStore.remove("5");
+    failedActionableSnsesStore.remove("2");
+    failedActionableSnsesStore.remove("2");
+    expect(get(failedActionableSnsesStore)).toEqual(["1", "3"]);
+  });
+});