From 9a900f9ff1050b5f13aee244cfd993915071ba71 Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins Date: Fri, 22 Sep 2023 08:36:56 +0100 Subject: [PATCH] :recycle: moving rule server actions to separate file (#1677) --- .../src/components/rules/ActionExpression.tsx | 60 ++++--- .../src/components/rules/RuleRow.tsx | 5 +- .../src/server/accounts/transaction-rules.ts | 21 ++- .../src/server/accounts/transactions.ts | 4 +- packages/loot-core/src/server/errors.ts | 4 +- packages/loot-core/src/server/main.ts | 129 +------------- packages/loot-core/src/server/rules/app.ts | 157 ++++++++++++++++++ .../src/server/rules/types/handlers.ts | 49 ++++++ packages/loot-core/src/types/models/rule.d.ts | 27 ++- .../loot-core/src/types/server-handlers.d.ts | 20 --- upcoming-release-notes/1677.md | 6 + 11 files changed, 293 insertions(+), 189 deletions(-) create mode 100644 packages/loot-core/src/server/rules/app.ts create mode 100644 packages/loot-core/src/server/rules/types/handlers.ts create mode 100644 upcoming-release-notes/1677.md diff --git a/packages/desktop-client/src/components/rules/ActionExpression.tsx b/packages/desktop-client/src/components/rules/ActionExpression.tsx index 0b6cc17877c..a320c79d933 100644 --- a/packages/desktop-client/src/components/rules/ActionExpression.tsx +++ b/packages/desktop-client/src/components/rules/ActionExpression.tsx @@ -1,7 +1,11 @@ import React from 'react'; import { mapField, friendlyOp } from 'loot-core/src/shared/rules'; -import { type ScheduleEntity } from 'loot-core/src/types/models'; +import { + type LinkScheduleRuleActionEntity, + type RuleActionEntity, + type SetRuleActionEntity, +} from 'loot-core/src/types/models'; import { type CSSProperties, theme } from '../../style'; import Text from '../common/Text'; @@ -14,20 +18,13 @@ let valueStyle = { color: theme.pageTextPositive, }; -type ActionExpressionProps = { - field: unknown; - op: unknown; - value: unknown; - options: unknown; +type ActionExpressionProps = RuleActionEntity & { style?: CSSProperties; }; export default function ActionExpression({ - field, - op, - value, - options, style, + ...props }: ActionExpressionProps) { return ( - {op === 'set' ? ( - <> - {friendlyOp(op)}{' '} - {mapField(field, options)}{' '} - to - - - ) : op === 'link-schedule' ? ( - <> - {friendlyOp(op)}{' '} - - + {props.op === 'set' ? ( + + ) : props.op === 'link-schedule' ? ( + ) : null} ); } + +function SetActionExpression({ + op, + field, + value, + options, +}: SetRuleActionEntity) { + return ( + <> + {friendlyOp(op)}{' '} + {mapField(field, options)}{' '} + to + + + ); +} + +function LinkScheduleActionExpression({ + op, + value, +}: LinkScheduleRuleActionEntity) { + return ( + <> + {friendlyOp(op)} + + ); +} diff --git a/packages/desktop-client/src/components/rules/RuleRow.tsx b/packages/desktop-client/src/components/rules/RuleRow.tsx index 9199a68ed3e..037a85cba91 100644 --- a/packages/desktop-client/src/components/rules/RuleRow.tsx +++ b/packages/desktop-client/src/components/rules/RuleRow.tsx @@ -105,10 +105,7 @@ const RuleRow = memo( {rule.actions.map((action, i) => ( ))} diff --git a/packages/loot-core/src/server/accounts/transaction-rules.ts b/packages/loot-core/src/server/accounts/transaction-rules.ts index 56cc6a73ae3..758d7f44bb9 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 { getApproxNumberThreshold, } from '../../shared/rules'; import { partitionByField, fastSetMerge } from '../../shared/util'; +import { type RuleActionEntity, type RuleEntity } from '../../types/models'; import { schemaConfig } from '../aql'; import * as db from '../db'; import { getMappings } from '../db/mappings'; @@ -27,6 +28,7 @@ import { migrateIds, iterateIds, } from './rules'; +import { batchUpdateTransactions } from './transactions'; // TODO: Detect if it looks like the user is creating a rename rule // and prompt to create it in the pre phase instead @@ -202,7 +204,9 @@ export function getRules() { return [...allRules.values()]; } -export async function insertRule(rule) { +export async function insertRule( + rule: Omit & { id?: string }, +) { rule = ruleModel.validate(rule); return db.insertWithUUID('rules', ruleModel.fromJS(rule)); } @@ -212,7 +216,7 @@ export async function updateRule(rule) { return db.update('rules', ruleModel.fromJS(rule)); } -export async function deleteRule(rule) { +export async function deleteRule(rule: T) { let schedule = await db.first('SELECT id FROM schedules WHERE rule = ?', [ rule.id, ]); @@ -477,7 +481,10 @@ export function conditionsToAQL(conditions, { recurDateBounds = 100 } = {}) { return { filters, errors }; } -export function applyActions(transactionIds, actions, handlers) { +export function applyActions( + transactionIds: string[], + actions: Array, +) { let parsedActions = actions .map(action => { if (action instanceof Action) { @@ -485,6 +492,10 @@ export function applyActions(transactionIds, actions, handlers) { } try { + if (action.op === 'link-schedule') { + return new Action(action.op, null, action.value, null, FIELD_TYPES); + } + return new Action( action.op, action.field, @@ -512,7 +523,7 @@ export function applyActions(transactionIds, actions, handlers) { return update; }); - return handlers['transactions-batch-update']({ updated }); + return batchUpdateTransactions({ updated }); } export function getRulesForPayee(payeeId) { @@ -583,7 +594,7 @@ function* getOneOfSetterRules( return null; } -export async function updatePayeeRenameRule(fromNames, to) { +export async function updatePayeeRenameRule(fromNames: string[], to: string) { let renameRule = getOneOfSetterRules('pre', 'imported_payee', 'payee', { actionValue: to, }).next().value; diff --git a/packages/loot-core/src/server/accounts/transactions.ts b/packages/loot-core/src/server/accounts/transactions.ts index c57b2972430..ed1591290e9 100644 --- a/packages/loot-core/src/server/accounts/transactions.ts +++ b/packages/loot-core/src/server/accounts/transactions.ts @@ -51,7 +51,7 @@ export async function batchUpdateTransactions({ }>; learnCategories?: boolean; detectOrphanPayees?: boolean; -}): Promise<{ added: TransactionEntity[]; updated: unknown[] }> { +}) { // Track the ids of each type of transaction change (see below for why) let addedIds = []; let updatedIds = updated ? updated.map(u => u.id) : []; @@ -126,7 +126,7 @@ export async function batchUpdateTransactions({ // to the client so that can apply them. Note that added // transactions just return the full transaction. let resultAdded = allAdded; - let resultUpdated: unknown[]; + let resultUpdated: Awaited>[]; await batchMessages(async () => { await Promise.all(allAdded.map(t => transfer.onInsert(t))); diff --git a/packages/loot-core/src/server/errors.ts b/packages/loot-core/src/server/errors.ts index 1450c447e66..a81a2a27bed 100644 --- a/packages/loot-core/src/server/errors.ts +++ b/packages/loot-core/src/server/errors.ts @@ -37,9 +37,9 @@ export class SyncError extends Error { export class TransactionError extends Error {} export class RuleError extends Error { - type; + type: string; - constructor(name, message) { + constructor(name: string, message: string) { super('RuleError: ' + message); this.type = name; } diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts index ac739846511..fd29a49b6a2 100644 --- a/packages/loot-core/src/server/main.ts +++ b/packages/loot-core/src/server/main.ts @@ -13,7 +13,6 @@ import * as sqlite from '../platform/server/sqlite'; import { isNonProductionEnvironment } from '../shared/environment'; import * as monthUtils from '../shared/months'; import q, { Query } from '../shared/query'; -import { FIELD_TYPES as ruleFieldTypes } from '../shared/rules'; import { amountToInteger, stringToInteger } from '../shared/util'; import { Handlers } from '../types/handlers'; @@ -21,7 +20,6 @@ import { exportToCSV, exportQueryToCSV } from './accounts/export-to-csv'; import * as link from './accounts/link'; import { parseFile } from './accounts/parse-file'; import { getStartingBalancePayee } from './accounts/payees'; -import { Condition, Action, rankRules } from './accounts/rules'; import * as bankSync from './accounts/sync'; import * as rules from './accounts/transaction-rules'; import { batchUpdateTransactions } from './accounts/transactions'; @@ -40,7 +38,7 @@ import * as cloudStorage from './cloud-storage'; import * as db from './db'; import * as mappings from './db/mappings'; import * as encryption from './encryption'; -import { APIError, TransactionError, PostError, RuleError } from './errors'; +import { APIError, TransactionError, PostError } from './errors'; import filtersApp from './filters/app'; import { handleBudgetImport } from './importers'; import app from './main-app'; @@ -49,6 +47,7 @@ import notesApp from './notes/app'; import * as Platform from './platform'; import { get, post } from './post'; import * as prefs from './prefs'; +import rulesApp from './rules/app'; import schedulesApp from './schedules/app'; import { getServer, setServer } from './server-config'; import * as sheet from './sheet'; @@ -499,128 +498,6 @@ handlers['payees-get-rules'] = async function ({ id }) { return rules.getRulesForPayee(id).map(rule => rule.serialize()); }; -function validateRule(rule) { - // Returns an array of errors, the array is the same link as the - // passed-in `array`, or null if there are no errors - function runValidation(array, validate) { - let result = array.map(item => { - try { - validate(item); - } catch (e) { - if (e instanceof RuleError) { - console.warn('Invalid rule', e); - return e.type; - } - throw e; - } - return null; - }); - - return result.some(Boolean) ? result : null; - } - - let conditionErrors = runValidation( - rule.conditions, - cond => - new Condition( - cond.op, - cond.field, - cond.value, - cond.options, - ruleFieldTypes, - ), - ); - - let actionErrors = runValidation( - rule.actions, - action => - new Action( - action.op, - action.field, - action.value, - action.options, - ruleFieldTypes, - ), - ); - - if (conditionErrors || actionErrors) { - return { - conditionErrors, - actionErrors, - }; - } - - return null; -} - -handlers['rule-validate'] = async function (rule) { - let error = validateRule(rule); - return { error }; -}; - -handlers['rule-add'] = mutator(async function (rule) { - let error = validateRule(rule); - if (error) { - return { error }; - } - - let id = await rules.insertRule(rule); - return { id }; -}); - -handlers['rule-update'] = mutator(async function (rule) { - let error = validateRule(rule); - if (error) { - return { error }; - } - - await rules.updateRule(rule); - return {}; -}); - -handlers['rule-delete'] = mutator(async function (rule) { - return rules.deleteRule(rule); -}); - -handlers['rule-delete-all'] = mutator(async function (ids) { - let someDeletionsFailed = false; - - await batchMessages(async () => { - for (let id of ids) { - let res = await rules.deleteRule({ id }); - if (res === false) { - someDeletionsFailed = true; - } - } - }); - - return { someDeletionsFailed }; -}); - -handlers['rule-apply-actions'] = mutator(async function ({ - transactionIds, - actions, -}) { - return rules.applyActions(transactionIds, actions, handlers); -}); - -handlers['rule-add-payee-rename'] = mutator(async function ({ fromNames, to }) { - return rules.updatePayeeRenameRule(fromNames, to); -}); - -handlers['rules-get'] = async function () { - return rankRules(rules.getRules()).map(rule => rule.serialize()); -}; - -handlers['rule-get'] = async function ({ id }) { - let rule = rules.getRules().find(rule => rule.id === id); - return rule ? rule.serialize() : null; -}; - -handlers['rules-run'] = async function ({ transaction }) { - return rules.runRules(transaction); -}; - handlers['make-filters-from-conditions'] = async function ({ conditions }) { return rules.conditionsToAQL(conditions); }; @@ -2252,7 +2129,7 @@ injectAPI.override((name, args) => runHandler(app.handlers[name], args)); // A hack for now until we clean up everything app.handlers = handlers; -app.combine(schedulesApp, budgetApp, notesApp, toolsApp, filtersApp); +app.combine(schedulesApp, budgetApp, notesApp, toolsApp, filtersApp, rulesApp); function getDefaultDocumentDir() { if (Platform.isMobile) { diff --git a/packages/loot-core/src/server/rules/app.ts b/packages/loot-core/src/server/rules/app.ts new file mode 100644 index 00000000000..47c0b73fb34 --- /dev/null +++ b/packages/loot-core/src/server/rules/app.ts @@ -0,0 +1,157 @@ +import { FIELD_TYPES as ruleFieldTypes } from '../../shared/rules'; +import { type RuleEntity } from '../../types/models'; +import { Condition, Action, rankRules } from '../accounts/rules'; +import * as rules from '../accounts/transaction-rules'; +import { createApp } from '../app'; +import { RuleError } from '../errors'; +import { mutator } from '../mutators'; +import { batchMessages } from '../sync'; +import { undoable } from '../undo'; + +import { RulesHandlers } from './types/handlers'; + +function validateRule(rule: Partial) { + // Returns an array of errors, the array is the same link as the + // passed-in `array`, or null if there are no errors + function runValidation(array: T[], validate: (item: T) => unknown) { + const result = array + .map(item => { + try { + validate(item); + } catch (e) { + if (e instanceof RuleError) { + console.warn('Invalid rule', e); + return e.type; + } + throw e; + } + return null; + }) + .filter((res): res is string => typeof res === 'string'); + + return result.length ? result : null; + } + + let conditionErrors = runValidation( + rule.conditions, + cond => + new Condition( + cond.op, + cond.field, + cond.value, + cond.options, + ruleFieldTypes, + ), + ); + + let actionErrors = runValidation(rule.actions, action => + action.op === 'link-schedule' + ? new Action(action.op, null, action.value, null, ruleFieldTypes) + : new Action( + action.op, + action.field, + action.value, + action.options, + ruleFieldTypes, + ), + ); + + if (conditionErrors || actionErrors) { + return { + conditionErrors, + actionErrors, + }; + } + + return null; +} + +// Expose functions to the client +let app = createApp(); + +app.method('rule-validate', async function (rule) { + let error = validateRule(rule); + return { error }; +}); + +app.method( + 'rule-add', + mutator(async function (rule) { + let error = validateRule(rule); + if (error) { + return { error }; + } + + let id = await rules.insertRule(rule); + return { id }; + }), +); + +app.method( + 'rule-update', + mutator(async function (rule) { + let error = validateRule(rule); + if (error) { + return { error }; + } + + await rules.updateRule(rule); + return {}; + }), +); + +app.method( + 'rule-delete', + mutator(async function (rule) { + return rules.deleteRule(rule); + }), +); + +app.method( + 'rule-delete-all', + mutator(async function (ids) { + let someDeletionsFailed = false; + + await batchMessages(async () => { + for (let id of ids) { + let res = await rules.deleteRule({ id }); + if (res === false) { + someDeletionsFailed = true; + } + } + }); + + return { someDeletionsFailed }; + }), +); + +app.method( + 'rule-apply-actions', + mutator( + undoable(async function ({ transactionIds, actions }) { + return rules.applyActions(transactionIds, actions); + }), + ), +); + +app.method( + 'rule-add-payee-rename', + mutator(async function ({ fromNames, to }) { + return rules.updatePayeeRenameRule(fromNames, to); + }), +); + +app.method('rules-get', async function () { + return rankRules(rules.getRules()).map(rule => rule.serialize()); +}); + +app.method('rule-get', async function ({ id }) { + let rule = rules.getRules().find(rule => rule.id === id); + return rule ? rule.serialize() : null; +}); + +app.method('rules-run', async function ({ transaction }) { + return rules.runRules(transaction); +}); + +export default app; diff --git a/packages/loot-core/src/server/rules/types/handlers.ts b/packages/loot-core/src/server/rules/types/handlers.ts new file mode 100644 index 00000000000..27e09e3e821 --- /dev/null +++ b/packages/loot-core/src/server/rules/types/handlers.ts @@ -0,0 +1,49 @@ +import { + type RuleEntity, + type TransactionEntity, + type RuleActionEntity, +} from '../../../types/models'; +import { type Action } from '../../accounts/rules'; + +type ValidationError = { + conditionErrors: string[]; + actionErrors: string[]; +}; + +export interface RulesHandlers { + 'rule-validate': ( + rule: Partial, + ) => Promise<{ error: ValidationError | null }>; + + 'rule-add': ( + rule: Omit, + ) => Promise<{ error: ValidationError } | { id: string }>; + + 'rule-update': ( + rule: Partial, + ) => Promise<{ error: ValidationError } | object>; + + 'rule-delete': (rule: RuleEntity) => Promise; + + 'rule-delete-all': ( + ids: string[], + ) => Promise<{ someDeletionsFailed: boolean }>; + + 'rule-apply-actions': (arg: { + transactionIds: string[]; + actions: Array; + }) => Promise; + + 'rule-add-payee-rename': (arg: { + fromNames: string[]; + to: string; + }) => Promise; + + 'rules-get': () => Promise; + + // TODO: change return value to `RuleEntity` + 'rule-get': (arg: { id: string }) => Promise; + + // TODO: change types to `TransactionEntity` + 'rules-run': (arg: { transaction }) => Promise; +} diff --git a/packages/loot-core/src/types/models/rule.d.ts b/packages/loot-core/src/types/models/rule.d.ts index e7e79faf83c..263cd78b3c9 100644 --- a/packages/loot-core/src/types/models/rule.d.ts +++ b/packages/loot-core/src/types/models/rule.d.ts @@ -1,24 +1,35 @@ +import { type ScheduleEntity } from './schedule'; + export interface RuleEntity { id: string; stage: string; - conditions_op: string; + conditions_op?: string; conditionsOp?: string; // TODO: this should not be here.. figure out howto remove it conditions: RuleConditionEntity[]; actions: RuleActionEntity[]; - tombstone: boolean; + tombstone?: boolean; } interface RuleConditionEntity { field: unknown; op: unknown; value: unknown; - options: unknown; - conditionsOp: unknown; + options?: unknown; + conditionsOp?: unknown; } -interface RuleActionEntity { - field: unknown; - op: unknown; +export type RuleActionEntity = + | SetRuleActionEntity + | LinkScheduleRuleActionEntity; + +export interface SetRuleActionEntity { + field: string; + op: 'set'; value: unknown; - options: unknown; + options?: unknown; +} + +export interface LinkScheduleRuleActionEntity { + op: 'link-schedule'; + value: ScheduleEntity; } diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts index 6cad98768bc..6d673054350 100644 --- a/packages/loot-core/src/types/server-handlers.d.ts +++ b/packages/loot-core/src/types/server-handlers.d.ts @@ -97,26 +97,6 @@ export interface ServerHandlers { 'payees-get-rules': (arg: { id }) => Promise; - 'rule-validate': (rule) => Promise<{ error: unknown }>; - - 'rule-add': (rule) => Promise<{ error: unknown } | { id: string }>; - - 'rule-add': (rule) => Promise<{ error: unknown } | unknown>; - - 'rule-delete': (rule) => Promise; - - 'rule-delete-all': (ids) => Promise; - - 'rule-apply-actions': (arg: { transactionIds; actions }) => Promise; - - 'rule-add-payee-rename': (arg: { fromNames; to }) => Promise; - - 'rules-get': () => Promise; - - 'rule-get': (arg: { id }) => Promise; - - 'rules-run': (arg: { transaction }) => Promise; - 'make-filters-from-conditions': (arg: { conditions; }) => Promise<{ filters: unknown[] }>; diff --git a/upcoming-release-notes/1677.md b/upcoming-release-notes/1677.md new file mode 100644 index 00000000000..1274b0652d2 --- /dev/null +++ b/upcoming-release-notes/1677.md @@ -0,0 +1,6 @@ +--- +category: Maintenance +authors: [MatissJanis] +--- + +Moving 'rules' server action handlers into a separate file