From 653a0ab104ab62ad629ff47c0f894d6601ce3a22 Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins <matiss@mja.lv> Date: Mon, 29 Jul 2024 18:35:49 +0100 Subject: [PATCH] :bug: (rules) fix error handling (#3149) --- .../modals/CreateEncryptionKeyModal.tsx | 6 ++-- .../src/components/modals/EditRule.jsx | 22 +++++++++------ .../modals/FixEncryptionKeyModal.tsx | 6 ++-- .../modals/GoCardlessInitialise.tsx | 6 ++-- .../components/modals/ImportTransactions.jsx | 6 ++-- .../components/modals/SimpleFinInitialise.tsx | 6 ++-- .../components/schedules/ScheduleDetails.jsx | 16 ++++++----- .../src/server/accounts/rules.test.ts | 7 +++++ .../loot-core/src/server/accounts/rules.ts | 4 +++ packages/loot-core/src/server/rules/app.ts | 28 +++++++++---------- upcoming-release-notes/3149.md | 6 ++++ 11 files changed, 68 insertions(+), 45 deletions(-) create mode 100644 upcoming-release-notes/3149.md diff --git a/packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx b/packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx index 8844739a4..d80da0f07 100644 --- a/packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx +++ b/packages/desktop-client/src/components/modals/CreateEncryptionKeyModal.tsx @@ -43,7 +43,7 @@ export function CreateEncryptionKeyModal({ const isRecreating = options.recreate; - async function onCreateKey() { + async function onCreateKey(close: () => void) { if (password !== '' && !loading) { setLoading(true); setError(null); @@ -60,6 +60,7 @@ export function CreateEncryptionKeyModal({ dispatch(sync()); setLoading(false); + close(); } } @@ -151,8 +152,7 @@ export function CreateEncryptionKeyModal({ <Form onSubmit={e => { e.preventDefault(); - onCreateKey(); - close(); + onCreateKey(close); }} > <View style={{ alignItems: 'center' }}> diff --git a/packages/desktop-client/src/components/modals/EditRule.jsx b/packages/desktop-client/src/components/modals/EditRule.jsx index 2256bc287..ad092f26d 100644 --- a/packages/desktop-client/src/components/modals/EditRule.jsx +++ b/packages/desktop-client/src/components/modals/EditRule.jsx @@ -861,7 +861,7 @@ export function EditRule({ defaultRule, onSave: originalOnSave }) { }); } - async function onSave() { + async function onSave(close) { const rule = { ...defaultRule, stage, @@ -879,7 +879,16 @@ export function EditRule({ defaultRule, onSave: originalOnSave }) { } if (error.actionErrors) { - setActionSplits(applyErrors(actionSplits, error.actionErrors)); + let usedErrorIdx = 0; + setActionSplits( + actionSplits.map(item => ({ + ...item, + actions: item.actions.map(action => ({ + ...action, + error: error.actionErrors[usedErrorIdx++] ?? null, + })), + })), + ); } } else { // If adding a rule, we got back an id @@ -888,6 +897,7 @@ export function EditRule({ defaultRule, onSave: originalOnSave }) { } originalOnSave?.(rule); + close(); } } @@ -1145,13 +1155,7 @@ export function EditRule({ defaultRule, onSave: originalOnSave }) { style={{ marginTop: 20 }} > <Button onClick={close}>Cancel</Button> - <Button - variant="primary" - onPress={() => { - onSave(); - close(); - }} - > + <Button variant="primary" onPress={() => onSave(close)}> Save </Button> </Stack> diff --git a/packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx b/packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx index c48a6e175..cd783296b 100644 --- a/packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx +++ b/packages/desktop-client/src/components/modals/FixEncryptionKeyModal.tsx @@ -37,7 +37,7 @@ export function FixEncryptionKeyModal({ const [showPassword, setShowPassword] = useState(false); const { isNarrowWidth } = useResponsive(); - async function onUpdateKey() { + async function onUpdateKey(close: () => void) { if (password !== '' && !loading) { setLoading(true); setError(null); @@ -53,6 +53,7 @@ export function FixEncryptionKeyModal({ } onSuccess?.(); + close(); } } @@ -104,8 +105,7 @@ export function FixEncryptionKeyModal({ <Form onSubmit={e => { e.preventDefault(); - onUpdateKey(); - close(); + onUpdateKey(close); }} > <View diff --git a/packages/desktop-client/src/components/modals/GoCardlessInitialise.tsx b/packages/desktop-client/src/components/modals/GoCardlessInitialise.tsx index 3ba1ea3d0..fc91a66a1 100644 --- a/packages/desktop-client/src/components/modals/GoCardlessInitialise.tsx +++ b/packages/desktop-client/src/components/modals/GoCardlessInitialise.tsx @@ -29,7 +29,7 @@ export const GoCardlessInitialise = ({ const [isValid, setIsValid] = useState(true); const [isLoading, setIsLoading] = useState(false); - const onSubmit = async () => { + const onSubmit = async (close: () => void) => { if (!secretId || !secretKey) { setIsValid(false); return; @@ -50,6 +50,7 @@ export const GoCardlessInitialise = ({ onSuccess(); setIsLoading(false); + close(); }; return ( @@ -113,8 +114,7 @@ export const GoCardlessInitialise = ({ variant="primary" isLoading={isLoading} onPress={() => { - onSubmit(); - close(); + onSubmit(close); }} > Save and continue diff --git a/packages/desktop-client/src/components/modals/ImportTransactions.jsx b/packages/desktop-client/src/components/modals/ImportTransactions.jsx index 56242cd4f..8fea03dbf 100644 --- a/packages/desktop-client/src/components/modals/ImportTransactions.jsx +++ b/packages/desktop-client/src/components/modals/ImportTransactions.jsx @@ -1089,7 +1089,7 @@ export function ImportTransactions({ options }) { setTransactions(newTransactions); } - async function onImport() { + async function onImport(close) { setLoadingState('importing'); const finalTransactions = []; @@ -1206,6 +1206,7 @@ export function ImportTransactions({ options }) { if (onImported) { onImported(didChange); } + close(); } const runImportPreviewCallback = useCallback(async () => { @@ -1682,8 +1683,7 @@ export function ImportTransactions({ options }) { } isLoading={loadingState === 'importing'} onPress={() => { - onImport(); - close(); + onImport(close); }} > Import{' '} diff --git a/packages/desktop-client/src/components/modals/SimpleFinInitialise.tsx b/packages/desktop-client/src/components/modals/SimpleFinInitialise.tsx index 2e5c356e2..21613af5c 100644 --- a/packages/desktop-client/src/components/modals/SimpleFinInitialise.tsx +++ b/packages/desktop-client/src/components/modals/SimpleFinInitialise.tsx @@ -28,7 +28,7 @@ export const SimpleFinInitialise = ({ const [isValid, setIsValid] = useState(true); const [isLoading, setIsLoading] = useState(false); - const onSubmit = async () => { + const onSubmit = async (close: () => void) => { if (!token) { setIsValid(false); return; @@ -43,6 +43,7 @@ export const SimpleFinInitialise = ({ onSuccess(); setIsLoading(false); + close(); }; return ( @@ -89,8 +90,7 @@ export const SimpleFinInitialise = ({ variant="primary" isLoading={isLoading} onPress={() => { - onSubmit(); - close(); + onSubmit(close); }} > Save and continue diff --git a/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx b/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx index 5c89c4fc2..995a8aa71 100644 --- a/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx +++ b/packages/desktop-client/src/components/schedules/ScheduleDetails.jsx @@ -353,7 +353,7 @@ export function ScheduleDetails({ id, transaction }) { transaction ? [transaction.id] : [], ); - async function onSave() { + async function onSave(close) { dispatch({ type: 'form-error', error: null }); if (state.fields.name) { const { data: sameName } = await runQuery( @@ -396,11 +396,14 @@ export function ScheduleDetails({ id, transaction }) { error: 'An error occurred while saving. Please visit https://actualbudget.org/contact/ for support.', }); - } else { - if (adding) { - await onLinkTransactions([...selectedInst.items], res.data); - } + return; } + + if (adding) { + await onLinkTransactions([...selectedInst.items], res.data); + } + + close(); } async function onEditRule(ruleId) { @@ -788,8 +791,7 @@ export function ScheduleDetails({ id, transaction }) { <Button variant="primary" onPress={() => { - onSave(); - close(); + onSave(close); }} > {adding ? 'Add' : 'Save'} diff --git a/packages/loot-core/src/server/accounts/rules.test.ts b/packages/loot-core/src/server/accounts/rules.test.ts index 8368131e6..556230081 100644 --- a/packages/loot-core/src/server/accounts/rules.test.ts +++ b/packages/loot-core/src/server/accounts/rules.test.ts @@ -12,6 +12,7 @@ import { const fieldTypes = new Map( Object.entries({ id: 'id', + account: 'id', date: 'date', name: 'string', category: 'string', @@ -322,6 +323,12 @@ describe('Action', () => { new Action(null, 'name', 'James', null, fieldTypes); }).toThrow(/invalid action operation/i); }); + + test('empty account values result in error', () => { + expect(() => { + new Action('set', 'account', '', null, fieldTypes); + }).toThrow(/Field cannot be empty/i); + }); }); describe('Rule', () => { diff --git a/packages/loot-core/src/server/accounts/rules.ts b/packages/loot-core/src/server/accounts/rules.ts index 4116cd010..a72c8b628 100644 --- a/packages/loot-core/src/server/accounts/rules.ts +++ b/packages/loot-core/src/server/accounts/rules.ts @@ -471,6 +471,10 @@ export class Action { this.type = 'id'; } + if (field === 'account') { + assert(value, 'no-null', `Field cannot be empty: ${field}`); + } + this.op = op; this.rawValue = value; this.value = value; diff --git a/packages/loot-core/src/server/rules/app.ts b/packages/loot-core/src/server/rules/app.ts index dd8bd11c6..01d37e1b7 100644 --- a/packages/loot-core/src/server/rules/app.ts +++ b/packages/loot-core/src/server/rules/app.ts @@ -15,22 +15,22 @@ function validateRule(rule: Partial<RuleEntity>) { // 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<T>(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; + const result = array.map(item => { + try { + validate(item); + } catch (e) { + if (e instanceof RuleError) { + console.warn('Invalid rule', e); + return e.type; } - return null; - }) - .filter((res): res is string => typeof res === 'string'); + throw e; + } + return null; + }); - return result.length ? result : null; + return result.filter((res): res is string => typeof res === 'string').length + ? result + : null; } const conditionErrors = runValidation( diff --git a/upcoming-release-notes/3149.md b/upcoming-release-notes/3149.md new file mode 100644 index 000000000..509736541 --- /dev/null +++ b/upcoming-release-notes/3149.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [MatissJanis] +--- + +Fix missing error handling in rules modal. -- GitLab