mirror of
https://github.com/actualbudget/actual.git
synced 2026-10-12 02:10:19 +00:00
[AI] Keep explicitly cleared fields empty when entering new transactions (#8792)
* [AI] Keep explicitly cleared fields empty when entering new transactions Rules re-run on every field commit while a new transaction is being entered, and they are allowed to fill any empty field. A field the user deliberately cleared (e.g. a category pre-assigned from the payee) was treated as empty too, so the rule immediately re-filled it - either on the clear itself or on a later run triggered by editing another field such as the amount. Track which fields the user explicitly emptied for the lifetime of the new-transaction row (desktop register) or edit screen (mobile), and bar rule runs from re-filling those fields until the row is added or reset. Auto-fill for untouched fields and notes append/prepend rules keep working as before. Fixes the desktop half of #7390; the mobile modal was fixed in #7521. Co-Authored-By: Claude Fable 5 <[email protected]> * [AI] Keep cleared fields blocked during mobile payee rule runs The payee exception in the mobile transaction edit bypassed shouldApplyRuleChange entirely, so selecting a payee could re-fill a field the user had explicitly cleared. Check the cleared-field set before the payee exception, matching the desktop behavior. Addresses CodeRabbit review feedback on #8792. Co-Authored-By: Claude Fable 5 <[email protected]> * [AI] Track cleared fields per transaction row Review feedback on the cleared-fields tracking: the fields were stored by name only, shared across the parent and all split rows, so clearing e.g. the category on one split row stopped rules from filling the category on its siblings too. Track cleared fields in a Map keyed by transaction id on both the desktop register and the mobile transaction edit screen. The rules merge is extracted into a pure applyRulesToTransaction() helper with unit tests covering the split scenarios, and split propagation now skips only rows whose field the user actually cleared instead of inferring a clear from the parent's (always empty) field value. Co-Authored-By: Claude Fable 5 <[email protected]> * [AI] Share split propagation between desktop and mobile rule merges The mobile transaction edit screen kept its own copy of the parent-to-subtransaction propagation step. Extract it into an exported propagateRuleChangeToSubtransactions() helper and call it from both paths so the two cannot drift apart. The mobile payee override is unchanged. Addresses CodeRabbit review feedback on #8792. Co-Authored-By: Claude Fable 5.1 <[email protected]> --------- Co-authored-by: bsmarsh333 <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Fable 5.1
bsmarsh333
parent
1eeffe3278
commit
a264f74038
@@ -85,6 +85,7 @@
|
||||
"#components/schedules": "./src/components/schedules/index.tsx",
|
||||
"#components/schedules/schedule-edit-utils": "./src/components/schedules/schedule-edit-utils.ts",
|
||||
"#components/select/getFirstDayOfWeek": "./src/components/select/getFirstDayOfWeek.ts",
|
||||
"#components/transactions/applyRulesToTransaction": "./src/components/transactions/applyRulesToTransaction.ts",
|
||||
"#components/transactions/table/columns": "./src/components/transactions/table/columns.ts",
|
||||
"#components/transactions/table/utils": "./src/components/transactions/table/utils.ts",
|
||||
"#components/util/accountValidation": "./src/components/util/accountValidation.ts",
|
||||
|
||||
@@ -81,7 +81,12 @@ import {
|
||||
} from '#components/mobile/MobileForms';
|
||||
import { getPrettyPayee } from '#components/mobile/utils';
|
||||
import { MobilePageHeader, Page } from '#components/Page';
|
||||
import { shouldApplyRuleChange } from '#components/transactions/table/utils';
|
||||
import { propagateRuleChangeToSubtransactions } from '#components/transactions/applyRulesToTransaction';
|
||||
import {
|
||||
getClearedFieldNames,
|
||||
shouldApplyRuleChange,
|
||||
trackClearedField,
|
||||
} from '#components/transactions/table/utils';
|
||||
import { useAccounts } from '#hooks/useAccounts';
|
||||
import { useCategories } from '#hooks/useCategories';
|
||||
import { useDateFormat } from '#hooks/useDateFormat';
|
||||
@@ -1709,6 +1714,12 @@ function TransactionEditUnconnected({
|
||||
searchParams,
|
||||
]);
|
||||
|
||||
// Fields the user explicitly emptied while entering the new transaction,
|
||||
// tracked per transaction row so a clear on one split row doesn't affect
|
||||
// its siblings. Rules re-run on every field commit, and they may not
|
||||
// re-fill these (e.g. a cleared pre-assigned category must stay cleared).
|
||||
const clearedFields = useRef(new Map<TransactionEntity['id'], Set<string>>());
|
||||
|
||||
const onUpdate = useCallback(
|
||||
async (
|
||||
serializedTransaction: TransactionEntity,
|
||||
@@ -1725,6 +1736,12 @@ function TransactionEditUnconnected({
|
||||
const newTransaction = { ...transaction };
|
||||
const changedFields = new Set<keyof TransactionEntity>([updatedField]);
|
||||
if (isTemporary(newTransaction)) {
|
||||
trackClearedField(clearedFields.current, newTransaction, updatedField);
|
||||
const clearedOnThisRow = getClearedFieldNames(
|
||||
clearedFields.current,
|
||||
newTransaction.id,
|
||||
);
|
||||
|
||||
const afterRules = await send('rules-run', {
|
||||
transaction: newTransaction,
|
||||
});
|
||||
@@ -1737,29 +1754,29 @@ function TransactionEditUnconnected({
|
||||
// (see shouldApplyRuleChange).
|
||||
// Or update all fields if the payee changes (assists location-based entry by
|
||||
// applying rules to prefill category, notes, etc. based on the selected payee)
|
||||
// — except fields the user explicitly cleared, which must stay empty.
|
||||
if (
|
||||
updatedField === 'payee' ||
|
||||
shouldApplyRuleChange(field, newTransaction[field], diff[field])
|
||||
!clearedOnThisRow.includes(field) &&
|
||||
(updatedField === 'payee' ||
|
||||
shouldApplyRuleChange(
|
||||
field,
|
||||
newTransaction[field],
|
||||
diff[field],
|
||||
))
|
||||
) {
|
||||
(newTransaction as Record<string, unknown>)[field] = diff[field];
|
||||
changedFields.add(field);
|
||||
}
|
||||
});
|
||||
|
||||
// When a rule updates a parent transaction, overwrite all changes to the current field in subtransactions.
|
||||
if (
|
||||
newTransaction.is_parent &&
|
||||
diff.subtransactions !== undefined &&
|
||||
updatedField !== null
|
||||
) {
|
||||
newTransaction.subtransactions = diff.subtransactions.map(
|
||||
(st, idx) => ({
|
||||
...(newTransaction.subtransactions?.[idx] || st),
|
||||
...(st[updatedField] != null && {
|
||||
[updatedField]: st[updatedField],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const subtransactions = propagateRuleChangeToSubtransactions(
|
||||
newTransaction,
|
||||
diff.subtransactions,
|
||||
updatedField,
|
||||
clearedFields.current,
|
||||
);
|
||||
if (subtransactions) {
|
||||
newTransaction.subtransactions = subtransactions;
|
||||
changedFields.add('subtransactions');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,7 @@ import {
|
||||
splitTransaction,
|
||||
updateTransaction,
|
||||
} from '@actual-app/core/shared/transactions';
|
||||
import { applyChanges, getChangedValues } from '@actual-app/core/shared/util';
|
||||
import { applyChanges } from '@actual-app/core/shared/util';
|
||||
import type {
|
||||
AccountEntity,
|
||||
CategoryEntity,
|
||||
@@ -36,7 +36,8 @@ import { pushModal } from '#modals/modalsSlice';
|
||||
import { addNotification } from '#notifications/notificationsSlice';
|
||||
import { useDispatch } from '#redux';
|
||||
|
||||
import { shouldApplyRuleChange } from './table/utils';
|
||||
import { applyRulesToTransaction } from './applyRulesToTransaction';
|
||||
import type { ClearedFieldsByTransaction } from './table/utils';
|
||||
import { TransactionTable } from './TransactionsTable';
|
||||
import type { TransactionTableProps } from './TransactionsTable';
|
||||
// When data changes, there are two ways to update the UI:
|
||||
@@ -265,6 +266,7 @@ export function TransactionList({
|
||||
async (
|
||||
transaction: TransactionEntity,
|
||||
updatedFieldName: string | null = null,
|
||||
clearedFields?: ClearedFieldsByTransaction,
|
||||
) => {
|
||||
const afterRules = await send('rules-run', { transaction });
|
||||
|
||||
@@ -281,35 +283,12 @@ export function TransactionList({
|
||||
);
|
||||
}
|
||||
|
||||
const diff = getChangedValues(transaction, afterRules);
|
||||
|
||||
const newTransaction: TransactionEntity = { ...transaction };
|
||||
if (diff) {
|
||||
Object.keys(diff).forEach(field => {
|
||||
if (
|
||||
shouldApplyRuleChange(field, newTransaction[field], diff[field])
|
||||
) {
|
||||
newTransaction[field] = diff[field];
|
||||
}
|
||||
});
|
||||
|
||||
// When a rule updates a parent transaction, overwrite all changes to the current field in subtransactions.
|
||||
if (
|
||||
transaction.is_parent &&
|
||||
diff.subtransactions !== undefined &&
|
||||
updatedFieldName !== null
|
||||
) {
|
||||
newTransaction.subtransactions = diff.subtransactions.map(
|
||||
(st, idx) => ({
|
||||
...(newTransaction.subtransactions?.[idx] || st),
|
||||
...(st[updatedFieldName] != null && {
|
||||
[updatedFieldName]: st[updatedFieldName],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
return newTransaction;
|
||||
return applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
updatedFieldName,
|
||||
clearedFields,
|
||||
);
|
||||
},
|
||||
[dispatch],
|
||||
);
|
||||
|
||||
@@ -160,8 +160,10 @@ import {
|
||||
makeTemporaryTransactions,
|
||||
selectAscDesc,
|
||||
serializeTransaction,
|
||||
trackClearedField,
|
||||
} from './table/utils';
|
||||
import type {
|
||||
ClearedFieldsByTransaction,
|
||||
SerializedTransaction,
|
||||
TransactionEditFunction,
|
||||
TransactionUpdateFunction,
|
||||
@@ -2612,6 +2614,7 @@ type TransactionTableInnerProps = {
|
||||
onApplyRules: (
|
||||
transaction: TransactionEntity,
|
||||
field: string,
|
||||
clearedFields?: ClearedFieldsByTransaction,
|
||||
) => Promise<TransactionEntity>;
|
||||
onSplit: (id: TransactionEntity['id']) => void;
|
||||
onAddSplit: (id: TransactionEntity['id']) => void;
|
||||
@@ -3022,6 +3025,7 @@ export type TransactionTableProps = {
|
||||
onApplyRules: (
|
||||
transaction: TransactionEntity,
|
||||
field: string | null,
|
||||
clearedFields?: ClearedFieldsByTransaction,
|
||||
) => Promise<TransactionEntity>;
|
||||
onSplit: (id: TransactionEntity['id']) => TransactionEntity['id'];
|
||||
onAddSplit: (id: TransactionEntity['id']) => TransactionEntity['id'];
|
||||
@@ -3288,6 +3292,14 @@ export const TransactionTable = forwardRef(
|
||||
const afterSaveFunc = useRef<null | (() => void)>(null);
|
||||
const [_, forceRerender] = useState({});
|
||||
const selectedItems = useSelectedItems();
|
||||
// Fields the user explicitly emptied while entering the new transaction,
|
||||
// tracked per row id so a clear on one split row doesn't affect its
|
||||
// siblings. Rules re-run on every field commit, and they may not re-fill
|
||||
// these (e.g. a cleared pre-assigned category must stay cleared until the
|
||||
// row is added or reset).
|
||||
const clearedFields = useRef(
|
||||
new Map<TransactionEntity['id'], Set<string>>(),
|
||||
);
|
||||
|
||||
latestState.current = {
|
||||
newTransactions: newTransactions ?? [],
|
||||
@@ -3299,6 +3311,7 @@ export const TransactionTable = forwardRef(
|
||||
// Derive new transactions from the `isAdding` prop
|
||||
if (prevIsAdding !== props.isAdding) {
|
||||
if (!prevIsAdding && props.isAdding) {
|
||||
clearedFields.current.clear();
|
||||
setNewTransactions(
|
||||
makeTemporaryTransactions(
|
||||
props.currentAccountId,
|
||||
@@ -3329,6 +3342,7 @@ export const TransactionTable = forwardRef(
|
||||
} else {
|
||||
const lastDate =
|
||||
transactions.length > 0 ? transactions[0].date : null;
|
||||
clearedFields.current.clear();
|
||||
setNewTransactions(
|
||||
makeTemporaryTransactions(
|
||||
props.currentAccountId,
|
||||
@@ -3381,6 +3395,7 @@ export const TransactionTable = forwardRef(
|
||||
}),
|
||||
);
|
||||
// Reset form like onAddTemporary does
|
||||
clearedFields.current.clear();
|
||||
setNewTransactions(
|
||||
makeTemporaryTransactions(
|
||||
props.currentAccountId,
|
||||
@@ -3647,9 +3662,22 @@ export const TransactionTable = forwardRef(
|
||||
|
||||
if (isTemporaryId(transaction.id)) {
|
||||
if (onApplyRulesProp) {
|
||||
// Remember fields the user explicitly emptied so later rule runs
|
||||
// (triggered by edits to other fields) can't re-fill them either.
|
||||
// `transaction` is the row that was actually edited (a split
|
||||
// child or the parent), so track the clear against its own id.
|
||||
if (updatedFieldName !== null) {
|
||||
trackClearedField(
|
||||
clearedFields.current,
|
||||
transaction,
|
||||
updatedFieldName,
|
||||
);
|
||||
}
|
||||
|
||||
groupedTransaction = await onApplyRulesProp(
|
||||
groupedTransaction,
|
||||
updatedFieldName,
|
||||
clearedFields.current,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3940,6 +3968,7 @@ export const TransactionTable = forwardRef(
|
||||
);
|
||||
|
||||
function onCloseAddTransaction() {
|
||||
clearedFields.current.clear();
|
||||
setNewTransactions(
|
||||
makeTemporaryTransactions(
|
||||
props.currentAccountId,
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
import type { TransactionEntity } from '@actual-app/core/types/models';
|
||||
|
||||
import { applyRulesToTransaction } from './applyRulesToTransaction';
|
||||
import { trackClearedField } from './table/utils';
|
||||
|
||||
const baseTransaction: TransactionEntity = {
|
||||
id: 'txn-1',
|
||||
account: 'account-1',
|
||||
amount: -1000,
|
||||
date: '2026-10-01',
|
||||
payee: 'amazon-payee-id',
|
||||
};
|
||||
|
||||
function clearedFieldsOf(
|
||||
...entries: Array<[TransactionEntity['id'], string[]]>
|
||||
) {
|
||||
return new Map(entries.map(([id, fields]) => [id, new Set(fields)]));
|
||||
}
|
||||
|
||||
describe('applyRulesToTransaction', () => {
|
||||
it('fills an empty category when the user edited a different field', () => {
|
||||
const transaction = { ...baseTransaction, category: undefined };
|
||||
const afterRules = { ...transaction, category: 'shopping-id' };
|
||||
|
||||
const result = applyRulesToTransaction(transaction, afterRules, 'payee');
|
||||
|
||||
expect(result.category).toBe('shopping-id');
|
||||
});
|
||||
|
||||
it('keeps a field empty when the user explicitly cleared it on this row', () => {
|
||||
const transaction = { ...baseTransaction, category: undefined };
|
||||
const afterRules = { ...transaction, category: 'shopping-id' };
|
||||
|
||||
const result = applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
'category',
|
||||
clearedFieldsOf(['txn-1', ['category']]),
|
||||
);
|
||||
|
||||
expect(result.category).toBeUndefined();
|
||||
});
|
||||
|
||||
it('still fills a field that was cleared on a different row', () => {
|
||||
const transaction = { ...baseTransaction, category: undefined };
|
||||
const afterRules = { ...transaction, category: 'shopping-id' };
|
||||
|
||||
const result = applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
'amount',
|
||||
clearedFieldsOf(['some-other-txn', ['category']]),
|
||||
);
|
||||
|
||||
expect(result.category).toBe('shopping-id');
|
||||
});
|
||||
|
||||
it('does not override a non-empty value the user chose', () => {
|
||||
const transaction = { ...baseTransaction, category: 'groceries-id' };
|
||||
const afterRules = { ...transaction, category: 'shopping-id' };
|
||||
|
||||
const result = applyRulesToTransaction(transaction, afterRules, 'payee');
|
||||
|
||||
expect(result.category).toBe('groceries-id');
|
||||
});
|
||||
|
||||
describe('split transactions', () => {
|
||||
function makeSplitTransaction(): TransactionEntity {
|
||||
return {
|
||||
...baseTransaction,
|
||||
is_parent: true,
|
||||
category: undefined,
|
||||
subtransactions: [
|
||||
{
|
||||
id: 'sub-1',
|
||||
account: 'account-1',
|
||||
amount: -600,
|
||||
date: '2026-10-01',
|
||||
},
|
||||
{
|
||||
id: 'sub-2',
|
||||
account: 'account-1',
|
||||
amount: -400,
|
||||
date: '2026-10-01',
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
it('clearing a field on one split row does not block rules on its siblings', () => {
|
||||
const transaction = makeSplitTransaction();
|
||||
const afterRules: TransactionEntity = {
|
||||
...transaction,
|
||||
subtransactions: [
|
||||
{ ...transaction.subtransactions![0], category: 'shopping-id' },
|
||||
{ ...transaction.subtransactions![1], category: 'shopping-id' },
|
||||
],
|
||||
};
|
||||
|
||||
// The user cleared the category on sub-1 only.
|
||||
const result = applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
'category',
|
||||
clearedFieldsOf(['sub-1', ['category']]),
|
||||
);
|
||||
|
||||
expect(result.subtransactions?.[0].category).toBeUndefined();
|
||||
expect(result.subtransactions?.[1].category).toBe('shopping-id');
|
||||
});
|
||||
|
||||
it('propagates rule values to all split rows when nothing was cleared', () => {
|
||||
// The parent of a split never has a category itself; that must not be
|
||||
// mistaken for the user having cleared it.
|
||||
const transaction = makeSplitTransaction();
|
||||
const afterRules: TransactionEntity = {
|
||||
...transaction,
|
||||
subtransactions: [
|
||||
{ ...transaction.subtransactions![0], category: 'shopping-id' },
|
||||
{ ...transaction.subtransactions![1], category: 'shopping-id' },
|
||||
],
|
||||
};
|
||||
|
||||
const result = applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
'category',
|
||||
);
|
||||
|
||||
expect(result.subtransactions?.[0].category).toBe('shopping-id');
|
||||
expect(result.subtransactions?.[1].category).toBe('shopping-id');
|
||||
});
|
||||
|
||||
it('does not propagate to any split row when the field was cleared on the parent', () => {
|
||||
const transaction = { ...makeSplitTransaction(), notes: '' };
|
||||
const afterRules: TransactionEntity = {
|
||||
...transaction,
|
||||
subtransactions: [
|
||||
{ ...transaction.subtransactions![0], notes: 'Amazon order' },
|
||||
{ ...transaction.subtransactions![1], notes: 'Amazon order' },
|
||||
],
|
||||
};
|
||||
|
||||
const result = applyRulesToTransaction(
|
||||
transaction,
|
||||
afterRules,
|
||||
'notes',
|
||||
clearedFieldsOf(['txn-1', ['notes']]),
|
||||
);
|
||||
|
||||
expect(result.subtransactions?.[0].notes).toBeUndefined();
|
||||
expect(result.subtransactions?.[1].notes).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('trackClearedField', () => {
|
||||
it('marks a field cleared when its value is empty and unmarks it when set', () => {
|
||||
const clearedFields = new Map<TransactionEntity['id'], Set<string>>();
|
||||
|
||||
trackClearedField(
|
||||
clearedFields,
|
||||
{ ...baseTransaction, category: undefined },
|
||||
'category',
|
||||
);
|
||||
expect(clearedFields.get('txn-1')?.has('category')).toBe(true);
|
||||
|
||||
trackClearedField(
|
||||
clearedFields,
|
||||
{ ...baseTransaction, category: 'groceries-id' },
|
||||
'category',
|
||||
);
|
||||
expect(clearedFields.get('txn-1')?.has('category')).toBe(false);
|
||||
});
|
||||
|
||||
it('tracks rows independently', () => {
|
||||
const clearedFields = new Map<TransactionEntity['id'], Set<string>>();
|
||||
|
||||
trackClearedField(
|
||||
clearedFields,
|
||||
{ ...baseTransaction, id: 'sub-1', category: undefined },
|
||||
'category',
|
||||
);
|
||||
|
||||
expect(clearedFields.get('sub-1')?.has('category')).toBe(true);
|
||||
expect(clearedFields.get('sub-2')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,94 @@
|
||||
import { getChangedValues } from '@actual-app/core/shared/util';
|
||||
import type { TransactionEntity } from '@actual-app/core/types/models';
|
||||
|
||||
import {
|
||||
getClearedFieldNames,
|
||||
isFieldClearedByUser,
|
||||
shouldApplyRuleChange,
|
||||
} from './table/utils';
|
||||
import type { ClearedFieldsByTransaction } from './table/utils';
|
||||
|
||||
/**
|
||||
* Merges the result of a rules run back into a transaction that is being
|
||||
* entered, respecting the user's manual input.
|
||||
*
|
||||
* Rules may fill fields the user left empty (plus notes append/prepend
|
||||
* merges; see `shouldApplyRuleChange`), but never a field the user explicitly
|
||||
* cleared. Cleared fields are tracked per transaction row, so clearing e.g.
|
||||
* the category on one split row blocks rules only on that row, not on its
|
||||
* siblings or the parent.
|
||||
*/
|
||||
export function applyRulesToTransaction(
|
||||
transaction: TransactionEntity,
|
||||
afterRules: TransactionEntity,
|
||||
updatedFieldName: string | null,
|
||||
clearedFields?: ClearedFieldsByTransaction,
|
||||
): TransactionEntity {
|
||||
const diff = getChangedValues(transaction, afterRules);
|
||||
const newTransaction: TransactionEntity = { ...transaction };
|
||||
|
||||
if (!diff) {
|
||||
return newTransaction;
|
||||
}
|
||||
|
||||
const clearedOnThisRow = getClearedFieldNames(clearedFields, transaction.id);
|
||||
Object.keys(diff).forEach(field => {
|
||||
if (
|
||||
shouldApplyRuleChange(
|
||||
field,
|
||||
Reflect.get(newTransaction, field),
|
||||
Reflect.get(diff, field),
|
||||
clearedOnThisRow,
|
||||
)
|
||||
) {
|
||||
Reflect.set(newTransaction, field, Reflect.get(diff, field));
|
||||
}
|
||||
});
|
||||
|
||||
const subtransactions = propagateRuleChangeToSubtransactions(
|
||||
newTransaction,
|
||||
diff.subtransactions,
|
||||
updatedFieldName,
|
||||
clearedFields,
|
||||
);
|
||||
if (subtransactions) {
|
||||
newTransaction.subtransactions = subtransactions;
|
||||
}
|
||||
|
||||
return newTransaction;
|
||||
}
|
||||
|
||||
/**
|
||||
* When a rule updates a parent transaction, pushes the edited field's rule
|
||||
* value down to its subtransactions — but never into a row where the user
|
||||
* explicitly cleared that field, and not at all if they cleared it on the
|
||||
* parent itself. Returns the updated subtransactions, or null when there is
|
||||
* nothing to propagate.
|
||||
*/
|
||||
export function propagateRuleChangeToSubtransactions(
|
||||
parent: TransactionEntity,
|
||||
ruleSubtransactions: TransactionEntity[] | undefined,
|
||||
updatedFieldName: string | null,
|
||||
clearedFields?: ClearedFieldsByTransaction,
|
||||
): TransactionEntity[] | null {
|
||||
if (
|
||||
!parent.is_parent ||
|
||||
ruleSubtransactions === undefined ||
|
||||
updatedFieldName === null ||
|
||||
isFieldClearedByUser(clearedFields, parent.id, updatedFieldName)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return ruleSubtransactions.map((st, idx) => {
|
||||
const base = parent.subtransactions?.[idx] ?? st;
|
||||
if (isFieldClearedByUser(clearedFields, base.id, updatedFieldName)) {
|
||||
return base;
|
||||
}
|
||||
const ruleValue = Reflect.get(st, updatedFieldName);
|
||||
if (ruleValue == null) {
|
||||
return base;
|
||||
}
|
||||
return { ...base, [updatedFieldName]: ruleValue };
|
||||
});
|
||||
}
|
||||
@@ -65,6 +65,29 @@ describe('shouldApplyRuleChange', () => {
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test('keeps a field empty when the user explicitly cleared it', () => {
|
||||
// Clearing a pre-assigned value (e.g. a category learned from the payee)
|
||||
// is manual input too — rules must not re-fill it, neither on the run for
|
||||
// the clear itself nor on later runs triggered by other field edits.
|
||||
expect(shouldApplyRuleChange('category', null, 'food', ['category'])).toBe(
|
||||
false,
|
||||
);
|
||||
expect(shouldApplyRuleChange('notes', '', 'memo', ['notes'])).toBe(false);
|
||||
});
|
||||
|
||||
test('still fills empty fields the user has not explicitly cleared', () => {
|
||||
expect(shouldApplyRuleChange('category', null, 'food', [])).toBe(true);
|
||||
expect(shouldApplyRuleChange('category', null, 'food', ['notes'])).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test('still applies notes append rules while other fields are cleared', () => {
|
||||
expect(
|
||||
shouldApplyRuleChange('notes', 'Coffee', 'Coffee Tip', ['category']),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deserializeTransaction', () => {
|
||||
|
||||
@@ -118,24 +118,33 @@ export function selectAscDesc(
|
||||
: defaultAscDesc;
|
||||
}
|
||||
|
||||
// An "empty" value from the perspective of rules: rules are only allowed to
|
||||
// fill fields the user left empty.
|
||||
export function isEmptyRuleTarget(value: unknown) {
|
||||
return value == null || value === '' || value === 0 || value === false;
|
||||
}
|
||||
|
||||
// Decides whether a rule result should be applied to a field while the user is
|
||||
// entering a new transaction. By default rules only fill fields the user left
|
||||
// empty, so their manual input isn't overwritten. The exception is the notes
|
||||
// field: append/prepend notes rules intentionally preserve the existing note and
|
||||
// add text before or after it, so we allow those through. The check stays
|
||||
// idempotent — rules are re-run on every keystroke during entry, so we must not
|
||||
// re-add text that the previous run already applied.
|
||||
// empty, so their manual input isn't overwritten. Fields the user explicitly
|
||||
// emptied (`clearedFieldNames`) also stay empty — clearing a pre-assigned
|
||||
// value is manual input too, so rules must not re-fill it on this or any later
|
||||
// run while the transaction is still being entered. The exception is the notes
|
||||
// field: append/prepend notes rules intentionally preserve the existing note
|
||||
// and add text before or after it, so we allow those through. The check stays
|
||||
// idempotent — rules are re-run on every keystroke during entry, so we must
|
||||
// not re-add text that the previous run already applied.
|
||||
export function shouldApplyRuleChange(
|
||||
field: string,
|
||||
currentValue: unknown,
|
||||
nextValue: unknown,
|
||||
clearedFieldNames: readonly string[] = [],
|
||||
) {
|
||||
if (
|
||||
currentValue == null ||
|
||||
currentValue === '' ||
|
||||
currentValue === 0 ||
|
||||
currentValue === false
|
||||
) {
|
||||
if (clearedFieldNames.includes(field)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (isEmptyRuleTarget(currentValue)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -167,6 +176,51 @@ export function shouldApplyRuleChange(
|
||||
return !alreadyApplied;
|
||||
}
|
||||
|
||||
// Which fields the user explicitly emptied, tracked per transaction row so
|
||||
// that clearing a field on one split row doesn't stop rules from filling the
|
||||
// same field on its siblings.
|
||||
export type ClearedFieldsByTransaction = ReadonlyMap<
|
||||
TransactionEntity['id'],
|
||||
ReadonlySet<string>
|
||||
>;
|
||||
|
||||
// Records whether the user's edit left `fieldName` empty on this row. An empty
|
||||
// value marks the field as explicitly cleared; a real value removes the mark.
|
||||
export function trackClearedField(
|
||||
clearedFields: Map<TransactionEntity['id'], Set<string>>,
|
||||
transaction: TransactionEntity,
|
||||
fieldName: string,
|
||||
) {
|
||||
if (isEmptyRuleTarget(Reflect.get(transaction, fieldName))) {
|
||||
const fields = clearedFields.get(transaction.id) ?? new Set<string>();
|
||||
fields.add(fieldName);
|
||||
clearedFields.set(transaction.id, fields);
|
||||
} else {
|
||||
clearedFields.get(transaction.id)?.delete(fieldName);
|
||||
}
|
||||
}
|
||||
|
||||
export function getClearedFieldNames(
|
||||
clearedFields: ClearedFieldsByTransaction | undefined,
|
||||
transactionId: TransactionEntity['id'] | undefined,
|
||||
): readonly string[] {
|
||||
if (clearedFields == null || transactionId == null) {
|
||||
return [];
|
||||
}
|
||||
return [...(clearedFields.get(transactionId) ?? [])];
|
||||
}
|
||||
|
||||
export function isFieldClearedByUser(
|
||||
clearedFields: ClearedFieldsByTransaction | undefined,
|
||||
transactionId: TransactionEntity['id'] | undefined,
|
||||
fieldName: string,
|
||||
) {
|
||||
return (
|
||||
transactionId != null &&
|
||||
(clearedFields?.get(transactionId)?.has(fieldName) ?? false)
|
||||
);
|
||||
}
|
||||
|
||||
export function makeTemporaryTransactions(
|
||||
currentAccountId: AccountEntity['id'] | null | undefined,
|
||||
currentCategoryId: CategoryEntity['id'] | null | undefined,
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
category: Bugfixes
|
||||
authors: [bsmarsh333]
|
||||
---
|
||||
|
||||
Keep explicitly cleared fields (e.g. a pre-assigned category) empty while entering a new transaction instead of letting rules immediately re-fill them.
|
||||
Reference in New Issue
Block a user