From 2bb7b3c2eec5b6d6b86a78f82218ef13049d4460 Mon Sep 17 00:00:00 2001 From: Julian Dominguez-Schatz <julian.dominguezschatz@gmail.com> Date: Thu, 8 Feb 2024 09:56:30 -0500 Subject: [PATCH] Add rules with splits (#2059) * Add split creation UI to rule creation modal * Support applying splits when rules execute * fix: deserialize transaction before running rules According to how rules are run in other places in the app, we should be supplying a "deserialized" (i.e., integer-for-amount and ISO date) transaction rather than a "serialized" (amount-plus-formatted-date) one. This fixes a crash in how split transactions are applied, as well as date-based rules not applying correctly previously (any rule with a date condition would never match on mobile). * Add release notes * Fix missing types pulled in from master * PR feedback: use `getActions` * PR feedback: use `flatMap` * Fix action deletion * Don't flicker upon split deletion * Let users specify parent transaction actions (e.g. linking schedules) * Support empty splits * Revert adding `no-op` action type * Support splits by percent * Fix types * Fix crash on transactions page when posting a transaction The crash would probably have occurred in other places too with auto-posting schedules :/ * Fix a bug where schedules wouldn't be marked as completed This was because the query that we previously used didn't select parent transactions, so no transaction was marked as being scheduled (since only parent transactions have schedule IDs). * Add feature flag * Limit set actions within splits to fewer fields * Fix merge conflict * Don't run split rules if feature is disabled * Fix percent-based splits not applying * Fix crash when editing parent transaction amount * Auto-format * Attempt to fix failing tests * More test/bug fixes * Add an extra split at the end if there is a remaining amount * Make sure split has correct values for dynamic remainder * Remove extraneous console.log --- .../src/components/modals/EditRule.jsx | 355 ++++++++++++++---- .../src/components/settings/Experimental.tsx | 1 + .../src/components/spreadsheet/CellValue.tsx | 4 +- .../src/components/spreadsheet/useFormat.ts | 10 +- .../desktop-client/src/components/table.tsx | 4 +- .../transactions/MobileTransaction.jsx | 16 +- .../transactions/TransactionsTable.jsx | 30 +- .../src/hooks/useFeatureFlag.ts | 1 + .../loot-core/src/server/accounts/rules.ts | 175 ++++++++- .../loot-core/src/server/accounts/sync.ts | 12 +- .../src/server/accounts/transaction-rules.ts | 14 +- packages/loot-core/src/server/rules/app.ts | 4 +- .../src/server/rules/types/handlers.ts | 2 +- packages/loot-core/src/shared/rules.ts | 23 ++ packages/loot-core/src/shared/schedules.ts | 2 +- packages/loot-core/src/shared/transactions.ts | 2 +- packages/loot-core/src/types/models/rule.d.ts | 13 +- packages/loot-core/src/types/prefs.d.ts | 3 +- upcoming-release-notes/2059.md | 6 + 19 files changed, 565 insertions(+), 112 deletions(-) create mode 100644 upcoming-release-notes/2059.md diff --git a/packages/desktop-client/src/components/modals/EditRule.jsx b/packages/desktop-client/src/components/modals/EditRule.jsx index 4d42814e5..e7ebb2094 100644 --- a/packages/desktop-client/src/components/modals/EditRule.jsx +++ b/packages/desktop-client/src/components/modals/EditRule.jsx @@ -1,6 +1,8 @@ import React, { useState, useEffect, useRef, useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { v4 as uuid } from 'uuid'; + import { initiallyLoadPayees, setUndoEnabled, @@ -26,10 +28,11 @@ import { amountToInteger, } from 'loot-core/src/shared/util'; +import { useFeatureFlag } from '../../hooks/useFeatureFlag'; import { useSelected, SelectedProvider } from '../../hooks/useSelected'; -import { SvgAdd, SvgSubtract } from '../../icons/v0'; +import { SvgDelete, SvgAdd, SvgSubtract } from '../../icons/v0'; import { SvgInformationOutline } from '../../icons/v1'; -import { theme } from '../../style'; +import { styles, theme } from '../../style'; import { Button } from '../common/Button'; import { Modal } from '../common/Modal'; import { Select } from '../common/Select'; @@ -122,6 +125,19 @@ export function OpSelect({ ); } +function SplitAmountMethodSelect({ options, style, value, onChange }) { + return ( + <View style={{ color: theme.pageTextPositive, ...style }}> + <Select + bare + options={options} + value={value} + onChange={value => onChange('method', value)} + /> + </View> + ); +} + function EditorButtons({ onAdd, onDelete }) { return ( <> @@ -310,8 +326,25 @@ const actionFields = [ 'date', 'amount', ].map(field => [field, mapField(field)]); +const parentOnlyFields = ['amount', 'cleared', 'account', 'date']; +const splitActionFields = actionFields.filter( + ([field]) => !parentOnlyFields.includes(field), +); +const splitAmountTypes = [ + ['fixed-amount', 'a fixed amount'], + ['fixed-percent', 'a fixed percentage'], + ['remainder', 'an equal portion of the remainder'], +]; function ActionEditor({ action, editorStyle, onChange, onDelete, onAdd }) { - const { field, op, value, type, error, inputKey = 'initial' } = action; + const { + field, + op, + value, + type, + error, + inputKey = 'initial', + options, + } = action; return ( <Editor style={editorStyle} error={error}> @@ -324,7 +357,7 @@ function ActionEditor({ action, editorStyle, onChange, onDelete, onAdd }) { </View> <FieldSelect - fields={actionFields} + fields={options?.splitIndex ? splitActionFields : actionFields} value={field} onChange={onChange} /> @@ -340,6 +373,30 @@ function ActionEditor({ action, editorStyle, onChange, onDelete, onAdd }) { /> </View> </> + ) : op === 'set-split-amount' ? ( + <> + <View style={{ padding: '5px 10px', lineHeight: '1em' }}> + allocate + </View> + + <SplitAmountMethodSelect + options={splitAmountTypes} + value={options.method} + onChange={onChange} + /> + + <View style={{ flex: 1 }}> + {options.method !== 'remainder' && ( + <GenericInput + key={inputKey} + field={field} + type="number" + value={value} + onChange={v => onChange('value', v)} + /> + )} + </View> + </> ) : op === 'link-schedule' ? ( <> <View @@ -355,10 +412,7 @@ function ActionEditor({ action, editorStyle, onChange, onDelete, onAdd }) { ) : null} <Stack direction="row"> - <EditorButtons - onAdd={onAdd} - onDelete={op !== 'link-schedule' && onDelete} - /> + <EditorButtons onAdd={onAdd} onDelete={op === 'set' && onDelete} /> </Stack> </Editor> ); @@ -587,6 +641,9 @@ function ConditionsList({ ); } +const getActions = splits => splits.flatMap(s => s.actions); +const getUnparsedActions = splits => getActions(splits).map(unparse); + // TODO: // * Dont touch child transactions? @@ -606,17 +663,32 @@ const conditionFields = [ ]); export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { + const splitsEnabled = useFeatureFlag('splitsInRules'); const [conditions, setConditions] = useState( defaultRule.conditions.map(parse), ); - const [actions, setActions] = useState(defaultRule.actions.map(parse)); + const [actionSplits, setActionSplits] = useState(() => { + const parsedActions = defaultRule.actions.map(parse); + return parsedActions.reduce( + (acc, action) => { + const splitIndex = action.options?.splitIndex ?? 0; + acc[splitIndex] = acc[splitIndex] ?? { id: uuid(), actions: [] }; + acc[splitIndex].actions.push(action); + return acc; + }, + // The pre-split group is always there + [{ id: uuid(), actions: [] }], + ); + }); const [stage, setStage] = useState(defaultRule.stage); const [conditionsOp, setConditionsOp] = useState(defaultRule.conditionsOp); const [transactions, setTransactions] = useState([]); const dispatch = useDispatch(); const scrollableEl = useRef(); - const isSchedule = actions.some(action => action.op === 'link-schedule'); + const isSchedule = getActions(actionSplits).some( + action => action.op === 'link-schedule', + ); useEffect(() => { dispatch(initiallyLoadPayees()); @@ -643,9 +715,11 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { if (filters.length > 0) { const conditionsOpKey = conditionsOp === 'or' ? '$or' : '$and'; + const parentOnlyCondition = + actionSplits.length > 1 ? { is_child: false } : {}; const { data: transactions } = await runQuery( q('transactions') - .filter({ [conditionsOpKey]: filters }) + .filter({ [conditionsOpKey]: filters, ...parentOnlyCondition }) .select('*'), ); setTransactions(transactions); @@ -654,49 +728,71 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { } } run(); - }, [actions, conditions, conditionsOp]); + }, [actionSplits, conditions, conditionsOp]); const selectedInst = useSelected('transactions', transactions, []); function addInitialAction() { - addAction(-1); + addActionToSplitAfterIndex(0, -1); } - function addAction(index) { - let fields = actionFields.map(f => f[0]); - for (const action of actions) { - fields = fields.filter(f => f !== action.field); + function addActionToSplitAfterIndex(splitIndex, actionIndex) { + let newAction; + if (splitIndex && !actionSplits[splitIndex]?.actions?.length) { + actionSplits[splitIndex] = { id: uuid(), actions: [] }; + newAction = { + op: 'set-split-amount', + options: { method: 'remainder', splitIndex }, + value: null, + }; + } else { + const fieldsArray = splitIndex === 0 ? actionFields : splitActionFields; + let fields = fieldsArray.map(f => f[0]); + for (const action of actionSplits[splitIndex].actions) { + fields = fields.filter(f => f !== action.field); + } + const field = fields[0] || 'category'; + newAction = { + type: FIELD_TYPES.get(field), + field, + op: 'set', + value: null, + options: { splitIndex }, + }; } - const field = fields[0] || 'category'; - const copy = [...actions]; - copy.splice(index + 1, 0, { - type: FIELD_TYPES.get(field), - field, - op: 'set', - value: null, - }); - setActions(copy); + const actionsCopy = [...actionSplits[splitIndex].actions]; + actionsCopy.splice(actionIndex + 1, 0, newAction); + const copy = [...actionSplits]; + copy[splitIndex] = { ...actionSplits[splitIndex], actions: actionsCopy }; + setActionSplits(copy); } function onChangeAction(action, field, value) { - setActions( - updateValue(actions, action, () => { - const a = { ...action }; - a[field] = value; - - if (field === 'field') { - a.type = FIELD_TYPES.get(a.field); - a.value = null; - return newInput(a); - } else if (field === 'op') { - a.value = null; - a.inputKey = '' + Math.random(); - return newInput(a); - } + setActionSplits( + actionSplits.map(({ id, actions }) => ({ + id, + actions: updateValue(actions, action, () => { + const a = { ...action }; + if (field === 'method') { + a.options = { ...a.options, method: value }; + } else { + a[field] = value; + + if (field === 'field') { + a.type = FIELD_TYPES.get(a.field); + a.value = null; + return newInput(a); + } else if (field === 'op') { + a.value = null; + a.inputKey = '' + Math.random(); + return newInput(a); + } + } - return a; - }), + return a; + }), + })), ); } @@ -709,16 +805,51 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { } function onRemoveAction(action) { - setActions(actions.filter(a => a !== action)); + setActionSplits(splits => + splits.map(({ id, actions }) => ({ + id, + actions: actions.filter(a => a !== action), + })), + ); + } + + function onRemoveSplit(splitIndexToRemove) { + setActionSplits(splits => { + const copy = []; + splits.forEach(({ id }, index) => { + if (index === splitIndexToRemove) { + return; + } + copy.push({ id, actions: [] }); + }); + getActions(splits).forEach(action => { + const currentSplitIndex = action.options?.splitIndex ?? 0; + if (currentSplitIndex === splitIndexToRemove) { + return; + } + const newSplitIndex = + currentSplitIndex > splitIndexToRemove + ? currentSplitIndex - 1 + : currentSplitIndex; + copy[newSplitIndex].actions.push({ + ...action, + options: { ...action.options, splitIndex: newSplitIndex }, + }); + }); + return copy; + }); } function onApply() { + const selectedTransactions = transactions.filter(({ id }) => + selectedInst.items.has(id), + ); send('rule-apply-actions', { - transactionIds: [...selectedInst.items], - actions: actions.map(unparse), + transactions: selectedTransactions, + actions: getUnparsedActions(actionSplits), }).then(() => { // This makes it refetch the transactions - setActions([...actions]); + setActionSplits([...actionSplits]); }); } @@ -728,7 +859,7 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { stage, conditionsOp, conditions: conditions.map(unparse), - actions: actions.map(unparse), + actions: getUnparsedActions(actionSplits), }; const method = rule.id ? 'rule-update' : 'rule-add'; @@ -740,7 +871,7 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { } if (error.actionErrors) { - setActions(applyErrors(actions, error.actionErrors)); + setActionSplits(applyErrors(actionSplits, error.actionErrors)); } } else { // If adding a rule, we got back an id @@ -759,6 +890,11 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { borderRadius: 4, }; + // Enable editing existing split rules even if the feature has since been disabled. + const showSplitButton = splitsEnabled + ? actionSplits.length > 0 + : actionSplits.length > 1; + return ( <Modal title="Rule" @@ -852,30 +988,112 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { Then apply these actions: </Text> <View style={{ flex: 1 }}> - {actions.length === 0 ? ( + {actionSplits.length === 0 && ( <Button style={{ alignSelf: 'flex-start' }} onClick={addInitialAction} > Add action </Button> - ) : ( - <Stack spacing={2} data-testid="action-list"> - {actions.map((action, i) => ( - <View key={i}> - <ActionEditor - ops={['set', 'link-schedule']} - action={action} - editorStyle={editorStyle} - onChange={(name, value) => { - onChangeAction(action, name, value); - }} - onDelete={() => onRemoveAction(action)} - onAdd={() => addAction(i)} - /> - </View> - ))} - </Stack> + )} + <Stack spacing={2} data-testid="action-split-list"> + {actionSplits.map(({ id, actions }, splitIndex) => ( + <View + key={id} + nativeStyle={ + actionSplits.length > 1 + ? { + borderColor: theme.tableBorder, + borderWidth: '1px', + borderRadius: '5px', + padding: '5px', + } + : {} + } + > + {actionSplits.length > 1 && ( + <Stack + direction="row" + justify="space-between" + spacing={1} + > + <Text + style={{ + ...styles.verySmallText, + marginBottom: '10px', + }} + > + {splitIndex === 0 + ? 'Before split' + : `Split ${splitIndex}`} + </Text> + {splitIndex && ( + <Button + type="bare" + onClick={() => onRemoveSplit(splitIndex)} + style={{ + width: 20, + height: 20, + }} + aria-label="Delete split" + > + <SvgDelete + style={{ + width: 8, + height: 8, + color: 'inherit', + }} + /> + </Button> + )} + </Stack> + )} + <Stack spacing={2} data-testid="action-list"> + {actions.map((action, actionIndex) => ( + <View key={actionIndex}> + <ActionEditor + ops={['set', 'link-schedule']} + action={action} + editorStyle={editorStyle} + onChange={(name, value) => { + onChangeAction(action, name, value); + }} + onDelete={() => onRemoveAction(action)} + onAdd={() => + addActionToSplitAfterIndex( + splitIndex, + actionIndex, + ) + } + /> + </View> + ))} + </Stack> + + {actions.length === 0 && ( + <Button + style={{ alignSelf: 'flex-start', marginTop: 5 }} + onClick={() => + addActionToSplitAfterIndex(splitIndex, -1) + } + > + Add action + </Button> + )} + </View> + ))} + </Stack> + {showSplitButton && ( + <Button + style={{ alignSelf: 'flex-start', marginTop: 15 }} + onClick={() => { + addActionToSplitAfterIndex(actionSplits.length, -1); + }} + > + {actionSplits.length > 1 + ? 'Add another split' + : 'Split into multiple transactions'} + </Button> )} </View> </View> @@ -905,7 +1123,10 @@ export function EditRule({ modalProps, defaultRule, onSave: originalOnSave }) { <SimpleTransactionsTable transactions={transactions} - fields={getTransactionFields(conditions, actions)} + fields={getTransactionFields( + conditions, + getActions(actionSplits), + )} style={{ border: '1px solid ' + theme.tableBorder, borderRadius: '6px 6px 0 0', diff --git a/packages/desktop-client/src/components/settings/Experimental.tsx b/packages/desktop-client/src/components/settings/Experimental.tsx index d58b9b623..094af8624 100644 --- a/packages/desktop-client/src/components/settings/Experimental.tsx +++ b/packages/desktop-client/src/components/settings/Experimental.tsx @@ -99,6 +99,7 @@ export function ExperimentalFeatures() { Goal templates </FeatureToggle> <FeatureToggle flag="simpleFinSync">SimpleFIN sync</FeatureToggle> + <FeatureToggle flag="splitsInRules">Splits in rules</FeatureToggle> </View> ) : ( <LinkButton diff --git a/packages/desktop-client/src/components/spreadsheet/CellValue.tsx b/packages/desktop-client/src/components/spreadsheet/CellValue.tsx index 0c453a974..7a298c3ff 100644 --- a/packages/desktop-client/src/components/spreadsheet/CellValue.tsx +++ b/packages/desktop-client/src/components/spreadsheet/CellValue.tsx @@ -5,7 +5,7 @@ import { type CSSProperties, styles } from '../../style'; import { Text } from '../common/Text'; import { ConditionalPrivacyFilter } from '../PrivacyFilter'; -import { useFormat } from './useFormat'; +import { type FormatType, useFormat } from './useFormat'; import { useSheetName } from './useSheetName'; import { useSheetValue } from './useSheetValue'; @@ -13,7 +13,7 @@ import { type Binding } from '.'; type CellValueProps = { binding: string | Binding; - type?: string; + type?: FormatType; formatter?: (value) => ReactNode; style?: CSSProperties; getStyle?: (value) => CSSProperties; diff --git a/packages/desktop-client/src/components/spreadsheet/useFormat.ts b/packages/desktop-client/src/components/spreadsheet/useFormat.ts index 28d0c1f0b..d1599dd6a 100644 --- a/packages/desktop-client/src/components/spreadsheet/useFormat.ts +++ b/packages/desktop-client/src/components/spreadsheet/useFormat.ts @@ -4,9 +4,15 @@ import { useSelector } from 'react-redux'; import { selectNumberFormat } from 'loot-core/src/client/selectors'; import { integerToCurrency } from 'loot-core/src/shared/util'; +export type FormatType = + | 'string' + | 'number' + | 'financial' + | 'financial-with-sign'; + function format( value: unknown, - type = 'string', + type: FormatType = 'string', formatter?: Intl.NumberFormat, ): string { switch (type) { @@ -49,7 +55,7 @@ export function useFormat() { const numberFormat = useSelector(selectNumberFormat); return useCallback( - (value: unknown, type = 'string') => + (value: unknown, type: FormatType = 'string') => format(value, type, numberFormat.formatter), [numberFormat], ); diff --git a/packages/desktop-client/src/components/table.tsx b/packages/desktop-client/src/components/table.tsx index d1c98ec48..5a51dd0f9 100644 --- a/packages/desktop-client/src/components/table.tsx +++ b/packages/desktop-client/src/components/table.tsx @@ -40,7 +40,7 @@ import { mergeConditionalPrivacyFilterProps, } from './PrivacyFilter'; import { type Binding } from './spreadsheet'; -import { useFormat } from './spreadsheet/useFormat'; +import { type FormatType, useFormat } from './spreadsheet/useFormat'; import { useSheetValue } from './spreadsheet/useSheetValue'; import { Tooltip, IntersectionBoundary } from './tooltips'; @@ -660,7 +660,7 @@ export function SelectCell({ type SheetCellValueProps = { binding: Binding; - type: string; + type: FormatType; getValueStyle?: (value: string | number) => CSSProperties; formatExpr?: (value) => string; unformatExpr?: (value: string) => unknown; diff --git a/packages/desktop-client/src/components/transactions/MobileTransaction.jsx b/packages/desktop-client/src/components/transactions/MobileTransaction.jsx index aa939b539..bda688058 100644 --- a/packages/desktop-client/src/components/transactions/MobileTransaction.jsx +++ b/packages/desktop-client/src/components/transactions/MobileTransaction.jsx @@ -992,19 +992,23 @@ function TransactionEditUnconnected(props) { return null; } - const onEdit = async transaction => { - let newTransaction = transaction; + const onEdit = async serializedTransaction => { + const transaction = deserializeTransaction( + serializedTransaction, + null, + dateFormat, + ); + // Run the rules to auto-fill in any data. Right now we only do // this on new transactions because that's how desktop works. if (isTemporary(transaction)) { const afterRules = await send('rules-run', { transaction }); const diff = getChangedValues(transaction, afterRules); - newTransaction = { ...transaction }; if (diff) { Object.keys(diff).forEach(field => { - if (newTransaction[field] == null) { - newTransaction[field] = diff[field]; + if (transaction[field] == null) { + transaction[field] = diff[field]; } }); } @@ -1012,7 +1016,7 @@ function TransactionEditUnconnected(props) { const { data: newTransactions } = updateTransaction( transactions, - deserializeTransaction(newTransaction, null, dateFormat), + transaction, ); setTransactions(newTransactions); }; diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx index 0472fa482..c99d1d759 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx @@ -35,6 +35,8 @@ import { updateTransaction, deleteTransaction, addSplitTransaction, + groupTransaction, + ungroupTransactions, } from 'loot-core/src/shared/transactions'; import { integerToCurrency, @@ -705,6 +707,7 @@ function PayeeIcons({ const Transaction = memo(function Transaction(props) { const { transaction: originalTransaction, + subtransactions, editing, showAccount, showBalance, @@ -843,7 +846,8 @@ const Transaction = memo(function Transaction(props) { // Run the transaction through the formatting so that we know // it's always showing the formatted result setTransaction(serializeTransaction(deserialized, showZeroInDeposit)); - onSave(deserialized); + + onSave(deserialized, subtransactions); } } @@ -1489,9 +1493,10 @@ function NewTransaction({ const error = transactions[0].error; const isDeposit = transactions[0].amount > 0; - const emptyChildTransactions = transactions.filter( - t => t.parent_id === transactions[0].id && t.amount === 0, + const childTransactions = transactions.filter( + t => t.parent_id === transactions[0].id, ); + const emptyChildTransactions = childTransactions.filter(t => t.amount === 0); return ( <View @@ -1513,6 +1518,7 @@ function NewTransaction({ key={transaction.id} editing={editingTransaction === transaction.id} transaction={transaction} + subtransactions={transaction.is_parent ? childTransactions : null} showAccount={showAccount} showCategory={showCategory} showBalance={showBalance} @@ -2064,18 +2070,28 @@ export const TransactionTable = forwardRef((props, ref) => { }, [props.onAdd, newNavigator.onEdit]); const onSave = useCallback( - async transaction => { + async (transaction, subtransactions = null) => { savePending.current = true; + let groupedTransaction = subtransactions + ? groupTransaction([transaction, ...subtransactions]) + : transaction; + if (isTemporaryId(transaction.id)) { if (props.onApplyRules) { - transaction = await props.onApplyRules(transaction); + groupedTransaction = await props.onApplyRules(groupedTransaction); } const newTrans = latestState.current.newTransactions; - setNewTransactions(updateTransaction(newTrans, transaction).data); + // Future refactor: we shouldn't need to iterate through the entire + // transaction list to ungroup, just the new transactions. + setNewTransactions( + ungroupTransactions( + updateTransaction(newTrans, groupedTransaction).data, + ), + ); } else { - props.onSave(transaction); + props.onSave(groupedTransaction); } }, [props.onSave], diff --git a/packages/desktop-client/src/hooks/useFeatureFlag.ts b/packages/desktop-client/src/hooks/useFeatureFlag.ts index 4ba88ede7..1395862d7 100644 --- a/packages/desktop-client/src/hooks/useFeatureFlag.ts +++ b/packages/desktop-client/src/hooks/useFeatureFlag.ts @@ -12,6 +12,7 @@ const DEFAULT_FEATURE_FLAG_STATE: Record<FeatureFlag, boolean> = { goalTemplatesEnabled: false, customReports: false, simpleFinSync: false, + splitsInRules: false, }; export function useFeatureFlag(name: FeatureFlag): boolean { diff --git a/packages/loot-core/src/server/accounts/rules.ts b/packages/loot-core/src/server/accounts/rules.ts index 96b33b525..68f0d67bf 100644 --- a/packages/loot-core/src/server/accounts/rules.ts +++ b/packages/loot-core/src/server/accounts/rules.ts @@ -12,9 +12,16 @@ import { } from '../../shared/months'; import { sortNumbers, getApproxNumberThreshold } from '../../shared/rules'; import { recurConfigToRSchedule } from '../../shared/schedules'; +import { + addSplitTransaction, + recalculateSplit, + splitTransaction, + ungroupTransaction, +} from '../../shared/transactions'; import { fastSetMerge } from '../../shared/util'; import { RuleConditionEntity } from '../../types/models'; import { RuleError } from '../errors'; +import * as prefs from '../prefs'; import { Schedule as RSchedule } from '../util/rschedule'; function assert(test, type, msg) { @@ -418,9 +425,8 @@ export class Condition { } } -type ActionOperator = 'set' | 'link-schedule'; - -const ACTION_OPS: ActionOperator[] = ['set', 'link-schedule']; +const ACTION_OPS = ['set', 'set-split-amount', 'link-schedule'] as const; +type ActionOperator = (typeof ACTION_OPS)[number]; export class Action { field; @@ -442,6 +448,9 @@ export class Action { assert(typeName, 'internal', `Invalid field for action: ${field}`); this.field = field; this.type = typeName; + } else if (op === 'set-split-amount') { + this.field = null; + this.type = 'number'; } else if (op === 'link-schedule') { this.field = null; this.type = 'id'; @@ -458,6 +467,14 @@ export class Action { case 'set': object[this.field] = this.value; break; + case 'set-split-amount': + switch (this.options.method) { + case 'fixed-amount': + object.amount = this.value; + break; + default: + } + break; case 'link-schedule': object.schedule = this.value; break; @@ -476,6 +493,142 @@ export class Action { } } +function execNonSplitActions(actions: Action[], transaction) { + const update = transaction; + actions.forEach(action => action.exec(update)); + return update; +} + +export function execActions(actions: Action[], transaction) { + const parentActions = actions.filter(action => !action.options?.splitIndex); + const childActions = actions.filter(action => action.options?.splitIndex); + const totalSplitCount = + actions.reduce( + (prev, cur) => Math.max(prev, cur.options?.splitIndex ?? 0), + 0, + ) + 1; + + let update = execNonSplitActions(parentActions, transaction); + if (!prefs.getPrefs()?.['flags.splitsInRules'] || totalSplitCount === 1) { + return update; + } + + if (update.is_child) { + // Rules with splits can't be applied to child transactions. + return update; + } + + const splitAmountActions = childActions.filter( + action => action.op === 'set-split-amount', + ); + const fixedSplitAmountActions = splitAmountActions.filter( + action => action.options.method === 'fixed-amount', + ); + const fixedAmountsBySplit: Record<number, number> = {}; + fixedSplitAmountActions.forEach(action => { + const splitIndex = action.options.splitIndex ?? 0; + fixedAmountsBySplit[splitIndex] = action.value; + }); + const fixedAmountSplitCount = Object.keys(fixedAmountsBySplit).length; + const totalFixedAmount = Object.values(fixedAmountsBySplit).reduce<number>( + (prev, cur: number) => prev + cur, + 0, + ); + if ( + fixedAmountSplitCount === totalSplitCount && + totalFixedAmount !== (transaction.amount ?? totalFixedAmount) + ) { + // Not all value would be distributed to a split. + return transaction; + } + + const { data, newTransaction } = splitTransaction( + ungroupTransaction(update), + transaction.id, + ); + update = recalculateSplit(newTransaction); + data[0] = update; + let newTransactions = data; + + for (const action of childActions) { + const splitIndex = action.options?.splitIndex ?? 0; + if (splitIndex >= update.subtransactions.length) { + const { data, newTransaction } = addSplitTransaction( + newTransactions, + transaction.id, + ); + update = recalculateSplit(newTransaction); + data[0] = update; + newTransactions = data; + } + action.exec(update.subtransactions[splitIndex]); + } + + // Make sure every transaction has an amount. + if (fixedAmountSplitCount !== totalSplitCount) { + // This is the amount that will be distributed to the splits that + // don't have a fixed amount. The last split will get the remainder. + // The amount will be zero if the parent transaction has no amount. + const amountToDistribute = + (transaction.amount ?? totalFixedAmount) - totalFixedAmount; + let remainingAmount = amountToDistribute; + + // First distribute the fixed percentages. + splitAmountActions + .filter(action => action.options.method === 'fixed-percent') + .forEach(action => { + const splitIndex = action.options.splitIndex; + const percent = action.value / 100; + const amount = Math.round(amountToDistribute * percent); + update.subtransactions[splitIndex].amount = amount; + remainingAmount -= amount; + }); + + // Then distribute the remainder. + const remainderSplitAmountActions = splitAmountActions.filter( + action => action.options.method === 'remainder', + ); + + // Check if there is any value left to distribute after all fixed + // (percentage and amount) splits have been distributed. + if (remainingAmount !== 0) { + // If there are no remainder splits explicitly added by the user, + // distribute the remainder to a virtual split that will be + // adjusted for the remainder. + if (remainderSplitAmountActions.length === 0) { + const splitIndex = totalSplitCount; + const { newTransaction } = addSplitTransaction( + newTransactions, + transaction.id, + ); + update = recalculateSplit(newTransaction); + update.subtransactions[splitIndex].amount = remainingAmount; + } else { + const amountPerRemainderSplit = Math.round( + remainingAmount / remainderSplitAmountActions.length, + ); + let lastNonFixedIndex = -1; + remainderSplitAmountActions.forEach(action => { + const splitIndex = action.options.splitIndex; + update.subtransactions[splitIndex].amount = amountPerRemainderSplit; + remainingAmount -= amountPerRemainderSplit; + lastNonFixedIndex = Math.max(lastNonFixedIndex, splitIndex); + }); + + // The last non-fixed split will be adjusted for the remainder. + update.subtransactions[lastNonFixedIndex].amount -= remainingAmount; + } + update = recalculateSplit(update); + } + } + + // The split index 0 is reserved for "Before split" actions. + // Remove that entry from the subtransactions. + update.subtransactions = update.subtransactions.slice(1); + + return update; +} + export class Rule { actions; conditions; @@ -520,15 +673,23 @@ export class Rule { }); } - execActions() { - const changes = {}; - this.actions.forEach(action => action.exec(changes)); + execActions(object) { + const result = execActions(this.actions, { + ...object, + subtransactions: object.subtransactions, + }); + const changes = Object.keys(result).reduce((prev, cur) => { + if (result[cur] !== object[cur]) { + prev[cur] = result[cur]; + } + return prev; + }, {}); return changes; } exec(object) { if (this.evalConditions(object)) { - return this.execActions(); + return this.execActions(object); } return null; } diff --git a/packages/loot-core/src/server/accounts/sync.ts b/packages/loot-core/src/server/accounts/sync.ts index ccf2d09ab..5a3ccfba8 100644 --- a/packages/loot-core/src/server/accounts/sync.ts +++ b/packages/loot-core/src/server/accounts/sync.ts @@ -487,7 +487,7 @@ export async function reconcileExternalTransactions(acctId, transactions) { transactionsStep1.push({ payee_name, trans, - subtransactions, + subtransactions: trans.subtransactions || subtransactions, match, fuzzyDataset, }); @@ -650,7 +650,7 @@ export async function reconcileTransactions(acctId, transactions) { transactionsStep1.push({ payee_name, trans, - subtransactions, + subtransactions: trans.subtransactions || subtransactions, match, fuzzyDataset, }); @@ -783,8 +783,12 @@ export async function addTransactions( }; // Add split transactions if they are given - if (subtransactions && subtransactions.length > 0) { - added.push(...makeSplitTransaction(finalTransaction, subtransactions)); + const updatedSubtransactions = + finalTransaction.subtransactions || subtransactions; + if (updatedSubtransactions && updatedSubtransactions.length > 0) { + added.push( + ...makeSplitTransaction(finalTransaction, updatedSubtransactions), + ); } else { added.push(finalTransaction); } diff --git a/packages/loot-core/src/server/accounts/transaction-rules.ts b/packages/loot-core/src/server/accounts/transaction-rules.ts index a8afebff6..926b6797c 100644 --- a/packages/loot-core/src/server/accounts/transaction-rules.ts +++ b/packages/loot-core/src/server/accounts/transaction-rules.ts @@ -11,6 +11,7 @@ import { sortNumbers, getApproxNumberThreshold, } from '../../shared/rules'; +import { ungroupTransaction } from '../../shared/transactions'; import { partitionByField, fastSetMerge } from '../../shared/util'; import { type TransactionEntity, @@ -32,6 +33,7 @@ import { rankRules, migrateIds, iterateIds, + execActions, } from './rules'; import { batchUpdateTransactions } from './transactions'; @@ -492,8 +494,8 @@ export function conditionsToAQL(conditions, { recurDateBounds = 100 } = {}) { return { filters, errors }; } -export function applyActions( - transactionIds: string[], +export async function applyActions( + transactions: TransactionEntity[], actions: Array<Action | RuleActionEntity>, ) { const parsedActions = actions @@ -526,12 +528,8 @@ export function applyActions( return null; } - const updated = transactionIds.map(id => { - const update = { id }; - for (const action of parsedActions) { - action.exec(update); - } - return update; + const updated = transactions.flatMap(trans => { + return ungroupTransaction(execActions(parsedActions, trans)); }); return batchUpdateTransactions({ updated }); diff --git a/packages/loot-core/src/server/rules/app.ts b/packages/loot-core/src/server/rules/app.ts index 62dd517d4..8fb059c5a 100644 --- a/packages/loot-core/src/server/rules/app.ts +++ b/packages/loot-core/src/server/rules/app.ts @@ -129,8 +129,8 @@ app.method( app.method( 'rule-apply-actions', mutator( - undoable(async function ({ transactionIds, actions }) { - return rules.applyActions(transactionIds, actions); + undoable(async function ({ transactions, actions }) { + return rules.applyActions(transactions, actions); }), ), ); diff --git a/packages/loot-core/src/server/rules/types/handlers.ts b/packages/loot-core/src/server/rules/types/handlers.ts index 95dfce095..429acddc9 100644 --- a/packages/loot-core/src/server/rules/types/handlers.ts +++ b/packages/loot-core/src/server/rules/types/handlers.ts @@ -31,7 +31,7 @@ export interface RulesHandlers { ) => Promise<{ someDeletionsFailed: boolean }>; 'rule-apply-actions': (arg: { - transactionIds: string[]; + transactions: TransactionEntity[]; actions: Array<Action | RuleActionEntity>; }) => Promise<null | { added: TransactionEntity[]; updated: unknown[] }>; diff --git a/packages/loot-core/src/shared/rules.ts b/packages/loot-core/src/shared/rules.ts index b1e40f373..9eb16dade 100644 --- a/packages/loot-core/src/shared/rules.ts +++ b/packages/loot-core/src/shared/rules.ts @@ -159,6 +159,13 @@ export function sortNumbers(num1, num2) { } export function parse(item) { + if (item.op === 'set-split-amount') { + if (item.options.method === 'fixed-amount') { + return { ...item, value: item.value && integerToAmount(item.value) }; + } + return item; + } + switch (item.type) { case 'number': { let parsed = item.value; @@ -186,6 +193,22 @@ export function parse(item) { } export function unparse({ error, inputKey, ...item }) { + if (item.op === 'set-split-amount') { + if (item.options.method === 'fixed-amount') { + return { + ...item, + value: item.value && amountToInteger(item.value), + }; + } + if (item.options.method === 'fixed-percent') { + return { + ...item, + value: item.value && parseFloat(item.value), + }; + } + return item; + } + switch (item.type) { case 'number': { let unparsed = item.value; diff --git a/packages/loot-core/src/shared/schedules.ts b/packages/loot-core/src/shared/schedules.ts index d8628ddfa..174b8102c 100644 --- a/packages/loot-core/src/shared/schedules.ts +++ b/packages/loot-core/src/shared/schedules.ts @@ -43,9 +43,9 @@ export function getHasTransactionsQuery(schedules) { }); return q('transactions') + .options({ splits: 'grouped' }) .filter({ $or: filters }) .orderBy({ date: 'desc' }) - .groupBy('schedule') .select(['schedule', 'date']); } diff --git a/packages/loot-core/src/shared/transactions.ts b/packages/loot-core/src/shared/transactions.ts index b642454e4..532047a22 100644 --- a/packages/loot-core/src/shared/transactions.ts +++ b/packages/loot-core/src/shared/transactions.ts @@ -97,7 +97,7 @@ export function ungroupTransactions(transactions: TransactionEntity[]) { }, []); } -function groupTransaction(split) { +export function groupTransaction(split) { return { ...split[0], subtransactions: split.slice(1) }; } diff --git a/packages/loot-core/src/types/models/rule.d.ts b/packages/loot-core/src/types/models/rule.d.ts index ce2b384f6..66fa53f99 100644 --- a/packages/loot-core/src/types/models/rule.d.ts +++ b/packages/loot-core/src/types/models/rule.d.ts @@ -46,10 +46,21 @@ export interface SetRuleActionEntity { field: string; op: 'set'; value: unknown; - options?: unknown; + options?: { + splitIndex?: number; + }; type?: string; } +export interface SetSplitAmountRuleActionEntity { + op: 'set-split-amount'; + value: number; + options?: { + splitIndex?: number; + method: 'fixed-amount' | 'fixed-percent' | 'remainder'; + }; +} + export interface LinkScheduleRuleActionEntity { op: 'link-schedule'; value: ScheduleEntity; diff --git a/packages/loot-core/src/types/prefs.d.ts b/packages/loot-core/src/types/prefs.d.ts index f483d672e..e924b3290 100644 --- a/packages/loot-core/src/types/prefs.d.ts +++ b/packages/loot-core/src/types/prefs.d.ts @@ -6,7 +6,8 @@ export type FeatureFlag = | 'reportBudget' | 'goalTemplatesEnabled' | 'customReports' - | 'simpleFinSync'; + | 'simpleFinSync' + | 'splitsInRules'; export type LocalPrefs = Partial< { diff --git a/upcoming-release-notes/2059.md b/upcoming-release-notes/2059.md new file mode 100644 index 000000000..1518c16d6 --- /dev/null +++ b/upcoming-release-notes/2059.md @@ -0,0 +1,6 @@ +--- +category: Features +authors: [jfdoming] +--- + +Support automatically splitting transactions with rules -- GitLab