Skip to content

Commit

Permalink
fix: Checks for empty transactions to define getTransactions as compl…
Browse files Browse the repository at this point in the history
…eted (#5874)

# Motivation

The `.some` function returns `false` when the transactions array is
empty because it cannot find `null` (`oldestTxId`). This is an error, as
it should handle this case.

You can reproduce the issue by following these steps:
- Navigate to the transactions page of a SubAccount with no
transactions.
- The `InfiniteScroll` will never be disabled as expected
[here](https://github.com/dfinity/nns-dapp/blob/74767085476d849d58943233ea36da723aedf96a/frontend/src/lib/components/accounts/UiTransactionsList.svelte#L22).
- However, since we check the size of transactions
[here](https://github.com/dfinity/nns-dapp/blob/74767085476d849d58943233ea36da723aedf96a/frontend/src/lib/components/accounts/UiTransactionsList.svelte#L16),
the bug does not have an impact.

# Changes

- Added a check to `oldestTxId` to determine whether `getTransactions`
is complete.

# Tests

- Added new test to catch the empty flow.

# Todos

- [ ] Add entry to changelog (if necessary).
Not necessary
  • Loading branch information
yhabib authored Nov 28, 2024
1 parent 558bd78 commit 6fbfbe4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
8 changes: 6 additions & 2 deletions frontend/src/lib/services/icp-transactions.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { icpTransactionsStore } from "$lib/stores/icp-transactions.store";
import { toastsError } from "$lib/stores/toasts.store";
import { toToastError } from "$lib/utils/error.utils";
import { sortTransactionsByIdDescendingOrder } from "$lib/utils/icp-transactions.utils";
import { nonNullish } from "@dfinity/utils";
import { isNullish, nonNullish } from "@dfinity/utils";
import { get } from "svelte/store";
import { getCurrentIdentity } from "./auth.services";

Expand All @@ -27,7 +27,11 @@ export const loadIcpAccountTransactions = async ({
start,
});

const completed = transactions.some(({ id }) => id === oldestTxId);
// We consider it complete if we find the oldestTxId in the list of transactions or if oldestTxId is null.
// The latter condition is necessary if the list of transactions is empty, which would otherwise return false.
const completed =
isNullish(oldestTxId) || transactions.some(({ id }) => id === oldestTxId);

icpTransactionsStore.addTransactions({
accountIdentifier,
transactions,
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/tests/lib/services/icp-transactions.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,24 @@ describe("icp-transactions services", () => {
expect(get(icpTransactionsStore)[accountIdentifier].completed).toBe(true);
});

it("sets complete to true when empty array", async () => {
const oldestTxId = null;
const transactions = [];

vi.spyOn(indexApi, "getTransactions").mockResolvedValue({
oldestTxId,
transactions: transactions,
balance: 200_000_000n,
});

await loadIcpAccountTransactions({
accountIdentifier,
start: undefined,
});

expect(get(icpTransactionsStore)[accountIdentifier].completed).toBe(true);
});

it("toasts error if api fails", async () => {
vi.spyOn(indexApi, "getTransactions").mockRejectedValue(
new Error("Something happened")
Expand Down

0 comments on commit 6fbfbe4

Please sign in to comment.