Skip to content

Commit

Permalink
GIX-1352: Init accounts service (#1969)
Browse files Browse the repository at this point in the history
# Motivation

Do not show errors from loading accounts on init.

# Changes

* Add a parameter to `syncAccounts` which is an error handler.
* Extract the current logic on error to a default error handler for
`syncAccounts`.
* New `initAccounts` service that uses `syncAccounts` with an empty
function as error handler.

# Tests

* Test new functionality of `syncAccounts`.
* Test for new service `initAcconts`.
  • Loading branch information
lmuntaner authored Feb 28, 2023
1 parent c97b02d commit e2748a6
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 34 deletions.
61 changes: 46 additions & 15 deletions frontend/src/lib/services/accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,34 +106,65 @@ export const loadAccounts = async ({
};
};

type SyncAccontsErrorHandler = (params: {
err: unknown;
certified: boolean;
}) => void;

/**
* Default error handler for syncAccounts.
*
* Ignores non-certified errors.
* Resets accountsStore and shows toast for certified errors.
*/
const defaultErrorHandlerAccounts: SyncAccontsErrorHandler = ({
err,
certified,
}: {
err: unknown;
certified: boolean;
}) => {
if (!certified) {
return;
}

accountsStore.reset();

toastsError(
toToastError({
err,
fallbackErrorLabelKey: "error.accounts_not_found",
})
);
};

/**
* - sync: load the account data using the ledger and the nns dapp canister itself
* Loads the account data using the ledger and the nns dapp canister.
*/
export const syncAccounts = (): Promise<void> => {
export const syncAccounts = (
errorHandler: SyncAccontsErrorHandler = defaultErrorHandlerAccounts
): Promise<void> => {
return queryAndUpdate<AccountsStoreData, unknown>({
request: (options) => loadAccounts(options),
onLoad: ({ response: accounts }) => accountsStore.set(accounts),
onError: ({ error: err, certified }) => {
console.error(err);

if (certified !== true) {
return;
}

// Explicitly handle only UPDATE errors
accountsStore.reset();

toastsError(
toToastError({
err,
fallbackErrorLabelKey: "error.accounts_not_found",
})
);
errorHandler({ err, certified });
},
logMessage: "Syncing Accounts",
});
};

const ignoreErrors: SyncAccontsErrorHandler = () => undefined;

/**
* This function is called on app load to sync the accounts.
*
* It ignores errors and does not show any toasts. Accounts will be synced again.
*/
export const initAccounts = () => syncAccounts(ignoreErrors);

export const addSubAccount = async ({
name,
}: {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/services/app.services.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { syncAccounts } from "./accounts.services";
import { initAccounts } from "./accounts.services";

export const initAppPrivateData = (): Promise<
[PromiseSettledResult<void[]>]
> => {
const initNns: Promise<void>[] = [syncAccounts()];
const initNns: Promise<void>[] = [initAccounts()];

/**
* If Nns load but Sns load fails it is "fine" to go on because Nns are core features.
Expand Down
136 changes: 136 additions & 0 deletions frontend/src/tests/lib/services/accounts.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getAccountIdentityByPrincipal,
getAccountTransactions,
getOrCreateAccount,
initAccounts,
loadAccounts,
renameSubAccount,
syncAccounts,
Expand All @@ -22,6 +23,7 @@ import {
import { accountsStore } from "$lib/stores/accounts.store";
import * as toastsFunctions from "$lib/stores/toasts.store";
import type { NewTransaction } from "$lib/types/transaction";
import { toastsStore } from "@dfinity/gix-components";
import { ICPToken, TokenAmount } from "@dfinity/nns";
import { get } from "svelte/store";
import {
Expand Down Expand Up @@ -168,6 +170,140 @@ describe("accounts-services", () => {
});
});

describe("initAccounts", () => {
beforeEach(() => {
toastsStore.reset();
});

it("should sync accounts", async () => {
const mainBalanceE8s = BigInt(10_000_000);
const queryAccountBalanceSpy = jest
.spyOn(ledgerApi, "queryAccountBalance")
.mockResolvedValue(mainBalanceE8s);
const queryAccountSpy = jest
.spyOn(nnsdappApi, "queryAccount")
.mockResolvedValue(mockAccountDetails);
const mockAccounts = {
main: {
...mockMainAccount,
balance: TokenAmount.fromE8s({
amount: mainBalanceE8s,
token: ICPToken,
}),
},
subAccounts: [],
hardwareWallets: [],
certified: true,
};
await initAccounts();

expect(queryAccountSpy).toHaveBeenCalledTimes(2);
expect(queryAccountBalanceSpy).toHaveBeenCalledWith({
identity: mockIdentity,
accountIdentifier: mockAccountDetails.account_identifier,
certified: true,
});
expect(queryAccountBalanceSpy).toHaveBeenCalledWith({
identity: mockIdentity,
accountIdentifier: mockAccountDetails.account_identifier,
certified: false,
});
expect(queryAccountBalanceSpy).toBeCalledTimes(2);

const accounts = get(accountsStore);
expect(accounts).toEqual(mockAccounts);
});

it("should not show toast errors", async () => {
jest.spyOn(ledgerApi, "queryAccountBalance");
jest
.spyOn(nnsdappApi, "queryAccount")
.mockRejectedValue(new Error("test"));

await initAccounts();

const toastsData = get(toastsStore);
expect(toastsData).toEqual([]);
});
});

describe("syncAccounts", () => {
it("should sync accounts", async () => {
const mainBalanceE8s = BigInt(10_000_000);
const queryAccountBalanceSpy = jest
.spyOn(ledgerApi, "queryAccountBalance")
.mockResolvedValue(mainBalanceE8s);
const queryAccountSpy = jest
.spyOn(nnsdappApi, "queryAccount")
.mockResolvedValue(mockAccountDetails);
const mockAccounts = {
main: {
...mockMainAccount,
balance: TokenAmount.fromE8s({
amount: mainBalanceE8s,
token: ICPToken,
}),
},
subAccounts: [],
hardwareWallets: [],
certified: true,
};
await syncAccounts();

expect(queryAccountSpy).toHaveBeenCalledTimes(2);
expect(queryAccountBalanceSpy).toHaveBeenCalledWith({
identity: mockIdentity,
accountIdentifier: mockAccountDetails.account_identifier,
certified: true,
});
expect(queryAccountBalanceSpy).toHaveBeenCalledWith({
identity: mockIdentity,
accountIdentifier: mockAccountDetails.account_identifier,
certified: false,
});
expect(queryAccountBalanceSpy).toBeCalledTimes(2);

const accounts = get(accountsStore);
expect(accounts).toEqual(mockAccounts);
});

it("should show toast on error if no handler is passed", async () => {
const errorTest = "test";
jest.spyOn(ledgerApi, "queryAccountBalance");
jest
.spyOn(nnsdappApi, "queryAccount")
.mockRejectedValue(new Error(errorTest));

await syncAccounts();

expect(get(toastsStore)).toMatchObject([
{
level: "error",
text: `${en.error.accounts_not_found} ${errorTest}`,
},
]);
});

it("should use handler passed", async () => {
const errorTest = new Error("test");
jest.spyOn(ledgerApi, "queryAccountBalance");
jest.spyOn(nnsdappApi, "queryAccount").mockRejectedValue(errorTest);

const handler = jest.fn();
await syncAccounts(handler);

expect(handler).toBeCalledTimes(2);
expect(handler).toBeCalledWith({
err: errorTest,
certified: false,
});
expect(handler).toBeCalledWith({
err: errorTest,
certified: true,
});
});
});

describe("services", () => {
const mainBalanceE8s = BigInt(10_000_000);

Expand Down
42 changes: 25 additions & 17 deletions frontend/src/tests/lib/services/app.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@
*/
import { NNSDappCanister } from "$lib/canisters/nns-dapp/nns-dapp.canister";
import { initAppPrivateData } from "$lib/services/app.services";
import { GovernanceCanister, LedgerCanister } from "@dfinity/nns";
import { toastsStore } from "@dfinity/gix-components";
import { LedgerCanister } from "@dfinity/nns";
import { mock } from "jest-mock-extended";
import { get } from "svelte/store";
import { mockAccountDetails } from "../../mocks/accounts.store.mock";
import { mockNeuron } from "../../mocks/neurons.mock";

describe("app-services", () => {
const mockLedgerCanister = mock<LedgerCanister>();
const mockNNSDappCanister = mock<NNSDappCanister>();
const mockGovernanceCanister = mock<GovernanceCanister>();

beforeEach(() => {
toastsStore.reset();
jest.clearAllMocks();
jest
.spyOn(LedgerCanister, "create")
.mockImplementation((): LedgerCanister => mockLedgerCanister);
Expand All @@ -22,24 +24,12 @@ describe("app-services", () => {
.spyOn(NNSDappCanister, "create")
.mockImplementation((): NNSDappCanister => mockNNSDappCanister);

jest
.spyOn(GovernanceCanister, "create")
.mockImplementation((): GovernanceCanister => mockGovernanceCanister);

mockCanisters();
jest.spyOn(console, "error").mockImplementation(() => undefined);
});

const mockCanisters = () => {
it("should init Nns", async () => {
mockNNSDappCanister.getAccount.mockResolvedValue(mockAccountDetails);
mockLedgerCanister.accountBalance.mockResolvedValue(BigInt(100_000_000));
mockGovernanceCanister.listNeurons.mockResolvedValue([mockNeuron]);
};

afterEach(() => {
jest.clearAllMocks();
});

it("should init Nns", async () => {
await initAppPrivateData();

// query + update calls
Expand All @@ -53,4 +43,22 @@ describe("app-services", () => {
numberOfCalls
);
});

it("should not show errors if loading accounts fails", async () => {
mockNNSDappCanister.getAccount.mockRejectedValue(new Error("test"));
mockLedgerCanister.accountBalance.mockResolvedValue(BigInt(100_000_000));
await initAppPrivateData();

// query + update calls
const numberOfCalls = 2;

await expect(mockNNSDappCanister.getAccount).toHaveBeenCalledTimes(
numberOfCalls
);

await expect(mockLedgerCanister.accountBalance).not.toBeCalled();

const toastData = get(toastsStore);
expect(toastData).toHaveLength(0);
});
});

0 comments on commit e2748a6

Please sign in to comment.