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 8844739a490..d80da0f072d 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 2256bc28775..ad092f26da5 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 c48a6e175fe..cd783296bb2 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 3ba1ea3d0f9..fc91a66a1a9 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 56242cd4f56..8fea03dbf0c 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 2e5c356e237..21613af5c1f 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 5c89c4fc275..995a8aa7163 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 8368131e657..556230081ab 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 4116cd01048..a72c8b628cd 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 dd8bd11c682..01d37e1b764 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 00000000000..50973654132
--- /dev/null
+++ b/upcoming-release-notes/3149.md
@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [MatissJanis]
+---
+
+Fix missing error handling in rules modal.