useTransactions hook to simplify loading of transactions (#3685)

* useTransactions hook to load transactions * Release notes + lint fix * Update useQuery * useTransactions update * Stabilize tests * Fx flaky test * Fix tests * Fix tests * Update queries * Apply coderabbit suggestions * Fix onlySync * Update useTransactions * Code rabbit suggestions * useTransactionsSearch hook * Debounce the useTransactionsSearch search method * usePreviewTransactions debounce fix * Fix lint * Coderabbit feedback + make useSchedules consistent with query pattern used in useTransactions * Code review feedback and improve schedules loading * Update error handling * Cancel debounce on unmount * Fix lint * set loading state on error * Fix test * VRT * Revert VRT --------- Co-authored-by: Matt Fiddaman <[email protected]>
actualbudget · Nov 12, 2024 · 3cefd98 · 3cefd98
1 parent fa2830a
commit 3cefd98
Show file tree

Hide file tree

Showing 37 changed files with 1,249 additions and 769 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -161,7 +161,12 @@ module.exports = {
     ],
     'no-with': 'warn',
     'no-whitespace-before-property': 'warn',
-    'react-hooks/exhaustive-deps': 'warn',
+    'react-hooks/exhaustive-deps': [
+      'warn',
+      {
+        additionalHooks: '(useQuery)',
+      },
+    ],
     'require-yield': 'warn',
     'rest-spread-spacing': ['warn', 'never'],
     strict: ['warn', 'never'],

diff --git a/packages/desktop-client/e2e/accounts.mobile.test.js b/packages/desktop-client/e2e/accounts.mobile.test.js
@@ -27,6 +27,7 @@ test.describe('Mobile Accounts', () => {
 
   test('opens the accounts page and asserts on balances', async () => {
     const accountsPage = await navigation.goToAccountsPage();
+    await accountsPage.waitFor();
 
     const account = await accountsPage.getNthAccount(1);
 
@@ -37,7 +38,10 @@ test.describe('Mobile Accounts', () => {
 
   test('opens individual account page and checks that filtering is working', async () => {
     const accountsPage = await navigation.goToAccountsPage();
+    await accountsPage.waitFor();
+
     const accountPage = await accountsPage.openNthAccount(0);
+    await accountPage.waitFor();
 
     await expect(accountPage.heading).toHaveText('Bank of America');
     await expect(accountPage.transactionList).toBeVisible();
@@ -50,6 +54,9 @@ test.describe('Mobile Accounts', () => {
     await expect(accountPage.transactions).toHaveCount(0);
     await expect(page).toMatchThemeScreenshots();
 
+    await accountPage.clearSearch();
+    await expect(accountPage.transactions).not.toHaveCount(0);
+
     await accountPage.searchByText('Kroger');
     await expect(accountPage.transactions).not.toHaveCount(0);
     await expect(page).toMatchThemeScreenshots();

diff --git a/packages/desktop-client/e2e/accounts.test.js b/packages/desktop-client/e2e/accounts.test.js
@@ -62,6 +62,8 @@ test.describe('Accounts', () => {
 
     test('creates a transfer from two existing transactions', async () => {
       accountPage = await navigation.goToAccountPage('For budget');
+      await accountPage.waitFor();
+
       await expect(accountPage.accountName).toHaveText('Budgeted Accounts');
 
       await accountPage.filterByNote('Test Acc Transfer');
@@ -109,6 +111,7 @@ test.describe('Accounts', () => {
         offBudget: false,
         balance: 0,
       });
+      await accountPage.waitFor();
     });
 
     async function importCsv(screenshot = false) {

diff --git a/packages/desktop-client/e2e/page-models/account-page.js b/packages/desktop-client/e2e/page-models/account-page.js
@@ -30,6 +30,10 @@ export class AccountPage {
     this.selectTooltip = this.page.getByTestId('transactions-select-tooltip');
   }
 
+  async waitFor() {
+    await this.transactionTable.waitFor();
+  }
+
   /**
    * Enter details of a transaction
    */

diff --git a/packages/desktop-client/e2e/page-models/mobile-account-page.js b/packages/desktop-client/e2e/page-models/mobile-account-page.js
@@ -15,6 +15,10 @@ export class MobileAccountPage {
     });
   }
 
+  async waitFor() {
+    await this.transactionList.waitFor();
+  }
+
   /**
    * Retrieve the balance of the account as a number
    */
@@ -29,6 +33,10 @@ export class MobileAccountPage {
     await this.searchBox.fill(term);
   }
 
+  async clearSearch() {
+    await this.searchBox.clear();
+  }
+
   /**
    * Go to transaction creation page
    */

diff --git a/packages/desktop-client/e2e/page-models/mobile-accounts-page.js b/packages/desktop-client/e2e/page-models/mobile-accounts-page.js
@@ -4,9 +4,14 @@ export class MobileAccountsPage {
   constructor(page) {
     this.page = page;
 
+    this.accountList = this.page.getByLabel('Account list');
     this.accounts = this.page.getByTestId('account');
   }
 
+  async waitFor() {
+    await this.accountList.waitFor();
+  }
+
   /**
    * Get the name and balance of the nth account
    */

diff --git a/packages/desktop-client/src/components/ManageRules.tsx b/packages/desktop-client/src/components/ManageRules.tsx
@@ -10,6 +10,8 @@ import React, {
 import { useTranslation } from 'react-i18next';
 import { useDispatch } from 'react-redux';
 
+import { useSchedules } from 'loot-core/client/data-hooks/schedules';
+import { q } from 'loot-core/shared/query';
 import { pushModal } from 'loot-core/src/client/actions/modals';
 import { initiallyLoadPayees } from 'loot-core/src/client/actions/queries';
 import { send } from 'loot-core/src/platform/client/fetch';
@@ -22,7 +24,6 @@ import { type NewRuleEntity } from 'loot-core/src/types/models';
 import { useAccounts } from '../hooks/useAccounts';
 import { useCategories } from '../hooks/useCategories';
 import { usePayees } from '../hooks/usePayees';
-import { useSchedules } from '../hooks/useSchedules';
 import { useSelected, SelectedProvider } from '../hooks/useSelected';
 import { theme } from '../style';
 
@@ -114,7 +115,9 @@ export function ManageRules({
   const [filter, setFilter] = useState('');
   const dispatch = useDispatch();
 
-  const { data: schedules = [] } = useSchedules();
+  const { schedules = [] } = useSchedules({
+    query: useMemo(() => q('schedules').select('*'), []),
+  });
   const { list: categories } = useCategories();
   const payees = usePayees();
   const accounts = useAccounts();

diff --git a/packages/desktop-client/src/components/accounts/Account.tsx b/packages/desktop-client/src/components/accounts/Account.tsx
@@ -19,10 +19,14 @@ import { type UndoState } from 'loot-core/server/undo';
 import { useFilters } from 'loot-core/src/client/data-hooks/filters';
 import {
   SchedulesProvider,
-  useDefaultSchedulesQueryTransform,
+  accountSchedulesQuery,
 } from 'loot-core/src/client/data-hooks/schedules';
 import * as queries from 'loot-core/src/client/queries';
-import { runQuery, pagedQuery } from 'loot-core/src/client/query-helpers';
+import {
+  runQuery,
+  pagedQuery,
+  type PagedQuery,
+} from 'loot-core/src/client/query-helpers';
 import { send, listen } from 'loot-core/src/platform/client/fetch';
 import { currentDay } from 'loot-core/src/shared/months';
 import { q, type Query } from 'loot-core/src/shared/query';
@@ -212,7 +216,7 @@ function AllTransactions({
     return balances;
   }, [filtered, prependBalances, balances]);
 
-  if (!prependTransactions) {
+  if (!prependTransactions?.length || filtered) {
     return children(transactions, balances);
   }
   return children(allTransactions, allBalances);
@@ -240,7 +244,7 @@ function getField(field?: string) {
 }
 
 type AccountInternalProps = {
-  accountId?: string;
+  accountId?: AccountEntity['id'] | 'budgeted' | 'offbudget' | 'uncategorized';
   filterConditions: RuleConditionEntity[];
   showBalances?: boolean;
   setShowBalances: (newValue: boolean) => void;
@@ -322,7 +326,7 @@ class AccountInternal extends PureComponent<
   AccountInternalProps,
   AccountInternalState
 > {
-  paged: ReturnType<typeof pagedQuery> | null;
+  paged: PagedQuery<TransactionEntity> | null;
   rootQuery: Query;
   currentQuery: Query;
   table: TableRef;
@@ -457,7 +461,7 @@ class AccountInternal extends PureComponent<
   }
 
   fetchAllIds = async () => {
-    const { data } = await runQuery(this.paged?.getQuery().select('id'));
+    const { data } = await runQuery(this.paged?.query.select('id'));
     // Remember, this is the `grouped` split type so we need to deal
     // with the `subtransactions` property
     return data.reduce((arr: string[], t: TransactionEntity) => {
@@ -472,7 +476,7 @@ class AccountInternal extends PureComponent<
   };
 
   fetchTransactions = (filterConditions?: ConditionEntity[]) => {
-    const query = this.makeRootQuery();
+    const query = this.makeRootTransactionsQuery();
     this.rootQuery = this.currentQuery = query;
     if (filterConditions) this.applyFilters(filterConditions);
     else this.updateQuery(query);
@@ -482,10 +486,10 @@ class AccountInternal extends PureComponent<
     }
   };
 
-  makeRootQuery = () => {
+  makeRootTransactionsQuery = () => {
     const accountId = this.props.accountId;
 
-    return queries.makeTransactionsQuery(accountId);
+    return queries.transactions(accountId);
   };
 
   updateQuery(query: Query, isFiltered: boolean = false) {
@@ -502,12 +506,9 @@ class AccountInternal extends PureComponent<
       query = query.filter({ reconciled: { $eq: false } });
     }
 
-    this.paged = pagedQuery(
-      query.select('*'),
-      async (
-        data: TransactionEntity[],
-        prevData: TransactionEntity[] | null,
-      ) => {
+    this.paged = pagedQuery(query.select('*'), {
+      onData: async (groupedData, prevData) => {
+        const data = ungroupTransactions([...groupedData]);
         const firstLoad = prevData == null;
 
         if (firstLoad) {
@@ -529,7 +530,7 @@ class AccountInternal extends PureComponent<
         this.setState(
           {
             transactions: data,
-            transactionCount: this.paged?.getTotalCount(),
+            transactionCount: this.paged?.totalCount,
             transactionsFiltered: isFiltered,
             loading: false,
             workingHard: false,
@@ -549,12 +550,11 @@ class AccountInternal extends PureComponent<
           },
         );
       },
-      {
+      options: {
         pageCount: 150,
         onlySync: true,
-        mapper: ungroupTransactions,
       },
-    );
+    });
   }
 
   UNSAFE_componentWillReceiveProps(nextProps: AccountInternalProps) {
@@ -590,7 +590,7 @@ class AccountInternal extends PureComponent<
       );
     } else {
       this.updateQuery(
-        queries.makeTransactionSearchQuery(
+        queries.transactionsSearch(
           this.currentQuery,
           this.state.search,
           this.props.dateFormat,
@@ -652,27 +652,19 @@ class AccountInternal extends PureComponent<
     );
   };
 
-  onTransactionsChange = (
-    newTransaction: TransactionEntity,
-    data: TransactionEntity[],
-  ) => {
+  onTransactionsChange = (updatedTransaction: TransactionEntity) => {
     // Apply changes to pagedQuery data
-    this.paged?.optimisticUpdate(
-      (data: TransactionEntity[]) => {
-        if (newTransaction._deleted) {
-          return data.filter(t => t.id !== newTransaction.id);
-        } else {
-          return data.map(t => {
-            return t.id === newTransaction.id ? newTransaction : t;
-          });
-        }
-      },
-      () => {
-        return data;
-      },
-    );
+    this.paged?.optimisticUpdate(data => {
+      if (updatedTransaction._deleted) {
+        return data.filter(t => t.id !== updatedTransaction.id);
+      } else {
+        return data.map(t => {
+          return t.id === updatedTransaction.id ? updatedTransaction : t;
+        });
+      }
+    });
 
-    this.props.updateNewTransactions(newTransaction.id);
+    this.props.updateNewTransactions(updatedTransaction.id);
   };
 
   canCalculateBalance = () => {
@@ -696,8 +688,7 @@ class AccountInternal extends PureComponent<
     }
 
     const { data } = await runQuery(
-      this.paged
-        ?.getQuery()
+      this.paged?.query
         .options({ splits: 'none' })
         .select([{ balance: { $sumOver: '$amount' } }]),
     );
@@ -862,13 +853,13 @@ class AccountInternal extends PureComponent<
   getBalanceQuery(id?: string) {
     return {
       name: `balance-query-${id}`,
-      query: this.makeRootQuery().calculate({ $sum: '$amount' }),
+      query: this.makeRootTransactionsQuery().calculate({ $sum: '$amount' }),
     } as const;
   }
 
   getFilteredAmount = async () => {
     const { data: amount } = await runQuery(
-      this.paged?.getQuery().calculate({ $sum: '$amount' }),
+      this.paged?.query.calculate({ $sum: '$amount' }),
     );
     return amount;
   };
@@ -1896,10 +1887,13 @@ export function Account() {
   const savedFiters = useFilters();
   const actionCreators = useActions();
 
-  const transform = useDefaultSchedulesQueryTransform(params.id);
+  const schedulesQuery = useMemo(
+    () => accountSchedulesQuery(params.id),
+    [params.id],
+  );
 
   return (
-    <SchedulesProvider transform={transform}>
+    <SchedulesProvider query={schedulesQuery}>
       <SplitsExpandedProvider
         initialMode={expandSplits ? 'collapse' : 'expand'}
       >

diff --git a/packages/desktop-client/src/components/accounts/Balance.jsx b/packages/desktop-client/src/components/accounts/Balance.jsx
@@ -68,8 +68,13 @@ function SelectedBalance({ selectedItems, account }) {
   });
 
   let scheduleBalance = null;
-  const scheduleData = useCachedSchedules();
-  const schedules = scheduleData ? scheduleData.schedules : [];
+
+  const { isLoading, schedules = [] } = useCachedSchedules();
+
+  if (isLoading) {
+    return null;
+  }
+
   const previewIds = [...selectedItems]
     .filter(id => isPreviewId(id))
     .map(id => id.slice(8));