From 49adbfdbb3967f38abd39303ba032f36954d4322 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Sun, 30 Aug 2026 12:01:06 +0600 Subject: [PATCH] fix(vue): keep nested product category selection across lazy tree loads ResourceCategoryTree was emitting a truncated selection after loadRoot, so nested additional categories disappeared from the hidden field and were wiped from ms3_product_categories on the next save. --- .../components/ResourceCategoryTree.test.js | 222 ++++++++++++++++++ .../src/components/ResourceCategoryTree.vue | 78 ++++-- vueManager/src/test/stubs/useLexicon.js | 3 + vueManager/vitest.config.js | 3 + 4 files changed, 286 insertions(+), 20 deletions(-) create mode 100644 vueManager/src/components/ResourceCategoryTree.test.js create mode 100644 vueManager/src/test/stubs/useLexicon.js diff --git a/vueManager/src/components/ResourceCategoryTree.test.js b/vueManager/src/components/ResourceCategoryTree.test.js new file mode 100644 index 000000000..d8782021b --- /dev/null +++ b/vueManager/src/components/ResourceCategoryTree.test.js @@ -0,0 +1,222 @@ +/* eslint-disable vue/one-component-per-file -- PrimeVue stubs for unit tests */ +import { flushPromises, mount } from '@vue/test-utils' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { defineComponent, h, ref } from 'vue' + +import request from '../request.js' +import ResourceCategoryTree from './ResourceCategoryTree.vue' + +vi.mock('../request.js', () => ({ + default: { + get: vi.fn(), + }, +})) + +const TreeStub = defineComponent({ + name: 'TreeStub', + props: { value: { type: Array, default: () => [] } }, + setup(props, { slots }) { + return () => + h( + 'div', + { class: 'tree-stub' }, + (props.value || []).map(node => slots.default?.({ node })) + ) + }, +}) + +const CheckboxStub = defineComponent({ + name: 'CheckboxStub', + props: { + modelValue: { type: Boolean, default: false }, + disabled: { type: Boolean, default: false }, + inputId: { type: String, default: '' }, + }, + emits: ['update:modelValue'], + setup(props, { emit }) { + return () => + h('input', { + type: 'checkbox', + class: 'tree-checkbox-stub', + checked: props.modelValue, + disabled: props.disabled, + id: props.inputId, + onChange: event => emit('update:modelValue', event.target.checked), + }) + }, +}) + +const ContextMenuStub = defineComponent({ + name: 'ContextMenu', + setup(_, { expose }) { + expose({ show: vi.fn() }) + return () => h('div', { class: 'context-menu-stub' }) + }, +}) + +const globalStubs = { + Tree: TreeStub, + Checkbox: CheckboxStub, + ContextMenu: ContextMenuStub, +} + +function categoryRow(id, overrides = {}) { + return { + id, + label: `Category ${id}`, + leaf: true, + checked: false, + selectable: true, + locked: false, + class_key: 'MiniShop3\\Model\\msCategory', + published: 1, + hidemenu: 0, + ...overrides, + } +} + +function mockCategoryRows(...rows) { + request.get.mockResolvedValue({ results: rows }) +} + +function mountTree(options = {}) { + const { + modelValue = [], + lockedIds = [], + apiUrl = '/api/mgr/product-data/1/categories/tree', + apiParams = {}, + onUpdateModelValue, + } = options + + const selected = ref([...modelValue]) + const emitLog = [] + + // Close the real v-model loop (emit → props update → watch) so #546 recursion is detectable. + const Host = defineComponent({ + setup() { + return () => + h(ResourceCategoryTree, { + modelValue: selected.value, + apiUrl, + apiParams, + lockedIds, + inputIdPrefix: 'test-cat-', + 'onUpdate:modelValue': value => { + selected.value = [...value] + emitLog.push([...value]) + onUpdateModelValue?.(value) + }, + }) + }, + }) + + const wrapper = mount(Host, { + global: { + stubs: globalStubs, + }, + }) + + return { wrapper, selected, emitLog } +} + +describe('ResourceCategoryTree', () => { + afterEach(() => { + vi.clearAllMocks() + }) + + it('keeps nestedDeep in selection after loadRoot when root page omits it (#641)', async () => { + const parentId = 10 + const nestedDeepId = 999 + + mockCategoryRows(categoryRow(parentId, { checked: true, locked: true })) + + const { selected, emitLog } = mountTree({ + modelValue: [parentId, nestedDeepId], + lockedIds: [parentId], + }) + + await flushPromises() + + expect(selected.value).toEqual(expect.arrayContaining([parentId, nestedDeepId])) + expect(selected.value).toHaveLength(2) + for (const ids of emitLog) { + expect(ids).toContain(nestedDeepId) + } + }) + + it('removes a visible category from modelValue when unchecked', async () => { + const removableId = 20 + + mockCategoryRows(categoryRow(removableId, { checked: true })) + + const { wrapper, selected } = mountTree({ + modelValue: [removableId], + }) + + await flushPromises() + + const checkbox = wrapper.find(`#test-cat-${removableId}`) + expect(checkbox.exists()).toBe(true) + expect(checkbox.element.checked).toBe(true) + + await checkbox.setValue(false) + await flushPromises() + + expect(selected.value).not.toContain(removableId) + expect(selected.value).toHaveLength(0) + }) + + it('enforces locked ids and emits when parent omitted them', async () => { + const lockedId = 10 + + mockCategoryRows(categoryRow(lockedId, { checked: true, locked: true })) + + const { selected, emitLog } = mountTree({ + modelValue: [], + lockedIds: [lockedId], + }) + + await flushPromises() + + expect(selected.value).toEqual([lockedId]) + expect(emitLog.length).toBeGreaterThan(0) + expect(emitLog.at(-1)).toEqual([lockedId]) + }) + + it('does not emit recursively when modelValue already includes locked ids (#546)', async () => { + const lockedId = 10 + const nestedDeepId = 999 + + mockCategoryRows(categoryRow(lockedId, { checked: true, locked: true })) + + const { emitLog } = mountTree({ + modelValue: [lockedId, nestedDeepId], + lockedIds: [lockedId], + }) + + await flushPromises() + + expect(emitLog.length).toBeLessThanOrEqual(1) + }) + + it('cannot uncheck a locked category', async () => { + const lockedId = 10 + + mockCategoryRows(categoryRow(lockedId, { checked: true, locked: true })) + + const { wrapper, selected } = mountTree({ + modelValue: [lockedId], + lockedIds: [lockedId], + }) + + await flushPromises() + + const checkbox = wrapper.find(`#test-cat-${lockedId}`) + expect(checkbox.element.disabled).toBe(true) + + await checkbox.setValue(false) + await flushPromises() + + expect(selected.value).toEqual([lockedId]) + }) +}) diff --git a/vueManager/src/components/ResourceCategoryTree.vue b/vueManager/src/components/ResourceCategoryTree.vue index bfc679403..bea6603c4 100644 --- a/vueManager/src/components/ResourceCategoryTree.vue +++ b/vueManager/src/components/ResourceCategoryTree.vue @@ -34,7 +34,15 @@ const contextMenu = ref(null) const contextNode = ref(null) const checkedSet = ref(new Set()) -const lockedSet = computed(() => new Set(props.lockedIds.map(id => Number(id)))) +/** @returns {number|null} Positive category id, or null if invalid. */ +function toCategoryId(value) { + const id = Number(value) + return Number.isFinite(id) && id > 0 ? id : null +} + +const lockedSet = computed( + () => new Set(props.lockedIds.map(toCategoryId).filter(id => id !== null)) +) const contextMenuItems = computed(() => [ { @@ -88,48 +96,73 @@ function toTreeNode(row) { data: { class_key: row.class_key, selectable: row.selectable !== false, - locked: isApiRowLocked(row), + locked: isLockedId(row.id, row.locked), published: row.published, hidemenu: row.hidemenu, }, } } +function normalizeIdList(ids) { + if (!Array.isArray(ids)) { + return [] + } + return ids.map(toCategoryId).filter(id => id !== null) +} + +function nodeId(node) { + return toCategoryId(node?.id) +} + +function isLockedId(id, flaggedLocked = false) { + const normalized = toCategoryId(id) + return Boolean(flaggedLocked) || (normalized !== null && lockedSet.value.has(normalized)) +} + +function applyLockedIds(base) { + const next = new Set(base) + for (const id of lockedSet.value) { + next.add(id) + } + return next +} + function mergeCheckedFromApi(rows) { const next = new Set(checkedSet.value) for (const row of rows) { - if (row.checked || lockedSet.value.has(Number(row.id))) { - next.add(row.id) + const id = toCategoryId(row.id) + if (id !== null && (row.checked || isLockedId(id, row.locked))) { + next.add(id) } } checkedSet.value = next } -function isApiRowLocked(row) { - return Boolean(row.locked) || lockedSet.value.has(Number(row.id)) -} - function isSelectableNode(node) { return !!node?.data?.selectable } function isLockedNode(node) { - return !!node?.data?.locked || lockedSet.value.has(Number(node?.id)) + return isLockedId(node?.id, node?.data?.locked) } function isChecked(node) { - return checkedSet.value.has(node.id) + return checkedSet.value.has(nodeId(node)) } function toggleNode(node, checked) { if (!isSelectableNode(node) || (isLockedNode(node) && !checked)) { return } + const id = nodeId(node) + if (id === null) { + return + } const next = new Set(checkedSet.value) if (checked) { - next.add(node.id) + next.add(id) } else { - next.delete(node.id) + next.delete(id) } checkedSet.value = next emitSelection() @@ -146,8 +179,10 @@ async function loadRoot() { } function ensureLockedChecked() { - const next = new Set(checkedSet.value) - lockedSet.value.forEach(id => next.add(id)) + const next = applyLockedIds(checkedSet.value) + if (next.size === checkedSet.value.size) { + return + } checkedSet.value = next emitSelection() } @@ -211,10 +246,14 @@ async function bulkToggleChecks(node, checked) { const next = new Set(checkedSet.value) function walk(n) { if (isSelectableNode(n)) { + const id = nodeId(n) + if (id === null) { + return + } if (checked || isLockedNode(n)) { - next.add(n.id) + next.add(id) } else { - next.delete(n.id) + next.delete(id) } } if (Array.isArray(n.children)) { @@ -255,15 +294,14 @@ watch( // Calling emitSelection() unconditionally here echoes the value we just received, // which reassigns props.modelValue and retriggers this watch — an infinite recursive // loop that freezes the page (#546). - const incoming = new Set(newIds || []) - const next = new Set(incoming) - lockedSet.value.forEach(id => next.add(id)) + const incoming = new Set(normalizeIdList(newIds)) + const next = applyLockedIds(incoming) checkedSet.value = next if (next.size !== incoming.size) { emitSelection() } }, - { deep: true } + { deep: true, immediate: true } ) watch(lockedSet, () => ensureLockedChecked()) diff --git a/vueManager/src/test/stubs/useLexicon.js b/vueManager/src/test/stubs/useLexicon.js new file mode 100644 index 000000000..ea0223007 --- /dev/null +++ b/vueManager/src/test/stubs/useLexicon.js @@ -0,0 +1,3 @@ +export function useLexicon() { + return { _: key => key } +} diff --git a/vueManager/vitest.config.js b/vueManager/vitest.config.js index ec1c05dfe..9aa5b23f9 100644 --- a/vueManager/vitest.config.js +++ b/vueManager/vitest.config.js @@ -8,6 +8,9 @@ export default defineConfig({ resolve: { alias: { '@': fileURLToPath(new URL('./src', import.meta.url)), + '@vuetools/useLexicon': fileURLToPath( + new URL('./src/test/stubs/useLexicon.js', import.meta.url) + ), }, }, test: {