From 646d0d90a437e5854d29cccfb8d3567bfd7361d5 Mon Sep 17 00:00:00 2001
From: Jed Fox <git@jedfox.com>
Date: Tue, 2 May 2023 14:08:05 -0400
Subject: [PATCH] Remove unused payee rules feature (#985)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixes #615. I would appreciate double-checking that I didn’t
accidentally delete anything that is important.
Since I’m removing the related API methods, this is technically a
breaking change (even if people would have no reason to remove this
stuff), so we should probably do a major release of the API package.
---
packages/api/methods.js | 16 ---
.../components/payees/ManagePayeesWithData.js | 6 +-
.../1682974838138_remove_payee_rules.sql | 5 +
.../server/accounts/transaction-rules.test.ts | 59 --------
.../src/server/accounts/transaction-rules.ts | 128 +-----------------
packages/loot-core/src/server/api-models.ts | 13 --
packages/loot-core/src/server/api.ts | 31 -----
packages/loot-core/src/server/db/index.ts | 28 ----
packages/loot-core/src/server/main.ts | 32 -----
packages/loot-core/src/server/models.ts | 18 ---
.../src/server/sync/sync.property.test.ts | 6 -
.../loot-core/src/types/api.handlers.d.ts | 8 --
.../loot-core/src/types/main.handlers.d.ts | 8 --
upcoming-release-notes/985.md | 6 +
14 files changed, 13 insertions(+), 351 deletions(-)
create mode 100644 packages/loot-core/migrations/1682974838138_remove_payee_rules.sql
create mode 100644 upcoming-release-notes/985.md
diff --git a/packages/api/methods.js b/packages/api/methods.js
index ccad98af6..90cda1ae2 100644
--- a/packages/api/methods.js
+++ b/packages/api/methods.js
@@ -149,19 +149,3 @@ export function updatePayee(id, fields) {
export function deletePayee(id) {
return send('api/payee-delete', { id });
}
-
-export function getPayeeRules(payeeId) {
- return send('api/payee-rules-get', { payeeId });
-}
-
-export function createPayeeRule(payeeId, rule) {
- return send('api/payee-rule-create', { payee_id: payeeId, rule });
-}
-
-export function updatePayeeRule(id, fields) {
- return send('api/payee-rule-update', { id, fields });
-}
-
-export function deletePayeeRule(id) {
- return send('api/payee-rule-delete', { id });
-}
diff --git a/packages/desktop-client/src/components/payees/ManagePayeesWithData.js b/packages/desktop-client/src/components/payees/ManagePayeesWithData.js
index d02ffd359..6c86e2cc3 100644
--- a/packages/desktop-client/src/components/payees/ManagePayeesWithData.js
+++ b/packages/desktop-client/src/components/payees/ManagePayeesWithData.js
@@ -58,11 +58,7 @@ function ManagePayeesWithData({
}, []);
async function onUndo({ tables, messages, meta, url }, scroll = false) {
- if (
- !tables.includes('payees') &&
- !tables.includes('payee_mapping') &&
- !tables.includes('payee_rules')
- ) {
+ if (!tables.includes('payees') && !tables.includes('payee_mapping')) {
return;
}
diff --git a/packages/loot-core/migrations/1682974838138_remove_payee_rules.sql b/packages/loot-core/migrations/1682974838138_remove_payee_rules.sql
new file mode 100644
index 000000000..070aecd53
--- /dev/null
+++ b/packages/loot-core/migrations/1682974838138_remove_payee_rules.sql
@@ -0,0 +1,5 @@
+BEGIN TRANSACTION;
+
+DROP TABLE payee_rules;
+
+COMMIT;
diff --git a/packages/loot-core/src/server/accounts/transaction-rules.test.ts b/packages/loot-core/src/server/accounts/transaction-rules.test.ts
index c3fc3a40c..50211264f 100644
--- a/packages/loot-core/src/server/accounts/transaction-rules.test.ts
+++ b/packages/loot-core/src/server/accounts/transaction-rules.test.ts
@@ -15,7 +15,6 @@ import {
resetState,
getProbableCategory,
updateCategoryRules,
- migrateOldRules,
} from './transaction-rules';
// TODO: write tests to make sure payee renaming is "pre" and category
@@ -368,64 +367,6 @@ describe('Transaction rules', () => {
});
});
- test('migrating from the old payee rules works', async () => {
- await loadRules();
- let categoryGroupId = await db.insertCategoryGroup({ name: 'general' });
- let categoryId = await db.insertCategory({
- name: 'food',
- cat_group: categoryGroupId,
- });
- let krogerId = await db.insertPayee({ name: 'kroger' });
- let lowesId = await db.insertPayee({ name: 'lowes', category: categoryId });
-
- await db.insertPayeeRule({
- payee_id: krogerId,
- type: 'contains',
- value: 'kroger',
- });
- await db.insertPayeeRule({
- payee_id: lowesId,
- type: 'equals',
- value: '123 lowes',
- });
- await db.insertPayeeRule({
- payee_id: lowesId,
- type: 'equals',
- value: 'lowes 456',
- });
-
- // Migrate!
- await migrateOldRules();
-
- expect(getRules().length).toBe(3);
-
- expect(runRules({ payee: null, imported_payee: '123 lowes' })).toEqual({
- category: categoryId,
- imported_payee: '123 lowes',
- payee: lowesId,
- });
- expect(runRules({ payee: null, imported_payee: 'lowes 456' })).toEqual({
- category: categoryId,
- imported_payee: 'lowes 456',
- payee: lowesId,
- });
- expect(runRules({ payee: null, imported_payee: '1 lowes 2' })).toEqual({
- imported_payee: '1 lowes 2',
- payee: null,
- });
- expect(
- runRules({
- payee: null,
- imported_payee: 'blah blah kroger bla',
- category: null,
- }),
- ).toEqual({
- imported_payee: 'blah blah kroger bla',
- payee: krogerId,
- category: null,
- });
- });
-
test('transactions can be queried by rule', async () => {
await loadRules();
let account = await db.insertAccount({ name: 'bank' });
diff --git a/packages/loot-core/src/server/accounts/transaction-rules.ts b/packages/loot-core/src/server/accounts/transaction-rules.ts
index d1f3602cc..46c7c5593 100644
--- a/packages/loot-core/src/server/accounts/transaction-rules.ts
+++ b/packages/loot-core/src/server/accounts/transaction-rules.ts
@@ -16,7 +16,7 @@ import * as db from '../db';
import { getMappings } from '../db/mappings';
import { RuleError } from '../errors';
import { requiredFields, toDateRepr } from '../models';
-import { setSyncingMode, batchMessages } from '../sync';
+import { batchMessages } from '../sync';
import { addSyncListener } from '../sync/index';
import {
@@ -712,129 +712,3 @@ export async function updateCategoryRules(transactions) {
}
});
}
-
-// This can be removed in the future
-export async function migrateOldRules() {
- let allPayees = await db.all(
- `SELECT p.*, c.id as category FROM payees p
- LEFT JOIN category_mapping cm ON cm.id = p.category
- LEFT JOIN categories c ON (c.id = cm.transferId AND c.tombstone = 0)
- WHERE p.tombstone = 0 AND transfer_acct IS NULL`,
- );
- let allRules = await db.all(
- `SELECT pr.*, pm.targetId as payee_id FROM payee_rules pr
- LEFT JOIN payee_mapping pm ON pm.id = pr.payee_id
- WHERE pr.tombstone = 0`,
- );
-
- let payeesById = new Map();
- for (let i = 0; i < allPayees.length; i++) {
- payeesById.set(allPayees[i].id, allPayees[i]);
- }
-
- let rulesByPayeeId = new Map();
- for (let i = 0; i < allRules.length; i++) {
- let item = allRules[i];
- let rules = rulesByPayeeId.get(item.payee_id) || [];
- rules.push(item);
- rulesByPayeeId.set(item.payee_id, rules);
- }
-
- let rules = [];
-
- // Convert payee name rules
- for (let [payeeId, payeeRules] of rulesByPayeeId.entries()) {
- let equals = payeeRules.filter(r => {
- let payee = payeesById.get(r.payee_id);
-
- return (
- (r.type === 'equals' || r.type == null) &&
- (!payee || r.value.toLowerCase() !== payee.name.toLowerCase())
- );
- });
- let contains = payeeRules.filter(r => r.type === 'contains');
- let actions = [{ op: 'set', field: 'payee', value: payeeId }];
-
- if (equals.length > 0) {
- rules.push({
- stage: null,
- conditionsOp: 'and',
- conditions: [
- {
- op: 'oneOf',
- field: 'imported_payee',
- value: equals.map(payeeRule => payeeRule.value),
- },
- ],
- actions,
- });
- }
-
- if (contains.length > 0) {
- rules = rules.concat(
- contains.map(payeeRule => ({
- stage: null,
- conditionsOp: 'and',
- conditions: [
- {
- op: 'contains',
- field: 'imported_payee',
- value: payeeRule.value,
- },
- ],
- actions,
- })),
- );
- }
- }
-
- // Convert category rules
- let catRules = allPayees
- .filter(p => p.category)
- .reduce((map, payee) => {
- let ids = map.get(payee.category) || new Set();
- ids.add(payee.id);
- map.set(payee.category, ids);
- return map;
- }, new Map());
-
- for (let [catId, payeeIds] of catRules) {
- rules.push({
- stage: null,
- conditionsOp: 'and',
- conditions: [
- {
- op: 'oneOf',
- field: 'payee',
- value: [...payeeIds],
- },
- ],
- actions: [
- {
- op: 'set',
- field: 'category',
- value: catId,
- },
- ],
- });
- }
-
- // Very important: we never want to sync migration changes, but it
- // still has to run through the syncing layer to make sure
- // projections are correct. This is only OK because we require a
- // sync reset after this.
- let prevMode = setSyncingMode('disabled');
- await batchMessages(async () => {
- for (let rule of rules) {
- await insertRule({
- stage: rule.stage,
- conditionsOp: rule.conditionsOp,
- conditions: rule.conditions,
- actions: rule.actions,
- });
- }
-
- await db.runQuery('DELETE FROM payee_rules', []);
- });
- setSyncingMode(prevMode);
-}
diff --git a/packages/loot-core/src/server/api-models.ts b/packages/loot-core/src/server/api-models.ts
index ea1da3d13..482a527a7 100644
--- a/packages/loot-core/src/server/api-models.ts
+++ b/packages/loot-core/src/server/api-models.ts
@@ -167,16 +167,3 @@ export const payeeModel = {
return payee;
},
};
-
-export const payeeRuleModel = {
- ...models.payeeRuleModel,
-
- toExternal(rule) {
- let { tombstone, ...result } = rule;
- return result;
- },
-
- fromExternal(rule) {
- return rule;
- },
-};
diff --git a/packages/loot-core/src/server/api.ts b/packages/loot-core/src/server/api.ts
index 4c73f5419..fc2c3c4ab 100644
--- a/packages/loot-core/src/server/api.ts
+++ b/packages/loot-core/src/server/api.ts
@@ -19,7 +19,6 @@ import {
categoryModel,
categoryGroupModel,
payeeModel,
- payeeRuleModel,
} from './api-models';
import { runQuery as aqlQuery } from './aql';
import * as cloudStorage from './cloud-storage';
@@ -545,36 +544,6 @@ handlers['api/payee-delete'] = withMutation(async function ({ id }) {
return handlers['payees-batch-change']({ deleted: [{ id }] });
});
-handlers['api/payee-rules-get'] = async function ({ payeeId }) {
- let rules = await handlers['payees-get-rules']({ id: payeeId });
- return rules.map(payeeRuleModel.toExternal);
-};
-
-handlers['api/payee-rule-create'] = withMutation(async function ({
- payee_id,
- rule,
-}) {
- return handlers['payees-add-rule']({
- payee_id,
- type: rule.type,
- value: rule.value || null,
- });
-});
-
-handlers['api/payee-rule-update'] = withMutation(async function ({
- id,
- fields,
-}) {
- return handlers['payees-update-rule']({
- id,
- ...payeeRuleModel.fromExternal(fields),
- });
-});
-
-handlers['api/payee-rule-delete'] = withMutation(async function ({ id }) {
- return handlers['payees-delete-rule']({ id });
-});
-
export default function installAPI(serverHandlers) {
handlers = Object.assign({}, serverHandlers, handlers);
return handlers;
diff --git a/packages/loot-core/src/server/db/index.ts b/packages/loot-core/src/server/db/index.ts
index 1d08c3d16..a68a64e21 100644
--- a/packages/loot-core/src/server/db/index.ts
+++ b/packages/loot-core/src/server/db/index.ts
@@ -24,7 +24,6 @@ import {
categoryModel,
categoryGroupModel,
payeeModel,
- payeeRuleModel,
} from '../models';
import { sendMessages, batchMessages } from '../sync';
@@ -454,10 +453,6 @@ export async function deletePayee(payee) {
// mappings.map(m => update('payee_mapping', { id: m.id, targetId: null }))
// );
- let rules = await all('SELECT * FROM payee_rules WHERE payee_id = ?', [
- payee.id,
- ]);
- await Promise.all(rules.map(rule => deletePayeeRule({ id: rule.id })));
return delete_('payees', payee.id);
}
@@ -533,29 +528,6 @@ export async function getPayeeByName(name) {
);
}
-export function insertPayeeRule(rule) {
- rule = payeeRuleModel.validate(rule);
- return insertWithUUID('payee_rules', rule);
-}
-
-export function deletePayeeRule(rule) {
- return delete_('payee_rules', rule.id);
-}
-
-export function updatePayeeRule(rule) {
- rule = payeeModel.validate(rule, { update: true });
- return update('payee_rules', rule);
-}
-
-export function getPayeeRules(id) {
- return all(
- `SELECT pr.* FROM payee_rules pr
- LEFT JOIN payee_mapping pm ON pm.id = pr.payee_id
- WHERE pm.targetId = ? AND pr.tombstone = 0`,
- [id],
- );
-}
-
export function getAccounts() {
return all(
`SELECT a.*, b.name as bankName, b.id as bankId FROM accounts a
diff --git a/packages/loot-core/src/server/main.ts b/packages/loot-core/src/server/main.ts
index a07047f9e..ac508c218 100644
--- a/packages/loot-core/src/server/main.ts
+++ b/packages/loot-core/src/server/main.ts
@@ -505,34 +505,6 @@ handlers['payees-get-rules'] = async function ({ id }) {
return rules.getRulesForPayee(id).map(rule => rule.serialize());
};
-handlers['payees-delete-rule'] = mutator(async function ({ id, payee_id }) {
- return withUndo(
- async () => {
- return await db.deletePayeeRule({ id });
- },
- { payeeId: payee_id },
- );
-});
-
-handlers['payees-update-rule'] = mutator(async function (rule) {
- return withUndo(
- async () => {
- return await db.updatePayeeRule(rule);
- },
- { payeeId: rule.payee_id },
- );
-});
-
-handlers['payees-add-rule'] = mutator(async function (rule) {
- return withUndo(
- async () => {
- let id = await db.insertPayeeRule(rule);
- return { ...rule, id };
- },
- { payeeId: rule.payee_id },
- );
-});
-
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
@@ -655,10 +627,6 @@ handlers['rules-run'] = async function ({ transaction }) {
return rules.runRules(transaction);
};
-handlers['rules-migrate'] = async function () {
- await rules.migrateOldRules();
-};
-
handlers['make-filters-from-conditions'] = async function ({ conditions }) {
return rules.conditionsToAQL(conditions);
};
diff --git a/packages/loot-core/src/server/models.ts b/packages/loot-core/src/server/models.ts
index 86bcd3ed5..40cb1002a 100644
--- a/packages/loot-core/src/server/models.ts
+++ b/packages/loot-core/src/server/models.ts
@@ -102,24 +102,6 @@ export const payeeModel = {
},
};
-export const payeeRuleModel = {
- validateType(rule) {
- const { type } = rule;
- if (type !== 'equals' && type !== 'contains') {
- throw new Error('Invalid rule type: ' + type);
- }
- },
-
- validate(rule, { update }: { update?: boolean } = {}) {
- if (!update || 'type' in rule) {
- payeeRuleModel.validateType(rule);
- }
-
- requiredFields('payee_rules', rule, ['payee_id', 'type'], update);
- return rule;
- },
-};
-
export const transactionModel = {
validate(trans, { update }: { update?: boolean } = {}) {
requiredFields('transaction', trans, ['date', 'acct'], update);
diff --git a/packages/loot-core/src/server/sync/sync.property.test.ts b/packages/loot-core/src/server/sync/sync.property.test.ts
index 0b636cbde..4242e3c61 100644
--- a/packages/loot-core/src/server/sync/sync.property.test.ts
+++ b/packages/loot-core/src/server/sync/sync.property.test.ts
@@ -84,12 +84,6 @@ let schema = {
category: 'text',
tombstone: 'integer',
},
- payee_rules: {
- payee_id: 'text',
- type: 'text',
- value: 'text',
- tombstone: 'integer',
- },
payee_mapping: { targetId: 'text' },
};
diff --git a/packages/loot-core/src/types/api.handlers.d.ts b/packages/loot-core/src/types/api.handlers.d.ts
index 962f081d7..30e7b5b66 100644
--- a/packages/loot-core/src/types/api.handlers.d.ts
+++ b/packages/loot-core/src/types/api.handlers.d.ts
@@ -112,12 +112,4 @@ export interface ApiHandlers {
'api/payee-update': (arg: { id; fields }) => Promise<unknown>;
'api/payee-delete': (arg: { id }) => Promise<unknown>;
-
- 'api/payee-rules-get': (arg: { payeeId }) => Promise<unknown>;
-
- 'api/payee-rule-create': (arg: { payee_id; rule }) => Promise<unknown>;
-
- 'api/payee-rule-update': (arg: { id; fields }) => Promise<unknown>;
-
- 'api/payee-rule-delete': (arg: { id }) => Promise<unknown>;
}
diff --git a/packages/loot-core/src/types/main.handlers.d.ts b/packages/loot-core/src/types/main.handlers.d.ts
index 49d3c0005..90cb6cc8b 100644
--- a/packages/loot-core/src/types/main.handlers.d.ts
+++ b/packages/loot-core/src/types/main.handlers.d.ts
@@ -81,12 +81,6 @@ export interface MainHandlers {
'payees-get-rules': (arg: { id }) => Promise<unknown>;
- 'payees-delete-rule': (arg: { id; payee_id }) => Promise<unknown>;
-
- 'payees-update-rule': (rule) => Promise<unknown>;
-
- 'payees-add-rule': (rule) => Promise<unknown>;
-
'rule-validate': (rule) => Promise<{ error: unknown }>;
'rule-add': (rule) => Promise<{ error: unknown } | { id: string }>;
@@ -107,8 +101,6 @@ export interface MainHandlers {
'rules-run': (arg: { transaction }) => Promise<unknown>;
- 'rules-migrate': () => Promise<unknown>;
-
'make-filters-from-conditions': (arg: { conditions }) => Promise<unknown>;
getCell: (arg: { sheetName; name }) => Promise<unknown>;
diff --git a/upcoming-release-notes/985.md b/upcoming-release-notes/985.md
new file mode 100644
index 000000000..890b46234
--- /dev/null
+++ b/upcoming-release-notes/985.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [j-f1]
+---
+
+Remove unused payee rules feature
--
GitLab