Skip to content

Commit

Permalink
Fix showing "no proposals" when not signed in (#5601)
Browse files Browse the repository at this point in the history
# Motivation

I found a bug that the "no proposals" message is not shown when
query+update is enabled (`FORCE_CALL_STRATEGY = undefined`) and the user
is not signed in.

This happened because `nothingFound` was not updated if
`$filteredProposals.certified === false && notForceCallStrategy()`. That
is if the proposals are the result of a query response, while we also
expect an update response.

The first problem is that when the user is not signed in, by default [we
don't follow the query call with an update
call](https://github.com/dfinity/nns-dapp/blob/a86684b1625ddf972d92052e2ba8e3d31feb21ac/frontend/src/lib/services/utils.services.ts#L72-L74).
So the assumption that because `notForceCallStrategy()`, the query call
will be followed by an update call, is not true when the user is not
signed in. This could be fixed by additionally checking if the user is
signed in.

But on closer inspection, the whole `nothingFound` field seems to be
unnecessary. We already show skeletons when we are loading proposals. So
if we are not loading proposals, and we also don't have any proposals,
then of course nothing is found.

This is analogous to the code on line 67 of
`frontend/src/lib/components/proposals/NnsProposalsList.svelte` which
does the same for the actionable proposals case.

# Changes

1. Remove `nothingFound` and the logic to update it, from
`frontend/src/lib/pages/NnsProposals.svelte`.
2. In `frontend/src/lib/components/proposals/NnsProposalsList.svelte`
show "nothing found" if we don't show skeletons and also don't have any
proposals to show.

# Tests

1. Add a unit test that the "nothing found" message is also shown when
the user is not signed in.
2. Existing tests pass.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Oct 10, 2024
1 parent a86684b commit 55fec50
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ proposal is successful, the changes it released will be moved from this file to
* Issue with setting exact dissolve delay on SNS neurons.
* Error on canister page for canisters that are not controlled by the user.
* Don't hide neurons which have only staked maturity.
* Not showing the "no proposals" message when not signed in.

#### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import type { ProposalInfo } from "@dfinity/nns";
import { isNullish } from "@dfinity/utils";
import { fade } from "svelte/transition";
export let nothingFound: boolean;
export let hidden: boolean;
export let disableInfiniteScroll: boolean;
export let loading: boolean;
Expand All @@ -39,7 +38,7 @@
<div in:fade data-tid="all-proposal-list">
{#if loadingAnimation === "skeleton"}
<LoadingProposals />
{:else if nothingFound}
{:else if $filteredActionableProposals.proposals.length === 0}
<NoProposals />
{:else}
<ListLoader loading={loadingAnimation === "spinner"}>
Expand Down
31 changes: 2 additions & 29 deletions frontend/src/lib/pages/NnsProposals.svelte
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
<script lang="ts">
import NnsProposalsList from "$lib/components/proposals/NnsProposalsList.svelte";
import { notForceCallStrategy } from "$lib/constants/mockable.constants";
import { AppPath } from "$lib/constants/routes.constants";
import {
filteredProposals,
sortedProposals,
} from "$lib/derived/proposals.derived";
import { sortedProposals } from "$lib/derived/proposals.derived";
import {
listNextProposals,
listProposals,
Expand All @@ -17,10 +13,7 @@
import { referrerPathStore } from "$lib/stores/routes.store";
import { toastsError } from "$lib/stores/toasts.store";
import { reloadRouteData } from "$lib/utils/navigation.utils";
import {
hasMatchingProposals,
lastProposalId,
} from "$lib/utils/proposals.utils";
import { lastProposalId } from "$lib/utils/proposals.utils";
import { debounce } from "@dfinity/utils";
import { onMount } from "svelte";
Expand Down Expand Up @@ -121,25 +114,6 @@
$: $proposalsFiltersStore, applyFilter();
const updateNothingFound = () => {
// Update the "nothing found" UI information only when the results of the certified query has been received to minimize UI glitches
if ($filteredProposals.certified === false && notForceCallStrategy()) {
if (loading) nothingFound = false;
return;
}
nothingFound =
initialized &&
!loading &&
!hasMatchingProposals({
proposals: $filteredProposals.proposals,
filters: $proposalsFiltersStore,
});
};
let nothingFound: boolean;
$: initialized, loading, $filteredProposals, (() => updateNothingFound())();
let loadingAnimation: "spinner" | "skeleton" | undefined = undefined;
$: loadingAnimation = !loading
? undefined
Expand All @@ -150,7 +124,6 @@

<NnsProposalsList
{hidden}
{nothingFound}
{disableInfiniteScroll}
{loading}
{loadingAnimation}
Expand Down
13 changes: 12 additions & 1 deletion frontend/src/tests/lib/pages/NnsProposals.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import {
proposalsFiltersStore,
proposalsStore,
} from "$lib/stores/proposals.store";
import { mockIdentity, resetIdentity } from "$tests/mocks/auth.store.mock";
import {
mockIdentity,
resetIdentity,
setNoIdentity,
} from "$tests/mocks/auth.store.mock";
import { IntersectionObserverActive } from "$tests/mocks/infinitescroll.mock";
import { mockProposals } from "$tests/mocks/proposals.store.mock";
import { NnsProposalListPo } from "$tests/page-objects/NnsProposalList.page-object";
Expand Down Expand Up @@ -384,6 +388,13 @@ describe("NnsProposals", () => {

expect(await po.getNoProposalsPo().isPresent()).toBe(true);
});

it("should render not found text when not signed in", async () => {
setNoIdentity();
const po = await renderComponent();

expect(await po.getNoProposalsPo().isPresent()).toBe(true);
});
});
});

Expand Down

0 comments on commit 55fec50

Please sign in to comment.