From 4e54c41728c996fe4b4dc76ff31b2584c16ec6ed Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 3 Jul 2026 10:21:14 +0000 Subject: [PATCH 1/2] fix(categories): allocate safe numeric ID when class list is empty _.max([]) returns undefined, so the previous expression `_.max(_.map(this.classes, 'id')) + 1` evaluated to NaN for an empty class list. Fix both callsites with `(_.max(...) ?? -1) + 1` which returns 0 when classes is empty, max+1 otherwise. Have addClass() return the assigned ID so CategoryEditTree no longer needs to recompute it with the same fragile expression. --- src/components/CategoryEditTree.vue | 4 +--- src/stores/categories.ts | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/components/CategoryEditTree.vue b/src/components/CategoryEditTree.vue index b7095d6f..ca0d04a1 100644 --- a/src/components/CategoryEditTree.vue +++ b/src/components/CategoryEditTree.vue @@ -85,13 +85,11 @@ export default { counter++; } - this.categoryStore.addClass({ + const lastId = this.categoryStore.addClass({ name: parent.name.concat([name]), rule: { type: 'regex', regex: 'FILL ME' }, }); - // Find the category with the max ID, and open an editor for it - const lastId = _.max(_.map(this.categoryStore.classes, 'id')); this.editingId = lastId; }, showEditModal: function () { diff --git a/src/stores/categories.ts b/src/stores/categories.ts index 7bdefbc4..dc972558 100644 --- a/src/stores/categories.ts +++ b/src/stores/categories.ts @@ -323,7 +323,7 @@ export const useCategoryStore = defineStore('categories', { const parent_depth = old_class.name.length; if (new_class.id === undefined || new_class.id === null) { - new_class.id = _.max(_.map(this.classes, 'id')) + 1; + new_class.id = (_.max(_.map(this.classes, 'id')) ?? -1) + 1; this.classes.push(new_class); } else { Object.assign(old_class, new_class); @@ -345,10 +345,11 @@ export const useCategoryStore = defineStore('categories', { this.classes_unsaved_changes = true; }, - addClass(this: State, new_class: Category) { - new_class.id = _.max(_.map(this.classes, 'id')) + 1; + addClass(this: State, new_class: Category): number { + new_class.id = (_.max(_.map(this.classes, 'id')) ?? -1) + 1; this.classes.push(new_class); this.classes_unsaved_changes = true; + return new_class.id; }, removeClass(this: State, classId: number) { this.classes = this.classes.filter((c: Category) => c.id !== classId); From 0d8d771c7bafeef72d20414ef4950dc41f7a5f08 Mon Sep 17 00:00:00 2001 From: Bob Date: Fri, 3 Jul 2026 10:25:51 +0000 Subject: [PATCH 2/2] fix(categories): use addClass return value in UI callers; add regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CategorizationSettings and CategoryBuilder were recomputing the new category ID with _.max(_.map(classes, 'id')) after calling addClass() — the same fragile expression that caused the NaN/null bug in #427. Use the ID returned by addClass() instead, removing the redundant (and now incorrect) recompute. Remove unused lodash import from CategorizationSettings. Add two regression tests: - addClass after clearAll assigns id 0, not NaN/null - addClass after partial deletion starts from max existing id --- src/views/settings/CategorizationSettings.vue | 5 +--- src/views/settings/CategoryBuilder.vue | 5 +--- test/unit/store/categories.test.node.ts | 25 +++++++++++++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/views/settings/CategorizationSettings.vue b/src/views/settings/CategorizationSettings.vue index 1dc19252..d3fe3d33 100644 --- a/src/views/settings/CategorizationSettings.vue +++ b/src/views/settings/CategorizationSettings.vue @@ -96,7 +96,6 @@ import 'vue-awesome/icons/angle-double-up'; import { useCategoryStore } from '~/stores/categories'; -import _ from 'lodash'; import { downloadFile } from '~/util/export'; export default { @@ -149,12 +148,10 @@ export default { }, methods: { addClass: function () { - this.categoryStore.addClass({ + const lastId = this.categoryStore.addClass({ name: ['New class'], rule: { type: 'regex', regex: 'FILL ME' }, }); - - const lastId = _.max(_.map(this.categoryStore.classes, 'id')); this.editingId = lastId; }, saveClasses: async function () { diff --git a/src/views/settings/CategoryBuilder.vue b/src/views/settings/CategoryBuilder.vue index 8731879c..99aa0689 100644 --- a/src/views/settings/CategoryBuilder.vue +++ b/src/views/settings/CategoryBuilder.vue @@ -291,13 +291,10 @@ export default { }, createRule(word: string) { console.log('Opening modal for creating rule with word: ' + word); - this.categoryStore.addClass({ + const lastId = this.categoryStore.addClass({ name: [word], rule: { type: 'regex', regex: _.escapeRegExp(word) }, }); - - // Find the category with the max ID, and open an editor for it - const lastId = _.max(_.map(this.categoryStore.classes, 'id')); this.create.word = word; this.create.categoryId = lastId; }, diff --git a/test/unit/store/categories.test.node.ts b/test/unit/store/categories.test.node.ts index 323c4a52..25238f3a 100644 --- a/test/unit/store/categories.test.node.ts +++ b/test/unit/store/categories.test.node.ts @@ -157,4 +157,29 @@ describe('categories store', () => { expect(decoded).toEqual(42); expect(encoded).toEqual(42); }); + + test('addClass after clearAll assigns id 0, not NaN/null', () => { + // Regression test for #427: _.max([]) returns undefined, so + // `_.max(_.map(this.classes, 'id')) + 1` evaluated to NaN for an empty + // class list. JSON.stringify(NaN) produces null, creating an uneditable category. + expect(categoryStore.classes).toHaveLength(0); + const id = categoryStore.addClass({ name: ['New class'], rule: { type: 'none' } }); + expect(id).toBe(0); + expect(categoryStore.classes[0].id).toBe(0); + // Verify the new category is immediately editable (updateClass should find it by id) + categoryStore.updateClass({ ...categoryStore.classes[0], name: ['Renamed'] }); + expect(categoryStore.classes[0].name).toEqual(['Renamed']); + }); + + test('addClass after partial deletion starts from max existing id', () => { + categoryStore.load([ + { name: ['A'], rule: { type: 'none' } }, + { name: ['B'], rule: { type: 'none' } }, + { name: ['C'], rule: { type: 'none' } }, + ]); + const maxBefore = Math.max(...categoryStore.classes.map(c => c.id ?? -1)); + categoryStore.removeClass(categoryStore.classes[1].id); + const id = categoryStore.addClass({ name: ['D'], rule: { type: 'none' } }); + expect(id).toBe(maxBefore + 1); + }); });