Skip to content

Commit

Permalink
NNS1-3504: Sort selectable universes by actionable proposals (#6131)
Browse files Browse the repository at this point in the history
# Motivation

On the voting page we want to sort the universes (both in the selector
and in the actionable proposals) based on the number of actionably
proposals or otherwise alphabetically.

Both the selector and the actionable proposals sections are rendered
based on `selectableUniversesStore`.
(This store is also used for other things, such as [finding the
currently selected
universe](https://github.com/dfinity/nns-dapp/blob/58b8d349c2912b678f69b1060342430f6992fb12/frontend/src/lib/components/accounts/IcrcWalletPage.svelte#L166),
which is why it can still also contain non-voting universes such as
ckBTC.)

# Changes

1. Sort `selectableUniversesStore` based on actionable proposal count
and title.

# Tests

1. Simplify the existing tests for `selectableUniversesStore`. Because
of the re-ordering, the tests were confusing in their previous form.
2. Add unit tests with multiple SNS projects.
3. Adapt `ActionableProposals.spec.ts` to the new ordering.
4. Manually tested at
https://qsgjb-riaaa-aaaaa-aaaga-cai.dskloet-ingress.devenv.dfinity.network/proposals/?actionable

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Jan 10, 2025
1 parent d724184 commit e038cc7
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ proposal is successful, the changes it released will be moved from this file to

- Allow `dfinity:` token prefix in QR code for ICP payment.
- Recover from interrupted canister creation in the frontend.
- Sort projects on the Voting page based on actionable proposals and name.

#### Deprecated

Expand Down
66 changes: 58 additions & 8 deletions frontend/src/lib/derived/selectable-universes.derived.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,80 @@
import { OWN_CANISTER_ID_TEXT } from "$lib/constants/canister-ids.constants";
import {
actionableProposalCountStore,
type ActionableProposalCountData,
} from "$lib/derived/actionable-proposals.derived";
import {
icrcCanistersStore,
type IcrcCanistersStore,
type IcrcCanistersStoreData,
} from "$lib/derived/icrc-canisters.derived";
import { pageStore, type Page } from "$lib/derived/page.derived";
import type { Universe } from "$lib/types/universe";
import {
createAscendingComparator,
createDescendingComparator,
mergeComparators,
} from "$lib/utils/sort.utils";
import { isAllTokensPath, isUniverseCkBTC } from "$lib/utils/universe.utils";
import { isNullish } from "@dfinity/utils";
import { derived, type Readable } from "svelte/store";
import { universesStore } from "./universes.derived";

type UniverseWithActionableProposalCount = {
universe: Universe;
actionableProposalCount: number;
};

const compareNnsFirst = createDescendingComparator(
({ universe: { canisterId } }: UniverseWithActionableProposalCount) =>
canisterId === OWN_CANISTER_ID_TEXT
);

const compareActionableProposalCount = createDescendingComparator(
({ actionableProposalCount }: UniverseWithActionableProposalCount) =>
actionableProposalCount
);

const compareTitle = createAscendingComparator(
({ universe: { title } }: UniverseWithActionableProposalCount) =>
title.toLowerCase()
);

export const selectableUniversesStore = derived<
[Readable<Universe[]>, Readable<Page>, IcrcCanistersStore],
[
Readable<Universe[]>,
Readable<Page>,
IcrcCanistersStore,
Readable<ActionableProposalCountData>,
],
Universe[]
>(
[universesStore, pageStore, icrcCanistersStore],
([universes, page, icrcCanisters]: [
[universesStore, pageStore, icrcCanistersStore, actionableProposalCountStore],
([universes, page, icrcCanisters, actionableProposalCounts]: [
Universe[],
Page,
IcrcCanistersStoreData,
ActionableProposalCountData,
]) =>
// Non-governance paths show all universes
// The rest show all universes except for ckBTC, and ICRC Tokens
universes.filter(
({ canisterId }) =>
isAllTokensPath(page) ||
(!isUniverseCkBTC(canisterId) && isNullish(icrcCanisters[canisterId]))
)
universes
.filter(
({ canisterId }) =>
isAllTokensPath(page) ||
(!isUniverseCkBTC(canisterId) && isNullish(icrcCanisters[canisterId]))
)
.map((universe) => ({
universe,
actionableProposalCount:
actionableProposalCounts[universe.canisterId] ?? 0,
}))
.sort(
mergeComparators([
compareNnsFirst,
compareActionableProposalCount,
compareTitle,
])
)
.map(({ universe }) => universe)
);
148 changes: 103 additions & 45 deletions frontend/src/tests/lib/derived/selectable-universes.derived.spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import { OWN_CANISTER_ID } from "$lib/constants/canister-ids.constants";
import { CKBTC_UNIVERSE_CANISTER_ID } from "$lib/constants/ckbtc-canister-ids.constants";
import { CKETH_UNIVERSE_CANISTER_ID } from "$lib/constants/cketh-canister-ids.constants";
import { AppPath } from "$lib/constants/routes.constants";
import { selectableUniversesStore } from "$lib/derived/selectable-universes.derived";
import { snsProjectsCommittedStore } from "$lib/derived/sns/sns-projects.derived";
import { actionableSnsProposalsStore } from "$lib/stores/actionable-sns-proposals.store";
import { page } from "$mocks/$app/stores";
import {
mockProjectSubscribe,
mockSnsFullProject,
} from "$tests/mocks/sns-projects.mock";
import { principal } from "$tests/mocks/sns-projects.mock";
import { mockSnsProposal } from "$tests/mocks/sns-proposals.mock";
import {
resetCkETHCanisters,
setCkETHCanisters,
} from "$tests/utils/cketh.test-utils";
import { setSnsProjects } from "$tests/utils/sns.test-utils";
import { get } from "svelte/store";

describe("selectable universes derived stores", () => {
Expand All @@ -27,64 +24,125 @@ describe("selectable universes derived stores", () => {

it("should return Nns, ckBTC and ckTESTBTC (flag for test is true) per default", () => {
const store = get(selectableUniversesStore);
expect(store.length).toEqual(3);
expect(store[0].summary).toBeUndefined();
expect(store[0].canisterId).toEqual(OWN_CANISTER_ID.toText());
expect(store[1].summary).toBeUndefined();
expect(store[1].canisterId).toEqual(CKBTC_UNIVERSE_CANISTER_ID.toText());
expect(store.map(({ title }) => title)).toEqual([
"Internet Computer",
"ckBTC",
"ckTESTBTC",
]);
});

it("should return CkETH in Accounts page", () => {
setCkETHCanisters();
const store = get(selectableUniversesStore);
expect(store.length).toEqual(4);
expect(store[3].summary).toBeUndefined();
expect(store[3].canisterId).toEqual(CKETH_UNIVERSE_CANISTER_ID.toText());
expect(store.map(({ title }) => title)).toEqual([
"Internet Computer",
"ckBTC",
"ckETH",
"ckTESTBTC",
]);
});

it("should not return ckBTC if path is not Account", () => {
it("should not return ckBTC or ckETH if path is not Account", () => {
page.mock({
routeId: AppPath.Neurons,
data: { universe: OWN_CANISTER_ID.toText() },
});

const store = get(selectableUniversesStore);
// 1 length = only NNS
expect(store.length).toEqual(1);
expect(store[0].canisterId).not.toEqual(
CKBTC_UNIVERSE_CANISTER_ID.toText()
);
expect(store.map(({ title }) => title)).toEqual(["Internet Computer"]);
});

it("should not return ckETH if path is not Account", () => {
page.mock({
routeId: AppPath.Neurons,
data: { universe: OWN_CANISTER_ID.toText() },
describe("with projects", () => {
it("should return Nns, ckBTC, ckTESTBTC (flag for test is true) and another project", () => {
setSnsProjects([
{
projectName: "SNS Project",
},
]);
const store = get(selectableUniversesStore);
expect(store.length).toEqual(4);
expect(store.map(({ title }) => title)).toEqual([
"Internet Computer",
"ckBTC",
"ckTESTBTC",
"SNS Project",
]);
});
setCkETHCanisters();

const store = get(selectableUniversesStore);
// 1 length = only NNS
expect(store.length).toEqual(1);
expect(store[0].canisterId).not.toEqual(
CKBTC_UNIVERSE_CANISTER_ID.toText()
);
});
it("should return NNS followed by SNS projects ordered alphabetically", () => {
page.mock({
routeId: AppPath.Proposals,
data: { universe: OWN_CANISTER_ID.toText() },
});

describe("with projects", () => {
beforeEach(() => {
vi.spyOn(snsProjectsCommittedStore, "subscribe").mockImplementation(
mockProjectSubscribe([mockSnsFullProject])
);
const rootCanisterId1 = principal(1);
const rootCanisterId2 = principal(2);
const rootCanisterId3 = principal(3);

setSnsProjects([
{
rootCanisterId: rootCanisterId1,
projectName: "Bravo",
},
{
rootCanisterId: rootCanisterId2,
projectName: "Alfa",
},
{
rootCanisterId: rootCanisterId3,
projectName: "Charlie",
},
]);

expect(get(selectableUniversesStore).map(({ title }) => title)).toEqual([
"Internet Computer",
"Alfa",
"Bravo",
"Charlie",
]);
});

it("should return Nns, ckBTC, ckTESTBTC (flag for test is true) and another project", () => {
const store = get(selectableUniversesStore);
expect(store.length).toEqual(4);
expect(store[3].summary).not.toBeUndefined();
expect(store[3].canisterId).toEqual(
mockSnsFullProject.rootCanisterId.toText()
);
it("should return NNS followed by SNS projects in reverse order of actionable proposals", () => {
page.mock({
routeId: AppPath.Proposals,
data: { universe: OWN_CANISTER_ID.toText() },
});

const rootCanisterIdA = principal(1);
const rootCanisterIdB = principal(2);
const rootCanisterIdC = principal(3);

setSnsProjects([
{
rootCanisterId: rootCanisterIdB,
projectName: "Bravo",
},
{
rootCanisterId: rootCanisterIdA,
projectName: "Alfa",
},
{
rootCanisterId: rootCanisterIdC,
projectName: "Charlie",
},
]);

actionableSnsProposalsStore.set({
rootCanisterId: rootCanisterIdC,
proposals: [mockSnsProposal, mockSnsProposal],
});

actionableSnsProposalsStore.set({
rootCanisterId: rootCanisterIdB,
proposals: [mockSnsProposal],
});

expect(get(selectableUniversesStore).map(({ title }) => title)).toEqual([
"Internet Computer",
"Charlie",
"Bravo",
"Alfa",
]);
});
});
});
16 changes: 9 additions & 7 deletions frontend/src/tests/lib/pages/ActionableProposals.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ describe("ActionableProposals", () => {
await po.getActionableSnses().getActionableSnsProposalsPos()
).toHaveLength(0);

// NOTE: Projects are sorted by reverse order of number of actionable
// proposals.
actionableSnsProposalsStore.set({
rootCanisterId: principal0,
proposals: [snsProposal0],
Expand All @@ -155,20 +157,20 @@ describe("ActionableProposals", () => {
await snsProposalsPos[0]
.getUniverseWithActionableProposalsPo()
.getTitle()
).toEqual("Sns Project 0");
).toEqual("Sns Project 1");
const proposalCardPos0 = await snsProposalsPos[0].getProposalCardPos();
expect(proposalCardPos0.length).toEqual(1);
expect(await proposalCardPos0[0].getProposalId()).toEqual("ID: 11");
expect(proposalCardPos0.length).toEqual(2);
expect(await proposalCardPos0[0].getProposalId()).toEqual("ID: 22");
expect(await proposalCardPos0[1].getProposalId()).toEqual("ID: 33");

expect(
await snsProposalsPos[1]
.getUniverseWithActionableProposalsPo()
.getTitle()
).toEqual("Sns Project 1");
).toEqual("Sns Project 0");
const proposalCardPos1 = await snsProposalsPos[1].getProposalCardPos();
expect(proposalCardPos1.length).toEqual(2);
expect(await proposalCardPos1[0].getProposalId()).toEqual("ID: 22");
expect(await proposalCardPos1[1].getProposalId()).toEqual("ID: 33");
expect(proposalCardPos1.length).toEqual(1);
expect(await proposalCardPos1[0].getProposalId()).toEqual("ID: 11");
});

it("should have actionable query parameter in card href", async () => {
Expand Down

0 comments on commit e038cc7

Please sign in to comment.