diff --git a/packages/desktop-client/src/components/accounts/Account.tsx b/packages/desktop-client/src/components/accounts/Account.tsx index 327b5b8c97..ab753b8f1c 100644 --- a/packages/desktop-client/src/components/accounts/Account.tsx +++ b/packages/desktop-client/src/components/accounts/Account.tsx @@ -75,6 +75,7 @@ import { pushModal, replaceModal, } from '@desktop-client/modals/modalsSlice'; +import type { ConfirmTransactionEditReason } from '@desktop-client/modals/modalsSlice'; import { addNotification } from '@desktop-client/notifications/notificationsSlice'; import { useCreatePayeeMutation } from '@desktop-client/payees'; import * as queries from '@desktop-client/queries'; @@ -1230,7 +1231,7 @@ class AccountInternal extends PureComponent< checkForReconciledTransactions = async ( ids: string[], - confirmReason: string, + confirmReason: ConfirmTransactionEditReason, onConfirm: (ids: string[]) => void, ) => { const { data } = await aqlQuery( diff --git a/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx b/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx index 52c72e9e1d..038b8e1bee 100644 --- a/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx +++ b/packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx @@ -686,7 +686,7 @@ const TransactionEditInner = memo( [categories, isBudgetTransfer, t], ); - const onSaveInner = useCallback(() => { + const onSaveInner = useCallback(async () => { const [unserializedTransaction] = unserializedTransactions; const onConfirmSave = () => { @@ -766,7 +766,7 @@ const TransactionEditInner = memo( return; } - if (unserializedTransaction.reconciled) { + if (unserializedTransactions.some(t => t.reconciled)) { // On mobile any save gives the warning. // On the web only certain changes trigger a warning. // Should we bring that here as well? Or does the nature of the editing form @@ -783,7 +783,37 @@ const TransactionEditInner = memo( }), ); } else { - onConfirmSave(); + const transferIds = unserializedTransactions + .map(t => t.transfer_id) + .filter((id): id is string => id != null); + + if (transferIds.length > 0) { + const { data } = await aqlQuery( + q('transactions') + .filter({ + id: { $oneof: transferIds }, + reconciled: true, + }) + .select('id'), + ); + if ((data as TransactionEntity[]).length > 0) { + dispatch( + pushModal({ + modal: { + name: 'confirm-transaction-edit', + options: { + onConfirm: onConfirmSave, + confirmReason: 'batchEditWithReconciledTransfer', + }, + }, + }), + ); + } else { + onConfirmSave(); + } + } else { + onConfirmSave(); + } } }, [ isAdding, @@ -943,8 +973,10 @@ const TransactionEditInner = memo( ); const onDeleteInner = useCallback( - (id: TransactionEntity['id']) => { - const [unserializedTransaction] = unserializedTransactions; + async (id: TransactionEntity['id']) => { + const [parentTransaction] = unserializedTransactions; + const targetTransaction = + unserializedTransactions.find(t => t.id === id) ?? parentTransaction; const onConfirmDelete = () => { dispatch( @@ -958,7 +990,7 @@ const TransactionEditInner = memo( onConfirm: () => { onDelete(id); - if (unserializedTransaction.id !== id) { + if (parentTransaction.id !== id) { // Only a child transaction was deleted. onClearActiveEdit(); return; @@ -972,7 +1004,7 @@ const TransactionEditInner = memo( ); }; - if (unserializedTransaction.reconciled) { + if (targetTransaction.reconciled) { dispatch( pushModal({ modal: { @@ -984,6 +1016,30 @@ const TransactionEditInner = memo( }, }), ); + } else if (targetTransaction.transfer_id) { + const { data } = await aqlQuery( + q('transactions') + .filter({ + id: targetTransaction.transfer_id, + reconciled: true, + }) + .select('id'), + ); + if ((data as TransactionEntity[]).length > 0) { + dispatch( + pushModal({ + modal: { + name: 'confirm-transaction-edit', + options: { + onConfirm: onConfirmDelete, + confirmReason: 'batchDeleteWithReconciledTransfer', + }, + }, + }), + ); + } else { + onConfirmDelete(); + } } else { onConfirmDelete(); } diff --git a/packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx b/packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx index a78b1141d1..633065afd4 100644 --- a/packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx +++ b/packages/desktop-client/src/components/modals/ConfirmTransactionEditModal.tsx @@ -47,13 +47,29 @@ export function ConfirmTransactionEditModal({ rightContent={ state.close()} />} /> - {confirmReason === 'batchDeleteWithReconciled' ? ( + {confirmReason === 'batchDeleteWithReconciledTransfer' ? ( + + + This transfer has a linked transaction in another account that + is reconciled. Deleting it may bring that account's + reconciliation out of balance. + + + ) : confirmReason === 'batchDeleteWithReconciled' ? ( Deleting reconciled transactions may bring your reconciliation out of balance. + ) : confirmReason === 'batchEditWithReconciledTransfer' ? ( + + + This transfer has a linked transaction in another account that + is reconciled. Editing it may bring that account's + reconciliation out of balance. + + ) : confirmReason === 'batchEditWithReconciled' ? ( @@ -61,6 +77,14 @@ export function ConfirmTransactionEditModal({ out of balance. + ) : confirmReason === 'batchDuplicateWithReconciledTransfer' ? ( + + + This transfer has a linked transaction in another account that + is reconciled. Duplicating it may bring that account's + reconciliation out of balance. + + ) : confirmReason === 'batchDuplicateWithReconciled' ? ( diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.tsx b/packages/desktop-client/src/components/transactions/TransactionsTable.tsx index 8d46e65207..b7fa5789db 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.tsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.tsx @@ -152,6 +152,7 @@ import { pushModal } from '@desktop-client/modals/modalsSlice'; import { NotesTagFormatter } from '@desktop-client/notes/NotesTagFormatter'; import { addNotification } from '@desktop-client/notifications/notificationsSlice'; import { getPayeesById } from '@desktop-client/payees'; +import { aqlQuery } from '@desktop-client/queries/aqlQuery'; import { useDispatch } from '@desktop-client/redux'; type TransactionHeaderProps = { @@ -993,7 +994,7 @@ const Transaction = memo(function Transaction({ const [showReconciliationWarning, setShowReconciliationWarning] = useState(false); - const onUpdate: TransactionUpdateFunction = (name, value) => { + const onUpdate: TransactionUpdateFunction = async (name, value) => { // Had some issues with this is called twice which is a problem now that we are showing a warning // modal if the transaction is locked. I added a boolean to guard against showing the modal twice. // I'm still not completely happy with how the cells update pre/post modal. Sometimes you have to @@ -1002,14 +1003,14 @@ const Transaction = memo(function Transaction({ // of the cell all have different implications as well. if (transaction[name] !== value) { - if ( - transaction.reconciled === true && - (name === 'credit' || - name === 'debit' || - name === 'payee' || - name === 'account' || - name === 'date') - ) { + const isReconciledField = + name === 'credit' || + name === 'debit' || + name === 'payee' || + name === 'account' || + name === 'date'; + + if (transaction.reconciled === true && isReconciledField) { if (showReconciliationWarning === false) { setShowReconciliationWarning(true); dispatch( @@ -1030,6 +1031,38 @@ const Transaction = memo(function Transaction({ }), ); } + } else if ( + isReconciledField && + transaction.transfer_id && + showReconciliationWarning === false + ) { + const { data } = await aqlQuery( + q('transactions') + .filter({ id: transaction.transfer_id, reconciled: true }) + .select('id'), + ); + if ((data as TransactionEntity[]).length > 0) { + setShowReconciliationWarning(true); + dispatch( + pushModal({ + modal: { + name: 'confirm-transaction-edit', + options: { + onCancel: () => { + setShowReconciliationWarning(false); + }, + onConfirm: () => { + setShowReconciliationWarning(false); + onUpdateAfterConfirm(name, value); + }, + confirmReason: 'batchEditWithReconciledTransfer', + }, + }, + }), + ); + } else { + onUpdateAfterConfirm(name, value); + } } else { onUpdateAfterConfirm(name, value); } diff --git a/packages/desktop-client/src/hooks/useTransactionBatchActions.ts b/packages/desktop-client/src/hooks/useTransactionBatchActions.ts index e24a839657..c6200af77e 100644 --- a/packages/desktop-client/src/hooks/useTransactionBatchActions.ts +++ b/packages/desktop-client/src/hooks/useTransactionBatchActions.ts @@ -21,10 +21,18 @@ import type { } from 'loot-core/types/models'; import { pushModal } from '@desktop-client/modals/modalsSlice'; -import type { Modal as ModalType } from '@desktop-client/modals/modalsSlice'; +import type { + ConfirmTransactionEditReason, + Modal as ModalType, +} from '@desktop-client/modals/modalsSlice'; import { aqlQuery } from '@desktop-client/queries/aqlQuery'; import { useDispatch } from '@desktop-client/redux'; +type BatchReconciledReason = Extract< + ConfirmTransactionEditReason, + `batch${string}Reconciled` +>; + type BatchEditProps = { name: keyof TransactionEntity; ids: Array; @@ -242,49 +250,35 @@ export function useTransactionBatchActions() { ); }; + const openFieldEditor = () => { + if (name === 'cleared') { + // Cleared just toggles it on/off and it depends on the data + // loaded. Need to clean this up in the future. + void onChange('cleared', null); + } else if (name === 'category') { + pushCategoryAutocompleteModal(); + } else if (name === 'payee') { + pushPayeeAutocompleteModal(); + } else if (name === 'account') { + pushAccountAutocompleteModal(); + } else { + pushEditField(); + } + }; + if ( name === 'amount' || name === 'payee' || name === 'account' || name === 'date' ) { - const reconciledTransactions = transactions.filter(t => t.reconciled); - if (reconciledTransactions.length > 0) { - dispatch( - pushModal({ - modal: { - name: 'confirm-transaction-edit', - options: { - onConfirm: () => { - if (name === 'payee') { - pushPayeeAutocompleteModal(); - } else if (name === 'account') { - pushAccountAutocompleteModal(); - } else { - pushEditField(); - } - }, - confirmReason: 'batchEditWithReconciled', - }, - }, - }), - ); - return; - } - } - - if (name === 'cleared') { - // Cleared just toggles it on/off and it depends on the data - // loaded. Need to clean this up in the future. - void onChange('cleared', null); - } else if (name === 'category') { - pushCategoryAutocompleteModal(); - } else if (name === 'payee') { - pushPayeeAutocompleteModal(); - } else if (name === 'account') { - pushAccountAutocompleteModal(); + await checkForReconciledTransactions( + ids, + 'batchEditWithReconciled', + openFieldEditor, + ); } else { - pushEditField(); + openFieldEditor(); } }; @@ -445,9 +439,18 @@ export function useTransactionBatchActions() { onSuccess?.(ids); }; + const transferReasonMap: Record< + BatchReconciledReason, + ConfirmTransactionEditReason + > = { + batchDeleteWithReconciled: 'batchDeleteWithReconciledTransfer', + batchEditWithReconciled: 'batchEditWithReconciledTransfer', + batchDuplicateWithReconciled: 'batchDuplicateWithReconciledTransfer', + }; + const checkForReconciledTransactions = async ( ids: Array, - confirmReason: string, + confirmReason: BatchReconciledReason, onConfirm: (ids: Array) => void, ) => { const { data } = await aqlQuery( @@ -457,6 +460,7 @@ export function useTransactionBatchActions() { .options({ splits: 'grouped' }), ); const transactions = ungroupTransactions(data as TransactionEntity[]); + if (transactions.length > 0) { dispatch( pushModal({ @@ -471,9 +475,46 @@ export function useTransactionBatchActions() { }, }), ); - } else { - onConfirm(ids); + return; } + + // check paired transfer transactions + const { data: selectedData } = await aqlQuery( + q('transactions') + .filter({ id: { $oneof: ids } }) + .select(['transfer_id']), + ); + + const transferIds = (selectedData as TransactionEntity[]) + .map(t => t.transfer_id) + .filter((id): id is string => id != null); + + if (transferIds.length > 0) { + const { data: reconciledTransfers } = await aqlQuery( + q('transactions') + .filter({ id: { $oneof: transferIds }, reconciled: true }) + .select('*'), + ); + + if ((reconciledTransfers as TransactionEntity[]).length > 0) { + dispatch( + pushModal({ + modal: { + name: 'confirm-transaction-edit', + options: { + onConfirm: () => { + onConfirm(ids); + }, + confirmReason: transferReasonMap[confirmReason], + }, + }, + }), + ); + return; + } + } + + onConfirm(ids); }; const onSetTransfer = async ( diff --git a/packages/desktop-client/src/modals/modalsSlice.ts b/packages/desktop-client/src/modals/modalsSlice.ts index 8818b9ce2a..c47f614187 100644 --- a/packages/desktop-client/src/modals/modalsSlice.ts +++ b/packages/desktop-client/src/modals/modalsSlice.ts @@ -28,6 +28,17 @@ import { signOut } from '@desktop-client/users/usersSlice'; const sliceName = 'modals'; +export type ConfirmTransactionEditReason = + | 'batchDeleteWithReconciled' + | 'batchDeleteWithReconciledTransfer' + | 'batchEditWithReconciled' + | 'batchEditWithReconciledTransfer' + | 'batchDuplicateWithReconciled' + | 'batchDuplicateWithReconciledTransfer' + | 'editReconciled' + | 'unlockReconciled' + | 'deleteReconciled'; + export type Modal = | { name: 'import-transactions'; @@ -518,7 +529,7 @@ export type Modal = options: { onConfirm: () => void; onCancel?: () => void; - confirmReason: string; + confirmReason: ConfirmTransactionEditReason; }; } | { diff --git a/upcoming-release-notes/7269.md b/upcoming-release-notes/7269.md new file mode 100644 index 0000000000..ea06cf4dfe --- /dev/null +++ b/upcoming-release-notes/7269.md @@ -0,0 +1,6 @@ +--- +category: Bugfixes +authors: [matt-fidd] +--- + +Show confirmation dialog when editing/duplicating/deleting transfers where the other half is reconciled