Skip to content

Commit

Permalink
Keep universe in goBack in IcrcWalletPage (#4122)
Browse files Browse the repository at this point in the history
# Motivation

In `IcrcWalletPage` (and other wallets) when there is no or an incorrect
account identifier, we navigate back to the accounts page.
In `SnsWallet` it navigates to the accounts page of the same universe
but in `IcrcWalletPage` it reverts to the ICP universe.
Instead it should stay with the universe of the wallet we were on.

In an earlier version of this PR, the test would end up in an infinite
loop because navigating to a different URL would trigger the
`selectedIcrcTokenUniverseIdStore` which would result in
`IcrcWalletPage` trying again to load the data and again trying to go
back to the accounts page etc.
In reality this would not happen, because as soon as the app navigates
away from the wallet page, the component would be unmounted and it
wouldn't try to load data anymore. To make the test more realistic, I
added a wrapper component that only renders the wallet if the page store
shows that we are on the wallet route.

# Changes

1. Include the universe in the URL when navigating from the
`IcrcWalletPage` to the accounts page.
2. Render `IcrcWallet` inside a wrapper component `WalletTest.svelte`
which only renders the component if we're on the wallet route, to make
the test more realistic and avoid an infinite loop.
3. Change the test to use the new test wrapper.
4. Change the test to expect the universe in the URL.
5. Drive-by improvement: Expect error toast messages.

# Tests

Unit tests updated.
Tested manually.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Jan 3, 2024
1 parent 9034f78 commit 8975afc
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ proposal is successful, the changes it released will be moved from this file to
- Min dissolve delay button updates not only for the first time.
- Fix scrollbar in multiline toast message.
- Go back to accounts page for incorrect account identifier in SNS wallet page.
- Stay on the same universe when navigating back from wallet to the accounts page.

#### Security

Expand Down
9 changes: 7 additions & 2 deletions frontend/src/lib/components/accounts/IcrcWalletPage.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import { replacePlaceholders } from "$lib/utils/i18n.utils";
import { i18n } from "$lib/stores/i18n";
import { goto } from "$app/navigation";
import { AppPath } from "$lib/constants/routes.constants";
import { buildAccountsUrl } from "$lib/utils/navigation.utils";
import type { UniverseCanisterId } from "$lib/types/universe";
import { selectedUniverseStore } from "$lib/derived/selected-universe.derived";
import IcrcBalancesObserver from "$lib/components/accounts/IcrcBalancesObserver.svelte";
Expand All @@ -31,7 +31,12 @@
const reloadOnlyAccountFromStore = () => setSelectedAccount();
const goBack = (): Promise<void> => goto(AppPath.Accounts);
const goBack = async (): Promise<void> =>
goto(
buildAccountsUrl({
universe: $selectedUniverseStore.canisterId,
})
);
// e.g. is called from "Receive" modal after user click "Done"
export const reloadAccount = async () => {
Expand Down
51 changes: 44 additions & 7 deletions frontend/src/tests/lib/pages/IcrcWallet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { tokensStore } from "$lib/stores/tokens.store";
import type { Account } from "$lib/types/account";
import { page } from "$mocks/$app/stores";
import AccountsTest from "$tests/lib/pages/AccountsTest.svelte";
import WalletTest from "$tests/lib/pages/WalletTest.svelte";
import { resetIdentity } from "$tests/mocks/auth.store.mock";
import {
mockCkETHMainAccount,
Expand All @@ -25,6 +26,7 @@ import { ReceiveModalPo } from "$tests/page-objects/ReceiveModal.page-object";
import { JestPageObjectElement } from "$tests/page-objects/jest.page-object";
import { blockAllCallsTo } from "$tests/utils/module.test-utils";
import { runResolvedPromises } from "$tests/utils/timers.test-utils";
import { toastsStore } from "@dfinity/gix-components";
import { render } from "@testing-library/svelte";
import { get } from "svelte/store";

Expand Down Expand Up @@ -81,7 +83,10 @@ describe("IcrcWallet", () => {
const renderWallet = async (props: {
accountIdentifier: string;
}): Promise<IcrcWalletPo> => {
const { container } = render(IcrcWallet, props);
const { container } = render(WalletTest, {
...props,
testComponent: IcrcWallet,
});
await runResolvedPromises();
return IcrcWalletPo.under(new JestPageObjectElement(container));
};
Expand All @@ -105,6 +110,7 @@ describe("IcrcWallet", () => {
vi.clearAllTimers();
tokensStore.reset();
overrideFeatureFlagsStore.reset();
toastsStore.reset();
resetIdentity();

vi.mocked(icrcIndexApi.getTransactions).mockResolvedValue({
Expand Down Expand Up @@ -231,27 +237,58 @@ describe("IcrcWallet", () => {
});

it("should nagigate to accounts when account identifier is missing", async () => {
expect(get(pageStore).path).toEqual(AppPath.Wallet);
expect(get(pageStore)).toEqual({
path: AppPath.Wallet,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
await renderWallet({
accountIdentifier: undefined,
});
expect(get(pageStore).path).toEqual(AppPath.Accounts);
expect(get(pageStore)).toEqual({
path: AppPath.Accounts,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
expect(get(toastsStore)).toMatchObject([
{
level: "error",
text: 'Sorry, the account "" was not found',
},
]);
});

it("should nagigate to accounts when account identifier is invalid", async () => {
expect(get(pageStore).path).toEqual(AppPath.Wallet);
expect(get(pageStore)).toEqual({
path: AppPath.Wallet,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
await renderWallet({
accountIdentifier: "invalid-account-identifier",
});
expect(get(pageStore).path).toEqual(AppPath.Accounts);
expect(get(pageStore)).toEqual({
path: AppPath.Accounts,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
expect(get(toastsStore)).toMatchObject([
{
level: "error",
text: 'Sorry, the account "invalid-account-identifier" was not found',
},
]);
});

it("should stay on the wallet page when account identifier is valid", async () => {
expect(get(pageStore).path).toEqual(AppPath.Wallet);
expect(get(pageStore)).toEqual({
path: AppPath.Wallet,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
await renderWallet({
accountIdentifier: mockCkETHMainAccount.identifier,
});
expect(get(pageStore).path).toEqual(AppPath.Wallet);
expect(get(pageStore)).toEqual({
path: AppPath.Wallet,
universe: CKETHSEPOLIA_UNIVERSE_CANISTER_ID.toText(),
});
expect(get(toastsStore)).toEqual([]);
});
});
});
15 changes: 15 additions & 0 deletions frontend/src/tests/lib/pages/WalletTest.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script lang="ts">
import { AppPath } from "$lib/constants/routes.constants";
import { pathForRouteId } from "$lib/utils/page.utils";
import { page } from "$app/stores";
import type { SvelteComponent } from "svelte";
export let testComponent: typeof SvelteComponent;
export let accountIdentifier: string | undefined = undefined;
let currentAppPath: string | undefined = undefined;
$: currentAppPath = pathForRouteId($page.route.id);
</script>

{#if currentAppPath === AppPath.Wallet}
<svelte:component this={testComponent} {accountIdentifier} />
{/if}

0 comments on commit 8975afc

Please sign in to comment.