From d9066a49c4dbcfbb507ff65696a279b06cadbeae Mon Sep 17 00:00:00 2001
From: Julian Dominguez-Schatz <julian.dominguezschatz@gmail.com>
Date: Wed, 14 Aug 2024 13:28:09 -0400
Subject: [PATCH] Remove some `any` types from the API (#3238)
* Remove some `any` types from the API
* Add release notes
* No backwards-incompatible changes
* Update tests to reflect API changes
* PR feedback: standardize on id for deletes
* PR feedback: restore partial updates
---
packages/api/methods.test.ts | 22 +++++++------------
packages/api/methods.ts | 4 ++--
.../server/accounts/transaction-rules.test.ts | 2 +-
.../src/server/accounts/transaction-rules.ts | 7 +++---
packages/loot-core/src/server/api.ts | 21 ++++++++++--------
packages/loot-core/src/server/rules/app.ts | 6 ++---
.../src/server/rules/types/handlers.ts | 13 ++++++-----
.../loot-core/src/types/api-handlers.d.ts | 12 ++++++----
.../loot-core/src/types/server-handlers.d.ts | 2 +-
upcoming-release-notes/3238.md | 6 +++++
10 files changed, 53 insertions(+), 42 deletions(-)
create mode 100644 upcoming-release-notes/3238.md
diff --git a/packages/api/methods.test.ts b/packages/api/methods.test.ts
index 8e0a9b586..53b39fa5d 100644
--- a/packages/api/methods.test.ts
+++ b/packages/api/methods.test.ts
@@ -81,28 +81,22 @@ describe('API CRUD operations', () => {
expect(groups).toEqual(
expect.arrayContaining([
expect.objectContaining({
- hidden: 0,
+ hidden: false,
id: 'fc3825fd-b982-4b72-b768-5b30844cf832',
- is_income: 0,
+ is_income: false,
name: 'Usual Expenses',
- sort_order: 16384,
- tombstone: 0,
}),
expect.objectContaining({
- hidden: 0,
+ hidden: false,
id: 'a137772f-cf2f-4089-9432-822d2ddc1466',
- is_income: 0,
+ is_income: false,
name: 'Investments and Savings',
- sort_order: 32768,
- tombstone: 0,
}),
expect.objectContaining({
- hidden: 0,
+ hidden: false,
id: '2E1F5BDB-209B-43F9-AF2C-3CE28E380C00',
- is_income: 1,
+ is_income: true,
name: 'Income',
- sort_order: 32768,
- tombstone: 0,
}),
]),
);
@@ -563,10 +557,10 @@ describe('API CRUD operations', () => {
);
// delete rules
- await api.deleteRule(rules[1]);
+ await api.deleteRule(rules[1].id);
expect(await api.getRules()).toHaveLength(1);
- await api.deleteRule(rules[0]);
+ await api.deleteRule(rules[0].id);
expect(await api.getRules()).toHaveLength(0);
});
diff --git a/packages/api/methods.ts b/packages/api/methods.ts
index fc3455063..a6dec0e74 100644
--- a/packages/api/methods.ts
+++ b/packages/api/methods.ts
@@ -205,8 +205,8 @@ export function updateRule(rule) {
return send('api/rule-update', { rule });
}
-export function deleteRule(id) {
- return send('api/rule-delete', { id });
+export function deleteRule(id: string) {
+ return send('api/rule-delete', id);
}
export function holdBudgetForNextMonth(month, amount) {
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 367da4b57..3ca6834d9 100644
--- a/packages/loot-core/src/server/accounts/transaction-rules.test.ts
+++ b/packages/loot-core/src/server/accounts/transaction-rules.test.ts
@@ -209,7 +209,7 @@ describe('Transaction rules', () => {
expect(transaction.payee).toBe('Kroger');
expect(transaction.category).toBe('food');
- await deleteRule({ id });
+ await deleteRule(id);
expect(getRules().length).toBe(0);
transaction = runRules({
payee: 'Kroger',
diff --git a/packages/loot-core/src/server/accounts/transaction-rules.ts b/packages/loot-core/src/server/accounts/transaction-rules.ts
index be667ce65..44a8e223c 100644
--- a/packages/loot-core/src/server/accounts/transaction-rules.ts
+++ b/packages/loot-core/src/server/accounts/transaction-rules.ts
@@ -223,16 +223,17 @@ export async function updateRule(rule) {
return db.update('rules', ruleModel.fromJS(rule));
}
-export async function deleteRule<T extends { id: string }>(rule: T) {
+export async function deleteRule(id: string) {
const schedule = await db.first('SELECT id FROM schedules WHERE rule = ?', [
- rule.id,
+ id,
]);
if (schedule) {
return false;
}
- return db.delete_('rules', rule.id);
+ await db.delete_('rules', id);
+ return true;
}
// Sync projections
diff --git a/packages/loot-core/src/server/api.ts b/packages/loot-core/src/server/api.ts
index 6d49fb813..a2e9348fd 100644
--- a/packages/loot-core/src/server/api.ts
+++ b/packages/loot-core/src/server/api.ts
@@ -44,12 +44,14 @@ let IMPORT_MODE = false;
// we also need to notify the UI manually if stuff has changed (if
// they are connecting to an already running instance, the UI should
// update). The wrapper handles that.
-function withMutation(handler) {
- return args => {
+function withMutation<Params extends Array<unknown>, ReturnType>(
+ handler: (...args: Params) => Promise<ReturnType>,
+) {
+ return (...args: Params) => {
return runMutator(
async () => {
const latestTimestamp = getClock().timestamp.toString();
- const result = await handler(args);
+ const result = await handler(...args);
const rows = await db.all(
'SELECT DISTINCT dataset FROM messages_crdt WHERE timestamp > ?',
@@ -464,7 +466,7 @@ handlers['api/transactions-add'] = withMutation(async function ({
runTransfers,
learnCategories,
});
- return 'ok';
+ return 'ok' as const;
});
handlers['api/transactions-get'] = async function ({
@@ -503,7 +505,7 @@ handlers['api/transaction-update'] = withMutation(async function ({
}
const { diff } = updateTransaction(transactions, { id, ...fields });
- return handlers['transactions-batch-update'](diff);
+ return handlers['transactions-batch-update'](diff)['updated'];
});
handlers['api/transaction-delete'] = withMutation(async function ({ id }) {
@@ -518,7 +520,7 @@ handlers['api/transaction-delete'] = withMutation(async function ({ id }) {
}
const { diff } = deleteTransaction(transactions, id);
- return handlers['transactions-batch-update'](diff);
+ return handlers['transactions-batch-update'](diff)['deleted'];
});
handlers['api/accounts-get'] = async function () {
@@ -590,7 +592,8 @@ handlers['api/categories-get'] = async function ({
handlers['api/category-groups-get'] = async function () {
checkFileOpen();
- return handlers['get-category-groups']();
+ const groups = await handlers['get-category-groups']();
+ return groups.map(categoryGroupModel.toExternal);
};
handlers['api/category-group-create'] = withMutation(async function ({
@@ -711,7 +714,7 @@ handlers['api/rule-create'] = withMutation(async function ({ rule }) {
handlers['api/rule-update'] = withMutation(async function ({ rule }) {
checkFileOpen();
- const updatedRule = handlers['rule-update'](rule);
+ const updatedRule = await handlers['rule-update'](rule);
if ('error' in updatedRule) {
throw APIError('Failed updating the rule', updatedRule.error);
@@ -720,7 +723,7 @@ handlers['api/rule-update'] = withMutation(async function ({ rule }) {
return updatedRule;
});
-handlers['api/rule-delete'] = withMutation(async function ({ id }) {
+handlers['api/rule-delete'] = withMutation(async function (id) {
checkFileOpen();
return handlers['rule-delete'](id);
});
diff --git a/packages/loot-core/src/server/rules/app.ts b/packages/loot-core/src/server/rules/app.ts
index 01d37e1b7..df9e272bd 100644
--- a/packages/loot-core/src/server/rules/app.ts
+++ b/packages/loot-core/src/server/rules/app.ts
@@ -111,8 +111,8 @@ app.method(
app.method(
'rule-delete',
- mutator(async function (rule) {
- return rules.deleteRule(rule);
+ mutator(async function (id) {
+ return rules.deleteRule(id);
}),
);
@@ -123,7 +123,7 @@ app.method(
await batchMessages(async () => {
for (const id of ids) {
- const res = await rules.deleteRule({ id });
+ const res = await rules.deleteRule(id);
if (res === false) {
someDeletionsFailed = true;
}
diff --git a/packages/loot-core/src/server/rules/types/handlers.ts b/packages/loot-core/src/server/rules/types/handlers.ts
index 429acddc9..7884f6e4c 100644
--- a/packages/loot-core/src/server/rules/types/handlers.ts
+++ b/packages/loot-core/src/server/rules/types/handlers.ts
@@ -18,13 +18,16 @@ export interface RulesHandlers {
'rule-add': (
rule: Omit<RuleEntity, 'id'>,
- ) => Promise<{ error: ValidationError } | { id: string }>;
+ ) => Promise<{ error: ValidationError } | RuleEntity>;
- 'rule-update': (
- rule: Partial<RuleEntity>,
- ) => Promise<{ error: ValidationError } | object>;
+ 'rule-update': <
+ PartialRule extends Partial<Omit<RuleEntity, 'id'>> &
+ Pick<RuleEntity, 'id'>,
+ >(
+ rule: PartialRule,
+ ) => Promise<{ error: ValidationError } | PartialRule>;
- 'rule-delete': (rule: Required<RuleEntity>) => Promise<false | void>;
+ 'rule-delete': (id: string) => Promise<boolean>;
'rule-delete-all': (
ids: string[],
diff --git a/packages/loot-core/src/types/api-handlers.d.ts b/packages/loot-core/src/types/api-handlers.d.ts
index cf26bac83..eb294292d 100644
--- a/packages/loot-core/src/types/api-handlers.d.ts
+++ b/packages/loot-core/src/types/api-handlers.d.ts
@@ -66,7 +66,7 @@ export interface ApiHandlers {
'api/budget-hold-for-next-month': (arg: {
month: string;
amount: number;
- }) => Promise<void>;
+ }) => Promise<boolean>;
'api/budget-reset-hold': (arg: { month: string }) => Promise<void>;
@@ -76,7 +76,11 @@ export interface ApiHandlers {
payees;
}) => Promise<unknown>;
- 'api/transactions-import': (arg: { accountId; transactions }) => Promise<{
+ 'api/transactions-import': (arg: {
+ accountId;
+ transactions;
+ isPreview?;
+ }) => Promise<{
errors?: { message: string }[];
added;
updated;
@@ -102,7 +106,7 @@ export interface ApiHandlers {
'api/transaction-delete': (arg: {
id;
- }) => Promise<Awaited<ReturnType<typeof batchUpdateTransactions>>['updated']>;
+ }) => Promise<Awaited<ReturnType<typeof batchUpdateTransactions>>['deleted']>;
'api/sync': () => Promise<void>;
@@ -176,5 +180,5 @@ export interface ApiHandlers {
'api/rule-update': (arg: { rule: RuleEntity }) => Promise<RuleEntity>;
- 'api/rule-delete': (arg: { id: string }) => Promise<boolean>;
+ 'api/rule-delete': (id: string) => Promise<boolean>;
}
diff --git a/packages/loot-core/src/types/server-handlers.d.ts b/packages/loot-core/src/types/server-handlers.d.ts
index 300484f7d..f455b2bbb 100644
--- a/packages/loot-core/src/types/server-handlers.d.ts
+++ b/packages/loot-core/src/types/server-handlers.d.ts
@@ -108,7 +108,7 @@ export interface ServerHandlers {
'payees-get-rule-counts': () => Promise<unknown>;
- 'payees-merge': (arg: { targetId; mergeIds }) => Promise<unknown>;
+ 'payees-merge': (arg: { targetId; mergeIds }) => Promise<void>;
'payees-batch-change': (arg: {
added?;
diff --git a/upcoming-release-notes/3238.md b/upcoming-release-notes/3238.md
new file mode 100644
index 000000000..9ffe939d8
--- /dev/null
+++ b/upcoming-release-notes/3238.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [jfdoming]
+---
+
+Remove some `any` types from the API
--
GitLab