From 51a87095eedda5a1762ddea5be3f595ae0e32fde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85smund=20Collin?= Date: Mon, 27 Jul 2026 23:53:22 +0200 Subject: [PATCH] fix(macros): stop marking existing macros as deleted in the settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues made the macros settings mark macros as deleted or break the list entirely: - `allMacros` in the expert tab was filtered by the search field, but `existsMacro()` and `getMacroDescription()` used it for existence checks. Typing in the "available macros" search therefore labelled every non-matching macro of the edited group as a deleted macro and hid its color/visibility buttons. The search filter now lives in a separate `filteredMacros` getter used only by `availableMacros`. - `searchMacros` is reset to null by the clearable prop of v-text-field, so clicking the clear icon threw on `.toLowerCase()` and broke the rendering of both settings tabs. Additionally `printer/getMacros` defaulted `configfile.settings` to null and then indexed it unconditionally, which threw when the macro objects were known but configfile was not populated yet. Signed-off-by: Åsmund Collin Co-Authored-By: Claude Opus 5 --- .../settings/SettingsMacrosTabExpert.vue | 19 ++- .../settings/SettingsMacrosTabSimple.vue | 8 +- src/store/printer/getters.ts | 2 +- .../settings/settingsMacrosTabExpert.spec.ts | 143 ++++++++++++++++++ .../settings/settingsMacrosTabSimple.spec.ts | 118 +++++++++++++++ tests/store/printer/getMacros.spec.ts | 81 ++++++++++ 6 files changed, 360 insertions(+), 11 deletions(-) create mode 100644 tests/components/settings/settingsMacrosTabExpert.spec.ts create mode 100644 tests/components/settings/settingsMacrosTabSimple.spec.ts create mode 100644 tests/store/printer/getMacros.spec.ts diff --git a/src/components/settings/SettingsMacrosTabExpert.vue b/src/components/settings/SettingsMacrosTabExpert.vue index bb0f8eaf1f..82a6132c67 100644 --- a/src/components/settings/SettingsMacrosTabExpert.vue +++ b/src/components/settings/SettingsMacrosTabExpert.vue @@ -353,7 +353,7 @@ export default class SettingsMacrosTabExpert extends Mixins(BaseMixin, ThemeMixi private boolFormEdit = false private editGroupId: string | null = '' - private searchMacros: string = '' + private searchMacros: string | null = '' get groupColors() { return [ @@ -397,18 +397,23 @@ export default class SettingsMacrosTabExpert extends Mixins(BaseMixin, ThemeMixi return colors } - get allMacros() { - const macros = this.$store.getters['printer/getMacros'] ?? [] - return macros.filter((macro: PrinterStateMacro) => { + get allMacros(): PrinterStateMacro[] { + return this.$store.getters['printer/getMacros'] ?? [] + } + + get filteredMacros() { + const search = (this.searchMacros ?? '').toLowerCase() + + return this.allMacros.filter((macro: PrinterStateMacro) => { return ( - macro.name.toLowerCase().includes(this.searchMacros.toLowerCase()) || - macro.description?.toLowerCase().includes(this.searchMacros.toLowerCase()) + macro.name.toLowerCase().includes(search) || + (macro.description?.toLowerCase().includes(search) ?? false) ) }) } get availableMacros() { - return this.allMacros.filter((m: GuiMacrosStateMacrogroupMacro) => !this.editGroupUsedMacros.includes(m.name)) + return this.filteredMacros.filter((m: PrinterStateMacro) => !this.editGroupUsedMacros.includes(m.name)) } get groups() { diff --git a/src/components/settings/SettingsMacrosTabSimple.vue b/src/components/settings/SettingsMacrosTabSimple.vue index 5fcea23b14..f66a7f3311 100644 --- a/src/components/settings/SettingsMacrosTabSimple.vue +++ b/src/components/settings/SettingsMacrosTabSimple.vue @@ -52,14 +52,16 @@ import { PrinterStateMacro } from '@/store/printer/types' }) export default class SettingsMacrosTabSimple extends Mixins(BaseMixin) { mdiMagnify = mdiMagnify - searchMacros: string = '' + searchMacros: string | null = '' get macros() { + const search = (this.searchMacros ?? '').toLowerCase() const macros = this.$store.getters['printer/getMacros'] ?? [] + return macros.filter((macro: PrinterStateMacro) => { return ( - macro.name.toLowerCase().includes(this.searchMacros.toLowerCase()) || - macro.description?.toLowerCase().includes(this.searchMacros.toLowerCase()) + macro.name.toLowerCase().includes(search) || + (macro.description?.toLowerCase().includes(search) ?? false) ) }) } diff --git a/src/store/printer/getters.ts b/src/store/printer/getters.ts index fc242a2c01..a065f59e11 100644 --- a/src/store/printer/getters.ts +++ b/src/store/printer/getters.ts @@ -144,7 +144,7 @@ export const getters: GetterTree = { getMacros: (state) => { const array: PrinterStateMacro[] = [] - const settings = state.configfile?.settings ?? null + const settings = state.configfile?.settings ?? {} const printerGcodes = state.gcode?.commands ?? {} const prefix = 'gcode_macro ' diff --git a/tests/components/settings/settingsMacrosTabExpert.spec.ts b/tests/components/settings/settingsMacrosTabExpert.spec.ts new file mode 100644 index 0000000000..8a05a5ad5a --- /dev/null +++ b/tests/components/settings/settingsMacrosTabExpert.spec.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from 'vitest' +import SettingsMacrosTabExpert from '@/components/settings/SettingsMacrosTabExpert.vue' +import type { PrinterStateMacro } from '@/store/printer/types' +import type { GuiMacrosStateMacrogroup } from '@/store/gui/macros/types' + +type ComponentOptions = { + macros?: Partial[] + group?: Partial + search?: string | null +} + +interface MacrosTabExpert { + searchMacros: string | null + editGroupId: string | null + allMacros: PrinterStateMacro[] + filteredMacros: PrinterStateMacro[] + availableMacros: PrinterStateMacro[] + existsMacro(macroname: string): boolean + getMacroDescription(macroname: string): string | null +} + +const MacrosTabExpertClass = SettingsMacrosTabExpert as unknown as new () => MacrosTabExpert + +const createComponent = (options: ComponentOptions = {}) => { + const component = new MacrosTabExpertClass() + + Object.defineProperty(component, '$store', { + value: { + getters: { + 'printer/getMacros': options.macros ?? [], + 'gui/macros/getMacrogroup': () => options.group, + }, + }, + }) + + Object.defineProperty(component, '$t', { value: (key: string) => key }) + + component.searchMacros = 'search' in options ? (options.search as string | null) : '' + component.editGroupId = 'group-1' + + return component +} + +const macroNames = (macros: PrinterStateMacro[]) => macros.map((macro) => macro.name) + +describe('SettingsMacrosTabExpert', () => { + describe('the search field only filters the available macros', () => { + it('keeps a group macro recognized while the search hides it', () => { + const component = createComponent({ + macros: [{ name: 'START_PRINT' }, { name: 'END_PRINT' }], + search: 'START', + }) + + expect(component.existsMacro('END_PRINT')).toBe(true) + }) + + it('keeps returning the real description of a macro hidden by the search', () => { + const component = createComponent({ + macros: [ + { name: 'START_PRINT', description: 'Heats up and homes' }, + { name: 'END_PRINT', description: 'Parks the toolhead' }, + ], + search: 'START', + }) + + expect(component.getMacroDescription('END_PRINT')).toBe('Parks the toolhead') + }) + + it('still narrows the available macros list by name', () => { + const component = createComponent({ + macros: [{ name: 'START_PRINT' }, { name: 'END_PRINT' }], + search: 'end', + }) + + expect(macroNames(component.availableMacros)).toStrictEqual(['END_PRINT']) + }) + + it('still narrows the available macros list by description', () => { + const component = createComponent({ + macros: [ + { name: 'START_PRINT', description: 'Heats up and homes' }, + { name: 'END_PRINT', description: 'Parks the toolhead' }, + ], + search: 'parks', + }) + + expect(macroNames(component.availableMacros)).toStrictEqual(['END_PRINT']) + }) + + it('excludes macros already used in the edited group', () => { + const component = createComponent({ + macros: [{ name: 'START_PRINT' }, { name: 'END_PRINT' }], + group: { macros: [{ name: 'START_PRINT', pos: 1 }] as GuiMacrosStateMacrogroup['macros'] }, + }) + + expect(macroNames(component.availableMacros)).toStrictEqual(['END_PRINT']) + }) + }) + + describe('a cleared search field', () => { + it('does not throw when the search is null', () => { + const component = createComponent({ + macros: [{ name: 'START_PRINT' }], + search: null, + }) + + expect(() => component.availableMacros).not.toThrow() + expect(macroNames(component.availableMacros)).toStrictEqual(['START_PRINT']) + }) + + it('does not throw when a macro has no description', () => { + const component = createComponent({ + macros: [{ name: 'START_PRINT', description: null }], + search: 'nomatch', + }) + + expect(() => component.availableMacros).not.toThrow() + expect(component.availableMacros).toStrictEqual([]) + }) + }) + + describe('deleted macro detection', () => { + it('reports a macro that is no longer in the config as deleted', () => { + const component = createComponent({ macros: [{ name: 'START_PRINT' }] }) + + expect(component.existsMacro('REMOVED_MACRO')).toBe(false) + expect(component.getMacroDescription('REMOVED_MACRO')).toBe('Settings.MacrosTab.DeletedMacro') + }) + + it('matches macro names case-insensitively', () => { + const component = createComponent({ macros: [{ name: 'Start_Print' }] }) + + expect(component.existsMacro('START_PRINT')).toBe(true) + expect(component.existsMacro('start_print')).toBe(true) + }) + + it('returns null instead of a description when the macro has no help text', () => { + const component = createComponent({ macros: [{ name: 'START_PRINT' }] }) + + expect(component.getMacroDescription('START_PRINT')).toBeNull() + }) + }) +}) diff --git a/tests/components/settings/settingsMacrosTabSimple.spec.ts b/tests/components/settings/settingsMacrosTabSimple.spec.ts new file mode 100644 index 0000000000..f0c97acc65 --- /dev/null +++ b/tests/components/settings/settingsMacrosTabSimple.spec.ts @@ -0,0 +1,118 @@ +import { describe, expect, it, vi } from 'vitest' +import SettingsMacrosTabSimple from '@/components/settings/SettingsMacrosTabSimple.vue' +import type { PrinterStateMacro } from '@/store/printer/types' + +type ComponentOptions = { + macros?: Partial[] + hiddenMacros?: string[] + search?: string | null +} + +interface MacrosTabSimple { + searchMacros: string | null + macros: PrinterStateMacro[] + hiddenMacros: string[] + getMacroStatus(name: string): boolean + changeMacroStatus(name: string): void +} + +const MacrosTabSimpleClass = SettingsMacrosTabSimple as unknown as new () => MacrosTabSimple + +const createComponent = (options: ComponentOptions = {}) => { + const dispatch = vi.fn() + const component = new MacrosTabSimpleClass() + + Object.defineProperty(component, '$store', { + value: { + state: { + gui: { macros: { hiddenMacros: options.hiddenMacros ?? [] } }, + }, + getters: { + 'printer/getMacros': options.macros ?? [], + }, + dispatch, + }, + }) + + component.searchMacros = 'search' in options ? (options.search as string | null) : '' + + return { component, dispatch } +} + +const macroNames = (macros: PrinterStateMacro[]) => macros.map((macro) => macro.name) + +describe('SettingsMacrosTabSimple', () => { + describe('search', () => { + it('filters by macro name', () => { + const { component } = createComponent({ + macros: [{ name: 'START_PRINT' }, { name: 'END_PRINT' }], + search: 'end', + }) + + expect(macroNames(component.macros)).toStrictEqual(['END_PRINT']) + }) + + it('filters by macro description', () => { + const { component } = createComponent({ + macros: [ + { name: 'START_PRINT', description: 'Heats up and homes' }, + { name: 'END_PRINT', description: 'Parks the toolhead' }, + ], + search: 'parks', + }) + + expect(macroNames(component.macros)).toStrictEqual(['END_PRINT']) + }) + + it('does not throw and lists every macro when the search is null', () => { + const { component } = createComponent({ + macros: [{ name: 'START_PRINT' }, { name: 'END_PRINT' }], + search: null, + }) + + expect(() => component.macros).not.toThrow() + expect(macroNames(component.macros)).toStrictEqual(['START_PRINT', 'END_PRINT']) + }) + + it('does not throw when a macro has no description', () => { + const { component } = createComponent({ + macros: [{ name: 'START_PRINT', description: null }], + search: 'nomatch', + }) + + expect(() => component.macros).not.toThrow() + expect(component.macros).toStrictEqual([]) + }) + }) + + describe('hiding macros', () => { + it('reports a macro as enabled when it is not hidden', () => { + const { component } = createComponent({ hiddenMacros: ['END_PRINT'] }) + + expect(component.getMacroStatus('START_PRINT')).toBe(true) + expect(component.getMacroStatus('END_PRINT')).toBe(false) + }) + + it('hides a visible macro', () => { + const { component, dispatch } = createComponent({ hiddenMacros: [] }) + + component.changeMacroStatus('Start_Print') + + expect(dispatch).toHaveBeenCalledWith('gui/macros/saveSetting', { + name: 'hiddenMacros', + value: ['START_PRINT'], + }) + }) + + it('unhides an already hidden macro', () => { + const { component, dispatch } = createComponent({ hiddenMacros: ['START_PRINT', 'END_PRINT'] }) + + component.changeMacroStatus('START_PRINT') + + expect(dispatch).toHaveBeenCalledWith('gui/macros/saveSetting', { + name: 'hiddenMacros', + value: ['END_PRINT'], + }) + }) + }) +}) diff --git a/tests/store/printer/getMacros.spec.ts b/tests/store/printer/getMacros.spec.ts new file mode 100644 index 0000000000..7ce390d8cd --- /dev/null +++ b/tests/store/printer/getMacros.spec.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest' +import { getters } from '@/store/printer/getters' +import type { PrinterState, PrinterStateMacro } from '@/store/printer/types' +import type { RootState } from '@/store/types' + +const runGetter = (state: Record): PrinterStateMacro[] => { + return getters.getMacros(state as unknown as PrinterState, {}, {} as RootState, {}) +} + +describe('printer/getMacros', () => { + it('returns the gcode_macro objects with their help text as description', () => { + const macros = runGetter({ + configfile: { settings: { 'gcode_macro start_print': { gcode: 'G28' } } }, + gcode: { commands: { START_PRINT: { help: 'Heats up and homes' } } }, + 'gcode_macro START_PRINT': { bed_temp: 60 }, + }) + + expect(macros).toHaveLength(1) + expect(macros[0].name).toBe('START_PRINT') + expect(macros[0].description).toBe('Heats up and homes') + expect(macros[0].variables).toStrictEqual({ bed_temp: 60 }) + }) + + it('sorts the macros case-insensitively', () => { + const macros = runGetter({ + configfile: { settings: {} }, + 'gcode_macro zzz_macro': {}, + 'gcode_macro Alpha': {}, + 'gcode_macro beta': {}, + }) + + expect(macros.map((macro) => macro.name)).toStrictEqual(['Alpha', 'beta', 'zzz_macro']) + }) + + it('hides macros starting with an underscore', () => { + const macros = runGetter({ + configfile: { settings: {} }, + 'gcode_macro _INTERNAL': {}, + 'gcode_macro START_PRINT': {}, + }) + + expect(macros.map((macro) => macro.name)).toStrictEqual(['START_PRINT']) + }) + + it('hides macros that override an existing command via rename_existing', () => { + const macros = runGetter({ + configfile: { + settings: { + 'gcode_macro pause': { rename_existing: 'BASE_PAUSE' }, + }, + }, + 'gcode_macro PAUSE': {}, + 'gcode_macro START_PRINT': {}, + }) + + expect(macros.map((macro) => macro.name)).toStrictEqual(['START_PRINT']) + }) + + it('sets the description to null when the macro has no help text', () => { + const macros = runGetter({ + configfile: { settings: {} }, + gcode: { commands: {} }, + 'gcode_macro START_PRINT': {}, + }) + + expect(macros[0].description).toBeNull() + }) + + it('does not throw when configfile is missing', () => { + expect(() => runGetter({ 'gcode_macro START_PRINT': {} })).not.toThrow() + expect(runGetter({ 'gcode_macro START_PRINT': {} }).map((macro) => macro.name)).toStrictEqual(['START_PRINT']) + }) + + it('does not throw when configfile.settings is missing', () => { + expect(() => runGetter({ configfile: {}, 'gcode_macro START_PRINT': {} })).not.toThrow() + }) + + it('returns an empty list when no printer objects are loaded', () => { + expect(runGetter({})).toStrictEqual([]) + }) +})