Refactor createNote (#1789)

* Update createNote to return a wrapper object

* Update how createNote creates errors

4 files changed, 44 insertions, 49 deletions

diff --git a/ext/js/data/anki-note-builder.js b/ext/js/data/anki-note-builder.js
index c36ad889..4427c276 100644
--- a/ext/js/data/anki-note-builder.js
+++ b/ext/js/data/anki-note-builder.js
@@ -42,8 +42,7 @@ class AnkiNoteBuilder {
         duplicateScope='collection',
         resultOutputMode='split',
         glossaryLayoutMode='default',
-        compactTags=false,
-        errors=null
+        compactTags=false
     }) {
         let duplicateScopeDeckName = null;
         let duplicateScopeCheckChildren = false;
@@ -53,6 +52,7 @@ class AnkiNoteBuilder {
             duplicateScopeCheckChildren = true;
         }
 
+        const errors = [];
         const commonData = this._createData(dictionaryEntry, mode, context, resultOutputMode, glossaryLayoutMode, compactTags, injectedMedia);
         const formattedFieldValuePromises = [];
         for (const [, fieldValue] of fields) {
@@ -68,7 +68,7 @@ class AnkiNoteBuilder {
             noteFields[fieldName] = formattedFieldValue;
         }
 
-        return {
+        const note = {
             fields: noteFields,
             tags,
             deckName,
@@ -82,6 +82,7 @@ class AnkiNoteBuilder {
                 }
             }
         };
+        return {note, errors};
     }
 
     async getRenderingData({
@@ -112,17 +113,15 @@ class AnkiNoteBuilder {
         };
     }
 
-    async _formatField(field, commonData, template, errors=null) {
+    async _formatField(field, commonData, template, errors) {
         return await this._stringReplaceAsync(field, this._markerPattern, async (g0, marker) => {
             try {
                 const {result} = await this._renderTemplateBatched(template, commonData, marker);
                 return result;
             } catch (e) {
-                if (Array.isArray(errors)) {
-                    const error = new Error(`Template render error for {${marker}}`);
-                    error.data = {error: e};
-                    errors.push(error);
-                }
+                const error = new Error(`Template render error for {${marker}}`);
+                error.data = {error: e};
+                errors.push(error);
                 return `{${marker}-render-error}`;
             }
         });
diff --git a/ext/js/display/display.js b/ext/js/display/display.js
index 7ede1384..bb7ced66 100644
--- a/ext/js/display/display.js
+++ b/ext/js/display/display.js
@@ -1259,12 +1259,13 @@ class Display extends EventDispatcher {
 
         this._hideAnkiNoteErrors(true);
 
-        const errors = [];
+        const allErrors = [];
         const overrideToken = this._progressIndicatorVisible.setOverride(true);
         try {
             const {anki: {suspendNewCards}} = this._options;
             const noteContext = this._getNoteContext();
-            const note = await this._createNote(dictionaryEntry, mode, noteContext, true, errors);
+            const {note, errors} = await this._createNote(dictionaryEntry, mode, noteContext, true);
+            allErrors.push(...errors);
 
             let noteId = null;
             let addNoteOkay = false;
@@ -1272,19 +1273,19 @@ class Display extends EventDispatcher {
                 noteId = await yomichan.api.addAnkiNote(note);
                 addNoteOkay = true;
             } catch (e) {
-                errors.length = 0;
-                errors.push(e);
+                allErrors.length = 0;
+                allErrors.push(e);
             }
 
             if (addNoteOkay) {
                 if (noteId === null) {
-                    errors.push(new Error('Note could not be added'));
+                    allErrors.push(new Error('Note could not be added'));
                 } else {
                     if (suspendNewCards) {
                         try {
                             await yomichan.api.suspendAnkiCardsForNote(noteId);
                         } catch (e) {
-                            errors.push(e);
+                            allErrors.push(e);
                         }
                     }
                     button.disabled = true;
@@ -1292,13 +1293,13 @@ class Display extends EventDispatcher {
                 }
             }
         } catch (e) {
-            errors.push(e);
+            allErrors.push(e);
         } finally {
             this._progressIndicatorVisible.clearOverride(overrideToken);
         }
 
-        if (errors.length > 0) {
-            this._showAnkiNoteErrors(errors);
+        if (allErrors.length > 0) {
+            this._showAnkiNoteErrors(allErrors);
         } else {
             this._hideAnkiNoteErrors(true);
         }
@@ -1480,11 +1481,11 @@ class Display extends EventDispatcher {
         const notePromises = [];
         for (const dictionaryEntry of dictionaryEntries) {
             for (const mode of modes) {
-                const notePromise = this._createNote(dictionaryEntry, mode, context, false, null);
+                const notePromise = this._createNote(dictionaryEntry, mode, context, false);
                 notePromises.push(notePromise);
             }
         }
-        const notes = await Promise.all(notePromises);
+        const notes = (await Promise.all(notePromises)).map(({note}) => note);
 
         let infos;
         if (forceCanAddValue !== null) {
@@ -1512,7 +1513,7 @@ class Display extends EventDispatcher {
         return results;
     }
 
-    async _createNote(dictionaryEntry, mode, context, injectMedia, errors) {
+    async _createNote(dictionaryEntry, mode, context, injectMedia) {
         const options = this._options;
         const template = this._ankiFieldTemplates;
         const {
@@ -1524,18 +1525,17 @@ class Display extends EventDispatcher {
         const {deck: deckName, model: modelName} = modeOptions;
         const fields = Object.entries(modeOptions.fields);
 
+        const errors = [];
         let injectedMedia = null;
         if (injectMedia) {
             let errors2;
             ({result: injectedMedia, errors: errors2} = await this._injectAnkiNoteMedia(dictionaryEntry, options, fields));
-            if (Array.isArray(errors)) {
-                for (const error of errors2) {
-                    errors.push(deserializeError(error));
-                }
+            for (const error of errors2) {
+                errors.push(deserializeError(error));
             }
         }
 
-        return await this._ankiNoteBuilder.createNote({
+        const {note, errors: createNoteErrors} = await this._ankiNoteBuilder.createNote({
             dictionaryEntry,
             mode,
             context,
@@ -1552,6 +1552,8 @@ class Display extends EventDispatcher {
             injectedMedia,
             errors
         });
+        errors.push(...createNoteErrors);
+        return {note, errors};
     }
 
     async _injectAnkiNoteMedia(dictionaryEntry, options, fields) {
@@ -1959,20 +1961,16 @@ class Display extends EventDispatcher {
         const ankiNotes = [];
         const modes = this._getModes(dictionaryEntry.type === 'term');
         for (const mode of modes) {
-            let ankiNote;
-            let ankiNoteException;
-            const errors = [];
+            let note;
+            let errors;
             try {
                 const noteContext = this._getNoteContext();
-                ankiNote = await this._createNote(dictionaryEntry, mode, noteContext, false, errors);
+                ({note: note, errors} = await this._createNote(dictionaryEntry, mode, noteContext, false));
             } catch (e) {
-                ankiNoteException = e;
-            }
-            const entry = {mode, ankiNote};
-            if (typeof ankiNoteException !== 'undefined') {
-                entry.ankiNoteException = ankiNoteException;
+                errors = [e];
             }
-            if (errors.length > 0) {
+            const entry = {mode, note};
+            if (Array.isArray(errors) && errors.length > 0) {
                 entry.errors = errors;
             }
             ankiNotes.push(entry);
diff --git a/ext/js/pages/settings/anki-templates-controller.js b/ext/js/pages/settings/anki-templates-controller.js
index 9502a2fd..aa565bad 100644
--- a/ext/js/pages/settings/anki-templates-controller.js
+++ b/ext/js/pages/settings/anki-templates-controller.js
@@ -147,8 +147,8 @@ class AnkiTemplatesController {
     }
 
     async _validate(infoNode, field, mode, showSuccessResult, invalidateInput) {
+        const allErrors = [];
         const text = this._renderTextInput.value || '';
-        const errors = [];
         let result = `No definition found for ${text}`;
         try {
             const optionsContext = this._settingsController.getOptionsContext();
@@ -168,7 +168,7 @@ class AnkiTemplatesController {
                 let template = options.anki.fieldTemplates;
                 if (typeof template !== 'string') { template = this._defaultFieldTemplates; }
                 const {general: {resultOutputMode, glossaryLayoutMode, compactTags}} = options;
-                const note = await this._ankiNoteBuilder.createNote({
+                const {note, errors} = await this._ankiNoteBuilder.createNote({
                     dictionaryEntry,
                     mode,
                     context,
@@ -180,13 +180,13 @@ class AnkiTemplatesController {
                     ],
                     resultOutputMode,
                     glossaryLayoutMode,
-                    compactTags,
-                    errors
+                    compactTags
                 });
                 result = note.fields.field;
+                allErrors.push(...errors);
             }
         } catch (e) {
-            errors.push(e);
+            allErrors.push(e);
         }
 
         const errorToMessageString = (e) => {
@@ -205,9 +205,9 @@ class AnkiTemplatesController {
             return `${e}`;
         };
 
-        const hasError = errors.length > 0;
+        const hasError = allErrors.length > 0;
         infoNode.hidden = !(showSuccessResult || hasError);
-        infoNode.textContent = hasError ? errors.map(errorToMessageString).join('\n') : (showSuccessResult ? result : '');
+        infoNode.textContent = hasError ? allErrors.map(errorToMessageString).join('\n') : (showSuccessResult ? result : '');
         infoNode.classList.toggle('text-danger', hasError);
         if (invalidateInput) {
             this._fieldTemplatesTextarea.dataset.invalid = `${hasError}`;
diff --git a/test/test-anki-note-builder.js b/test/test-anki-note-builder.js
index 2517e521..97489aa8 100644
--- a/test/test-anki-note-builder.js
+++ b/test/test-anki-note-builder.js
@@ -219,8 +219,7 @@ async function getRenderResults(dictionaryEntries, type, mode, template, AnkiNot
             query: 'query',
             fullQuery: 'fullQuery'
         };
-        const errors = [];
-        const noteFields = (await ankiNoteBuilder.createNote({
+        const {note: {fields: noteFields}, errors} = await ankiNoteBuilder.createNote({
             dictionaryEntry,
             mode: null,
             context,
@@ -234,11 +233,10 @@ async function getRenderResults(dictionaryEntries, type, mode, template, AnkiNot
             duplicateScope: 'collection',
             resultOutputMode: mode,
             glossaryLayoutMode: 'default',
-            compactTags: false,
-            errors
-        })).fields;
+            compactTags: false
+        });
         if (!write) {
-            assert.deepStrictEqual(errors, []);
+            assert.strictEqual(errors.length, 0);
         }
         results.push(noteFields);
     }