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

[Redux Toolkit Migration] queriesSlice #4016

Merged
merged 30 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a6f4576
Migrate to accountSlice
joel-jeremy Dec 19, 2024
943281d
Fix lint and typecheck errors
joel-jeremy Dec 19, 2024
813388d
Update types
joel-jeremy Dec 19, 2024
4222add
Fix lint
joel-jeremy Dec 19, 2024
7c11cd4
Fix types
joel-jeremy Dec 19, 2024
1be216f
Cleanup
joel-jeremy Dec 19, 2024
4822331
Rename file
joel-jeremy Dec 20, 2024
ea3160c
Rename state
joel-jeremy Dec 20, 2024
512480b
Cleanup
joel-jeremy Dec 20, 2024
7dd17a5
Fix typecheck error
joel-jeremy Jan 8, 2025
6fb9827
Move createAppAsyncThunk
joel-jeremy Jan 9, 2025
27c6693
Queries slice
joel-jeremy Dec 19, 2024
964c261
Release notes
joel-jeremy Dec 19, 2024
723d69a
Cleanup types
joel-jeremy Dec 20, 2024
19cdc3a
Cleanup
joel-jeremy Jan 1, 2025
a454660
Fix typecheck error
joel-jeremy Jan 7, 2025
943fda1
Lint
joel-jeremy Jan 7, 2025
79ca573
Fix typecheck error
joel-jeremy Jan 8, 2025
3d0d64e
Fix import
joel-jeremy Jan 9, 2025
1356ed2
Fix typo
joel-jeremy Jan 13, 2025
d5a904f
Fix typo
joel-jeremy Jan 14, 2025
cbf5cf1
Update
joel-jeremy Jan 14, 2025
66e2897
Copy category list so that sorting is not applied directly on categor…
joel-jeremy Jan 14, 2025
acbfcce
Update setLastTransaction payload
joel-jeremy Jan 14, 2025
f7c216e
Remove optional optional chaining on unwrap
joel-jeremy Jan 14, 2025
e84a73f
Rename accountId
joel-jeremy Jan 15, 2025
d28d75c
Fix type
joel-jeremy Jan 16, 2025
f31bcfe
Remove return value of initiallyLoadPayees since no callers use it
joel-jeremy Jan 16, 2025
8e42b2e
No need to getPayees. Just use the already loaded payees.
joel-jeremy Jan 16, 2025
96c781d
Notify on action errors
joel-jeremy Jan 17, 2025
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
2 changes: 1 addition & 1 deletion packages/desktop-client/src/components/ManageRules.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import React, {
import { useTranslation } from 'react-i18next';

import { useSchedules } from 'loot-core/client/data-hooks/schedules';
import { initiallyLoadPayees } from 'loot-core/client/queries/queriesSlice';
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';
import * as undo from 'loot-core/src/platform/client/undo';
import { getNormalisedString } from 'loot-core/src/shared/normalisation';
Expand Down
30 changes: 17 additions & 13 deletions packages/desktop-client/src/components/accounts/Account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@ import { v4 as uuidv4 } from 'uuid';
import { unlinkAccount } from 'loot-core/client/accounts/accountsSlice';
import {
addNotification,
createPayee,
getPayees,
initiallyLoadPayees,
markAccountRead,
openAccountCloseModal,
pushModal,
reopenAccount,
replaceModal,
syncAndDownload,
} from 'loot-core/client/actions';
import {
createPayee,
initiallyLoadPayees,
markAccountRead,
reopenAccount,
updateAccount,
updateNewTransactions,
} from 'loot-core/client/actions';
} from 'loot-core/client/queries/queriesSlice';
import { type AppDispatch } from 'loot-core/client/store';
import { validForTransfer } from 'loot-core/client/transfer';
import { type UndoState } from 'loot-core/server/undo';
Expand Down Expand Up @@ -59,7 +60,6 @@ import {
type NewRuleEntity,
type RuleActionEntity,
type AccountEntity,
type PayeeEntity,
type RuleConditionEntity,
type TransactionEntity,
type TransactionFilterEntity,
Expand Down Expand Up @@ -501,7 +501,7 @@ class AccountInternal extends PureComponent<
else this.updateQuery(query);

if (this.props.accountId) {
this.props.dispatch(markAccountRead(this.props.accountId));
this.props.dispatch(markAccountRead({ id: this.props.accountId }));
}
};

Expand Down Expand Up @@ -687,7 +687,7 @@ class AccountInternal extends PureComponent<
}
});

this.props.dispatch(updateNewTransactions(updatedTransaction.id));
this.props.dispatch(updateNewTransactions({ id: updatedTransaction.id }));
};

canCalculateBalance = () => {
Expand Down Expand Up @@ -781,7 +781,10 @@ class AccountInternal extends PureComponent<
const account = this.props.accounts.find(
account => account.id === this.props.accountId,
);
this.props.dispatch(updateAccount({ ...account, name } as AccountEntity));
if (!account) {
throw new Error(`Account with ID ${this.props.accountId} not found.`);
}
this.props.dispatch(updateAccount({ account: { ...account, name } }));
this.setState({ editingName: false, nameError: '' });
}
};
Expand Down Expand Up @@ -829,7 +832,7 @@ class AccountInternal extends PureComponent<
this.props.dispatch(openAccountCloseModal(accountId));
break;
case 'reopen':
this.props.dispatch(reopenAccount(accountId));
this.props.dispatch(reopenAccount({ id: accountId }));
break;
case 'export':
const accountName = this.getAccountTitle(account, accountId);
Expand Down Expand Up @@ -944,7 +947,7 @@ class AccountInternal extends PureComponent<
onCreatePayee = (name: string) => {
const trimmed = name.trim();
if (trimmed !== '') {
return this.props.dispatch(createPayee(name));
return this.props.dispatch(createPayee({ name })).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of this .unwrap API tbh, wish it threw by default like a normal Promise :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Error handling needed for createPayee operation

The review comment is valid. The codebase shows that:

  • addGenericErrorNotification exists and is commonly used for error handling
  • Other instances of createPayee with unwrap() also lack error handling
  • Similar async operations in the codebase handle errors appropriately

The suggested try-catch solution using addGenericErrorNotification aligns with the existing error handling patterns in the codebase.

🔗 Analysis chain

Consider handling potential errors when creating a payee

The use of .unwrap() after dispatch(createPayee({ name })) will throw an error if the promise is rejected. Ensure that this error is caught or handled appropriately to prevent unhandled promise rejections.

Consider wrapping the dispatch call in a try-catch block:

+ try {
    return await this.props.dispatch(createPayee({ name })).unwrap();
+ } catch (error) {
+   // Handle the error, e.g., display a notification
+   this.props.dispatch(addGenericErrorNotification());
+   return null;
+ }
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find createPayee usage
rg "createPayee" -A 3 -B 3

# Look for error notification patterns
rg "addGenericErrorNotification|addNotification.*error" -A 2 -B 2

# Find error handling patterns with unwrap
rg "\.unwrap\(\)" -A 3 -B 3

Length of output: 13725

}
return null;
};
Expand Down Expand Up @@ -1322,7 +1325,8 @@ class AccountInternal extends PureComponent<
const onConfirmTransfer = async (ids: string[]) => {
this.setState({ workingHard: true });

const payees: PayeeEntity[] = await this.props.dispatch(getPayees());
const payees = this.props.payees;

const { data: transactions } = await runQuery(
q('transactions')
.filter({ id: { $oneof: ids } })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ import { Trans, useTranslation } from 'react-i18next';

import { css, cx } from '@emotion/css';

import { createPayee } from 'loot-core/src/client/actions/queries';
import { getActivePayees } from 'loot-core/src/client/reducers/queries';
import {
createPayee,
getActivePayees,
} from 'loot-core/client/queries/queriesSlice';
import { getNormalisedString } from 'loot-core/src/shared/normalisation';
import {
type AccountEntity,
Expand Down Expand Up @@ -326,7 +328,8 @@ export function PayeeAutocomplete({
if (!clearOnBlur) {
onSelect?.(makeNew(idOrIds, rawInputValue), rawInputValue);
} else {
const create = payeeName => dispatch(createPayee(payeeName));
const create = payeeName =>
dispatch(createPayee({ name: payeeName })).unwrap();

if (Array.isArray(idOrIds)) {
idOrIds = await Promise.all(
Expand Down
52 changes: 31 additions & 21 deletions packages/desktop-client/src/components/budget/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import React, { memo, useMemo, useState, useEffect } from 'react';
import { useTranslation } from 'react-i18next';

import {
addNotification,
applyBudgetAction,
createCategory,
createGroup,
Expand All @@ -12,10 +11,10 @@ import {
getCategories,
moveCategory,
moveCategoryGroup,
pushModal,
updateCategory,
updateGroup,
} from 'loot-core/src/client/actions';
} from 'loot-core/client/queries/queriesSlice';
import { addNotification, pushModal } from 'loot-core/src/client/actions';
import { useSpreadsheet } from 'loot-core/src/client/SpreadsheetProvider';
import { send, listen } from 'loot-core/src/platform/client/fetch';
import * as monthUtils from 'loot-core/src/shared/months';
Expand Down Expand Up @@ -203,15 +202,15 @@ function BudgetInner(props: BudgetInnerProps) {

if (category.id === 'new') {
dispatch(
createCategory(
category.name,
category.cat_group,
category.is_income,
category.hidden,
),
createCategory({
name: category.name,
groupId: category.cat_group,
isIncome: category.is_income,
isHidden: category.hidden,
}),
);
} else {
dispatch(updateCategory(category));
dispatch(updateCategory({ category }));
}
};

Expand All @@ -224,21 +223,21 @@ function BudgetInner(props: BudgetInnerProps) {
category: id,
onDelete: transferCategory => {
if (id !== transferCategory) {
dispatch(deleteCategory(id, transferCategory));
dispatch(deleteCategory({ id, transferId: transferCategory }));
}
},
}),
);
} else {
dispatch(deleteCategory(id));
dispatch(deleteCategory({ id }));
}
};

const onSaveGroup = group => {
if (group.id === 'new') {
dispatch(createGroup(group.name));
dispatch(createGroup({ name: group.name }));
} else {
dispatch(updateGroup(group));
dispatch(updateGroup({ group }));
}
};

Expand All @@ -258,26 +257,29 @@ function BudgetInner(props: BudgetInnerProps) {
pushModal('confirm-category-delete', {
group: id,
onDelete: transferCategory => {
dispatch(deleteGroup(id, transferCategory));
dispatch(deleteGroup({ id, transferId: transferCategory }));
},
}),
);
} else {
dispatch(deleteGroup(id));
dispatch(deleteGroup({ id }));
}
};

const onApplyBudgetTemplatesInGroup = async categories => {
dispatch(
applyBudgetAction(startMonth, 'apply-multiple-templates', {
applyBudgetAction({
month: startMonth,
categories,
type: 'apply-multiple-templates',
args: {
categories,
},
}),
);
};

const onBudgetAction = (month, type, args) => {
dispatch(applyBudgetAction(month, type, args));
dispatch(applyBudgetAction({ month, type, args }));
};

const onShowActivity = (categoryId, month) => {
Expand Down Expand Up @@ -316,11 +318,19 @@ function BudgetInner(props: BudgetInnerProps) {
return;
}

dispatch(moveCategory(sortInfo.id, sortInfo.groupId, sortInfo.targetId));
dispatch(
moveCategory({
id: sortInfo.id,
groupId: sortInfo.groupId,
targetId: sortInfo.targetId,
}),
);
};

const onReorderGroup = async sortInfo => {
dispatch(moveCategoryGroup(sortInfo.id, sortInfo.targetId));
dispatch(
moveCategoryGroup({ id: sortInfo.id, targetId: sortInfo.targetId }),
);
};

const onToggleCollapse = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,9 @@ import React, {

import {
collapseModals,
getPayees,
markAccountRead,
openAccountCloseModal,
pushModal,
reopenAccount,
syncAndDownload,
updateAccount,
} from 'loot-core/client/actions';
import {
accountSchedulesQuery,
Expand All @@ -25,6 +21,12 @@ import {
useTransactionsSearch,
} from 'loot-core/client/data-hooks/transactions';
import * as queries from 'loot-core/client/queries';
import {
getPayees,
markAccountRead,
reopenAccount,
updateAccount,
} from 'loot-core/client/queries/queriesSlice';
import { listen, send } from 'loot-core/platform/client/fetch';
import { type Query } from 'loot-core/shared/query';
import { isPreviewId } from 'loot-core/shared/transactions';
Expand Down Expand Up @@ -111,7 +113,7 @@ function AccountHeader({ account }: { readonly account: AccountEntity }) {

const onSave = useCallback(
(account: AccountEntity) => {
dispatch(updateAccount(account));
dispatch(updateAccount({ account }));
},
[dispatch],
);
Expand All @@ -138,7 +140,7 @@ function AccountHeader({ account }: { readonly account: AccountEntity }) {
}, [account.id, dispatch]);

const onReopenAccount = useCallback(() => {
dispatch(reopenAccount(account.id));
dispatch(reopenAccount({ id: account.id }));
}, [account.id, dispatch]);

const onClick = useCallback(() => {
Expand Down Expand Up @@ -262,7 +264,7 @@ function TransactionListWithPreviews({

useEffect(() => {
if (accountId) {
dispatch(markAccountRead(accountId));
dispatch(markAccountRead({ id: accountId }));
}
}, [accountId, dispatch]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React, { useCallback, useEffect, useState } from 'react';

import { getPayees } from 'loot-core/client/actions';
import {
useTransactions,
useTransactionsSearch,
} from 'loot-core/client/data-hooks/transactions';
import * as queries from 'loot-core/client/queries';
import { getPayees } from 'loot-core/client/queries/queriesSlice';
import { listen } from 'loot-core/platform/client/fetch';
import * as monthUtils from 'loot-core/shared/months';
import { q } from 'loot-core/shared/query';
Expand Down
Loading
Loading