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
Show file tree
Hide file tree
Changes from 24 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
Expand Up @@ -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'],
Expand Down
7 changes: 7 additions & 0 deletions packages/desktop-client/e2e/accounts.mobile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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();
Expand All @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions packages/desktop-client/e2e/accounts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -109,6 +111,7 @@ test.describe('Accounts', () => {
offBudget: false,
balance: 0,
});
await accountPage.waitFor();
});

async function importCsv(screenshot = false) {
Expand Down
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
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export class MobileAccountPage {
});
}

async waitFor() {
await this.transactionList.waitFor();
}

/**
* Retrieve the balance of the account as a number
*/
Expand All @@ -29,6 +33,10 @@ export class MobileAccountPage {
await this.searchBox.fill(term);
}

async clearSearch() {
await this.searchBox.clear();
}

/**
* Go to transaction creation page
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
7 changes: 5 additions & 2 deletions packages/desktop-client/src/components/ManageRules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -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();
Expand Down
84 changes: 39 additions & 45 deletions packages/desktop-client/src/components/accounts/Account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) => {
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -549,12 +550,11 @@ class AccountInternal extends PureComponent<
},
);
},
{
options: {
pageCount: 150,
onlySync: true,
mapper: ungroupTransactions,
},
);
});
}

UNSAFE_componentWillReceiveProps(nextProps: AccountInternalProps) {
Expand Down Expand Up @@ -590,7 +590,7 @@ class AccountInternal extends PureComponent<
);
} else {
this.updateQuery(
queries.makeTransactionSearchQuery(
queries.transactionsSearch(
this.currentQuery,
this.state.search,
this.props.dateFormat,
Expand Down Expand Up @@ -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 = () => {
Expand All @@ -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' } }]),
);
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -1887,10 +1878,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'}
>
Expand Down
9 changes: 7 additions & 2 deletions packages/desktop-client/src/components/accounts/Balance.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading