From 8975afc5db8d30a682571cb1618f314b5165aab9 Mon Sep 17 00:00:00 2001 From: David de Kloet <122978264+dskloetd@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:24:55 +0100 Subject: [PATCH] Keep universe in goBack in IcrcWalletPage (#4122) # 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). --- CHANGELOG-Nns-Dapp-unreleased.md | 1 + .../components/accounts/IcrcWalletPage.svelte | 9 +++- .../src/tests/lib/pages/IcrcWallet.spec.ts | 51 ++++++++++++++++--- .../src/tests/lib/pages/WalletTest.svelte | 15 ++++++ 4 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 frontend/src/tests/lib/pages/WalletTest.svelte diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index 9a37cd65b4f..2b95531075a 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -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 diff --git a/frontend/src/lib/components/accounts/IcrcWalletPage.svelte b/frontend/src/lib/components/accounts/IcrcWalletPage.svelte index ec522a361e6..54e1cf2767f 100644 --- a/frontend/src/lib/components/accounts/IcrcWalletPage.svelte +++ b/frontend/src/lib/components/accounts/IcrcWalletPage.svelte @@ -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"; @@ -31,7 +31,12 @@ const reloadOnlyAccountFromStore = () => setSelectedAccount(); - const goBack = (): Promise => goto(AppPath.Accounts); + const goBack = async (): Promise => + goto( + buildAccountsUrl({ + universe: $selectedUniverseStore.canisterId, + }) + ); // e.g. is called from "Receive" modal after user click "Done" export const reloadAccount = async () => { diff --git a/frontend/src/tests/lib/pages/IcrcWallet.spec.ts b/frontend/src/tests/lib/pages/IcrcWallet.spec.ts index 40be27c2b91..b97417a950d 100644 --- a/frontend/src/tests/lib/pages/IcrcWallet.spec.ts +++ b/frontend/src/tests/lib/pages/IcrcWallet.spec.ts @@ -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, @@ -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"; @@ -81,7 +83,10 @@ describe("IcrcWallet", () => { const renderWallet = async (props: { accountIdentifier: string; }): Promise => { - const { container } = render(IcrcWallet, props); + const { container } = render(WalletTest, { + ...props, + testComponent: IcrcWallet, + }); await runResolvedPromises(); return IcrcWalletPo.under(new JestPageObjectElement(container)); }; @@ -105,6 +110,7 @@ describe("IcrcWallet", () => { vi.clearAllTimers(); tokensStore.reset(); overrideFeatureFlagsStore.reset(); + toastsStore.reset(); resetIdentity(); vi.mocked(icrcIndexApi.getTransactions).mockResolvedValue({ @@ -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([]); }); }); }); diff --git a/frontend/src/tests/lib/pages/WalletTest.svelte b/frontend/src/tests/lib/pages/WalletTest.svelte new file mode 100644 index 00000000000..bd4f8d59c78 --- /dev/null +++ b/frontend/src/tests/lib/pages/WalletTest.svelte @@ -0,0 +1,15 @@ + + +{#if currentAppPath === AppPath.Wallet} + +{/if}