Skip to content

Commit

Permalink
Fetch actionable with pagination (#4655)
Browse files Browse the repository at this point in the history
# Motivation

Use multiple requests to get as much proposals with the status
`ACCEPT_VOTES` as possible because there is limitation on the backend
side, how many proposals can be requested with a single request. New
approach uses maximum 5 requests (with the limit 100 for Nns and 20 for
Snses each).

# Changes

- new const `MAX_ACTIONABLE_REQUEST_COUNT` = 5
- extend `loadActionableProposals` to make multiple calls
- extend `loadActionableSnsProposals` to make multiple calls
- added an error when page limit reached to have a feedback when this
happens.

# Tests

- `loadActionableProposals` makes multiple calls
- `loadActionableSnsProposals` makes multiple calls
- manually with changing the page size

# Todos

- [ ] Add entry to changelog (if necessary).
Not yet.

# Screenshots

<img width="611" alt="image"
src="https://github.com/dfinity/nns-dapp/assets/98811342/461f17b2-e69f-4be4-af70-5e55298ed6df">
  • Loading branch information
mstrasinskis authored Mar 26, 2024
1 parent eb37b66 commit d1f4d99
Show file tree
Hide file tree
Showing 5 changed files with 287 additions and 34 deletions.
1 change: 1 addition & 0 deletions frontend/src/lib/constants/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export const DEFAULT_LIST_PAGINATION_LIMIT = 100;
export const DEFAULT_TRANSACTION_PAGE_LIMIT = 100;
export const MAX_ACTIONABLE_REQUEST_COUNT = 5;
// Use a different limit for Icrc transactions
// the Index canister needs to query the Icrc Ledger canister for each transaction - i.e. it needs an update call
export const DEFAULT_INDEX_TRANSACTION_PAGE_LIMIT = 20;
Expand Down
45 changes: 37 additions & 8 deletions frontend/src/lib/services/actionable-proposals.services.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { queryProposals as queryNnsProposals } from "$lib/api/proposals.api";
import {
DEFAULT_LIST_PAGINATION_LIMIT,
MAX_ACTIONABLE_REQUEST_COUNT,
} from "$lib/constants/constants";
import { getCurrentIdentity } from "$lib/services/auth.services";
import { listNeurons } from "$lib/services/neurons.services";
import { actionableNnsProposalsStore } from "$lib/stores/actionable-nns-proposals.store";
import { definedNeuronsStore, neuronsStore } from "$lib/stores/neurons.store";
import type { ProposalsFiltersStore } from "$lib/stores/proposals.store";
import { lastProposalId } from "$lib/utils/proposals.utils";
import type { ProposalInfo } from "@dfinity/nns";
import {
ProposalRewardStatus,
Expand All @@ -29,7 +34,6 @@ export const loadActionableProposals = async (): Promise<void> => {
return;
}

// Load max 100 proposals (DEFAULT_LIST_PAGINATION_LIMIT) solve when more than 100 proposals with UI (display 99 cards + some CTA).
const proposals = await queryProposals();
// Filter proposals that have at least one votable neuron
const votableProposals = proposals.filter(
Expand All @@ -48,7 +52,8 @@ const queryNeurons = async (): Promise<NeuronInfo[]> => {
return get(definedNeuronsStore);
};

const queryProposals = (): Promise<ProposalInfo[]> => {
/// Fetch all (500 max) proposals that are accepting votes.
const queryProposals = async (): Promise<ProposalInfo[]> => {
const identity = getCurrentIdentity();
const filters: ProposalsFiltersStore = {
// We just want to fetch proposals that are accepting votes, so we don't need to filter by rest of the filters.
Expand All @@ -58,10 +63,34 @@ const queryProposals = (): Promise<ProposalInfo[]> => {
excludeVotedProposals: false,
lastAppliedFilter: undefined,
};
return queryNnsProposals({
beforeProposal: undefined,
identity,
filters,
certified: false,
});

let sortedProposals: ProposalInfo[] = [];
for (
let pagesLoaded = 0;
pagesLoaded < MAX_ACTIONABLE_REQUEST_COUNT;
pagesLoaded++
) {
// Fetch all proposals that are accepting votes.
const page = await queryNnsProposals({
beforeProposal: lastProposalId(sortedProposals),
identity,
filters,
certified: false,
});
// Sort proposals by id in descending order to be sure that "lastProposalId" returns correct id.
sortedProposals = [...sortedProposals, ...page].sort(
({ id: proposalIdA }, { id: proposalIdB }) =>
Number((proposalIdB ?? 0n) - (proposalIdA ?? 0n))
);

if (page.length !== DEFAULT_LIST_PAGINATION_LIMIT) {
break;
}

if (pagesLoaded === MAX_ACTIONABLE_REQUEST_COUNT - 1) {
console.error("Max actionable pages loaded");
}
}

return sortedProposals;
};
71 changes: 54 additions & 17 deletions frontend/src/lib/services/actionable-sns-proposals.services.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { queryProposals, querySnsNeurons } from "$lib/api/sns-governance.api";
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 { snsNeuronsStore } from "$lib/stores/sns-neurons.store";
import { votableSnsNeurons } from "$lib/utils/sns-neuron.utils";
import {
lastProposalId,
sortSnsProposalsById,
} from "$lib/utils/sns-proposals.utils";
import type { Identity } from "@dfinity/agent";
import { Principal } from "@dfinity/principal";
import type { SnsListProposalsResponse, SnsNeuron } from "@dfinity/sns";
import type { SnsNeuron } from "@dfinity/sns";
import { SnsProposalRewardStatus } from "@dfinity/sns";
import type { ProposalData } from "@dfinity/sns/dist/candid/sns_governance";
import { fromNullable, nonNullish } from "@dfinity/utils";
import { get } from "svelte/store";

Expand Down Expand Up @@ -37,15 +43,12 @@ const loadActionableProposalsForSns = async (
}

const identity = await getAuthenticatedIdentity();
const { proposals: allProposals, include_ballots_by_caller } =
const { proposals: allProposals, includeBallotsByCaller } =
await querySnsProposals({
rootCanisterId: rootCanisterIdText,
identity,
});

const includeBallotsByCaller =
fromNullable(include_ballots_by_caller) ?? false;

if (!includeBallotsByCaller) {
// No need to fetch neurons if there are no actionable proposals support.
actionableSnsProposalsStore.set({
Expand Down Expand Up @@ -113,16 +116,50 @@ const querySnsProposals = async ({
}: {
rootCanisterId: string;
identity: Identity;
}): Promise<SnsListProposalsResponse> => {
return queryProposals({
params: {
limit: DEFAULT_SNS_PROPOSALS_PAGE_SIZE,
includeRewardStatus: [
SnsProposalRewardStatus.PROPOSAL_REWARD_STATUS_ACCEPT_VOTES,
],
},
identity,
certified: false,
rootCanisterId: Principal.fromText(rootCanisterId),
});
}): Promise<{ proposals: ProposalData[]; includeBallotsByCaller: boolean }> => {
let sortedProposals: ProposalData[] = [];
let includeBallotsByCaller = false;
for (
let pagesLoaded = 0;
pagesLoaded < MAX_ACTIONABLE_REQUEST_COUNT;
pagesLoaded++
) {
// Fetch all proposals that are accepting votes.
const { proposals: page, include_ballots_by_caller } = await queryProposals(
{
params: {
limit: DEFAULT_SNS_PROPOSALS_PAGE_SIZE,
includeRewardStatus: [
SnsProposalRewardStatus.PROPOSAL_REWARD_STATUS_ACCEPT_VOTES,
],
beforeProposal: lastProposalId(sortedProposals),
},
identity,
certified: false,
rootCanisterId: Principal.fromText(rootCanisterId),
}
);

// Sort proposals by id in descending order to be sure that "lastProposalId" returns correct id.
sortedProposals = sortSnsProposalsById([
...sortedProposals,
...page,
]) as ProposalData[];
// Canisters w/o ballot support, returns undefined for `include_ballots_by_caller`.
includeBallotsByCaller = fromNullable(include_ballots_by_caller) ?? false;

// no more proposals available
if (page.length !== DEFAULT_SNS_PROPOSALS_PAGE_SIZE) {
break;
}

if (pagesLoaded === MAX_ACTIONABLE_REQUEST_COUNT - 1) {
console.error("Max actionable sns pages loaded");
}
}

return {
proposals: sortedProposals,
includeBallotsByCaller,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,33 @@ import { loadActionableProposals } from "$lib/services/actionable-proposals.serv
import { actionableNnsProposalsStore } from "$lib/stores/actionable-nns-proposals.store";
import { authStore } from "$lib/stores/auth.store";
import { neuronsStore } from "$lib/stores/neurons.store";
import { mockAuthStoreSubscribe } from "$tests/mocks/auth.store.mock";
import {
mockAuthStoreSubscribe,
mockIdentity,
} from "$tests/mocks/auth.store.mock";
import { mockNeuron } from "$tests/mocks/neurons.mock";
import { mockProposalInfo } from "$tests/mocks/proposal.mock";
import { runResolvedPromises } from "$tests/utils/timers.test-utils";
import { silentConsoleErrors } from "$tests/utils/utils.test-utils";
import type { NeuronInfo, ProposalInfo } from "@dfinity/nns";
import { ProposalRewardStatus, Vote } from "@dfinity/nns";
import { get } from "svelte/store";

describe("actionable-proposals.services", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.restoreAllMocks();
});

describe("updateActionableProposals", () => {
const votedProposalId = 1234n;
const neuronId = 0n;
const neuron1: NeuronInfo = {
...mockNeuron,
neuronId,
recentBallots: [
{
vote: Vote.Yes,
proposalId: 1n,
proposalId: votedProposalId,
},
],
};
Expand All @@ -35,10 +40,17 @@ describe("actionable-proposals.services", () => {
};
const votedProposal: ProposalInfo = {
...mockProposalInfo,
id: 1n,
id: votedProposalId,
};
const fiveHundredsProposal = Array.from(Array(500))
.map((_, index) => ({
...mockProposalInfo,
id: BigInt(index),
}))
.reverse();
let spyQueryProposals;
let spyQueryNeurons;
let spyConsoleError;

beforeEach(() => {
vi.clearAllMocks();
Expand Down Expand Up @@ -103,6 +115,78 @@ describe("actionable-proposals.services", () => {
);
});

it("should query list proposals using multiple calls", async () => {
const firstResponseProposals = fiveHundredsProposal.slice(0, 100);
const secondResponseProposals = [fiveHundredsProposal[100]];

spyQueryProposals = vi
.spyOn(api, "queryProposals")
.mockResolvedValueOnce(firstResponseProposals)
.mockResolvedValueOnce(secondResponseProposals);
expect(spyQueryProposals).not.toHaveBeenCalled();

await loadActionableProposals();

expect(spyQueryProposals).toHaveBeenCalledTimes(2);
expect(spyQueryProposals).toHaveBeenCalledWith({
identity: mockIdentity,
beforeProposal: undefined,
certified: false,
filters: {
excludeVotedProposals: false,
lastAppliedFilter: undefined,
rewards: [ProposalRewardStatus.AcceptVotes],
status: [],
topics: [],
},
});
expect(spyQueryProposals).toHaveBeenCalledWith({
identity: mockIdentity,
beforeProposal:
firstResponseProposals[firstResponseProposals.length - 1].id,
certified: false,
filters: {
excludeVotedProposals: false,
lastAppliedFilter: undefined,
rewards: [ProposalRewardStatus.AcceptVotes],
status: [],
topics: [],
},
});
expect(get(actionableNnsProposalsStore)?.proposals?.length).toEqual(101);
expect(get(actionableNnsProposalsStore)?.proposals).toEqual([
...firstResponseProposals,
...secondResponseProposals,
]);
});

it("should log an error when request count limit reached", async () => {
spyQueryProposals = vi
.spyOn(api, "queryProposals")
.mockResolvedValueOnce(fiveHundredsProposal.slice(0, 100))
.mockResolvedValueOnce(fiveHundredsProposal.slice(100, 200))
.mockResolvedValueOnce(fiveHundredsProposal.slice(200, 300))
.mockResolvedValueOnce(fiveHundredsProposal.slice(300, 400))
.mockResolvedValueOnce(fiveHundredsProposal.slice(400, 500));
spyConsoleError = silentConsoleErrors();
expect(spyQueryProposals).not.toHaveBeenCalled();
expect(spyConsoleError).not.toHaveBeenCalled();

await loadActionableProposals();

expect(spyQueryProposals).toHaveBeenCalledTimes(5);
// expect an error message
expect(spyConsoleError).toHaveBeenCalledTimes(1);
expect(spyConsoleError).toHaveBeenCalledWith(
"Max actionable pages loaded"
);

expect(get(actionableNnsProposalsStore)?.proposals?.length).toEqual(500);
expect(get(actionableNnsProposalsStore)?.proposals).toEqual(
fiveHundredsProposal
);
});

it("should update actionable nns proposals store with votable proposals only", async () => {
expect(get(actionableNnsProposalsStore)).toEqual({
proposals: undefined,
Expand Down
Loading

0 comments on commit d1f4d99

Please sign in to comment.