Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useTransactions hook to simplify loading of transactions #3685

Merged
merged 27 commits into from
Nov 12, 2024
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2f3ce3c
useTransactions hook to load transactions
joel-jeremy Oct 13, 2024
d1a4ed7
Release notes + lint fix
joel-jeremy Oct 13, 2024
8dd5d9a
Update useQuery
joel-jeremy Oct 13, 2024
61079b0
useTransactions update
joel-jeremy Oct 13, 2024
ec6bc84
Stabilize tests
joel-jeremy Oct 14, 2024
9dc569f
Fx flaky test
joel-jeremy Oct 14, 2024
ad4c9fa
Fix tests
joel-jeremy Oct 14, 2024
48bb13e
Fix tests
joel-jeremy Oct 14, 2024
33d04c2
Update queries
joel-jeremy Oct 14, 2024
bc6cb82
Apply coderabbit suggestions
joel-jeremy Oct 15, 2024
392340d
Fix onlySync
joel-jeremy Oct 15, 2024
87dfb93
Update useTransactions
joel-jeremy Oct 17, 2024
e9c7d24
Code rabbit suggestions
joel-jeremy Oct 17, 2024
3f9fec7
useTransactionsSearch hook
joel-jeremy Oct 17, 2024
6b86abb
Debounce the useTransactionsSearch search method
joel-jeremy Oct 17, 2024
825d9f6
usePreviewTransactions debounce fix
joel-jeremy Oct 17, 2024
5777af3
Fix lint
joel-jeremy Oct 17, 2024
24eb0c2
Coderabbit feedback + make useSchedules consistent with query pattern…
joel-jeremy Oct 17, 2024
a0d12ce
Code review feedback and improve schedules loading
joel-jeremy Oct 18, 2024
f82ad1f
Update error handling
joel-jeremy Oct 18, 2024
3ca0bfd
Cancel debounce on unmount
joel-jeremy Oct 18, 2024
4a75b1e
Fix lint
joel-jeremy Oct 19, 2024
9a6ae01
set loading state on error
joel-jeremy Oct 19, 2024
5a042b0
Fix test
joel-jeremy Oct 19, 2024
2d27543
VRT
joel-jeremy Oct 31, 2024
d787213
Revert VRT
joel-jeremy Oct 31, 2024
b398e38
Merge branch 'master' into useTransactions-hook
matt-fidd Nov 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -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)',
},
],
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
'require-yield': 'warn',
'rest-spread-spacing': ['warn', 'never'],
strict: ['warn', 'never'],
7 changes: 7 additions & 0 deletions packages/desktop-client/e2e/accounts.mobile.test.js
Original file line number Diff line number Diff line change
@@ -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();
3 changes: 3 additions & 0 deletions packages/desktop-client/e2e/accounts.test.js
Original file line number Diff line number Diff line change
@@ -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();

joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
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) {
4 changes: 4 additions & 0 deletions packages/desktop-client/e2e/page-models/account-page.js
Original file line number Diff line number Diff line change
@@ -30,6 +30,10 @@ export class AccountPage {
this.selectTooltip = this.page.getByTestId('transactions-select-tooltip');
}

async waitFor() {
await this.transactionTable.waitFor();
}
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved

/**
* Enter details of a transaction
*/
Original file line number Diff line number Diff line change
@@ -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
*/
Original file line number Diff line number Diff line change
@@ -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
*/
7 changes: 5 additions & 2 deletions packages/desktop-client/src/components/ManageRules.tsx
Original file line number Diff line number Diff line change
@@ -9,6 +9,8 @@ import React, {
} from 'react';
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';
@@ -21,7 +23,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';

@@ -113,7 +114,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();
84 changes: 39 additions & 45 deletions packages/desktop-client/src/components/accounts/Account.tsx
Original file line number Diff line number Diff line change
@@ -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'));
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
// 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);
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
};

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,
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
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;
});
}
});
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved

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
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
.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' }),
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved
);
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],
);
joel-jeremy marked this conversation as resolved.
Show resolved Hide resolved

return (
<SchedulesProvider transform={transform}>
<SchedulesProvider query={schedulesQuery}>
<SplitsExpandedProvider
initialMode={expandSplits ? 'collapse' : 'expand'}
>
9 changes: 7 additions & 2 deletions packages/desktop-client/src/components/accounts/Balance.jsx
Original file line number Diff line number Diff line change
@@ -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));
Loading