From 6a5f6a7765d99f653261d3e049e54955f990ef44 Mon Sep 17 00:00:00 2001 From: James Wampler Date: Tue, 30 Jun 2026 15:57:13 -0700 Subject: [PATCH] Fix undefined tags crash in features table, redesign tag assignment UX Guard against missing tags array in FeaturesView so a malformed row doesn't blow up the whole table render. Replace the separate create-tag-then-click-to-assign flow in TagsChipInput with a single type-and-add input: typing >=2 chars suggests existing unassigned tags, picking one assigns it instead of creating a duplicate, and the color picker is now a dialog of 15 preset swatches plus a custom picker trigger. --- .../src/components/features/TagsChipInput.vue | 275 +++++++++--------- src/admin/src/views/FeaturesView.vue | 6 +- .../components/features/TagsChipInput.spec.ts | 181 ++++++++---- .../tests/unit/views/FeaturesView.spec.ts | 47 ++- 4 files changed, 317 insertions(+), 192 deletions(-) diff --git a/src/admin/src/components/features/TagsChipInput.vue b/src/admin/src/components/features/TagsChipInput.vue index f2494ef..5eb5438 100755 --- a/src/admin/src/components/features/TagsChipInput.vue +++ b/src/admin/src/components/features/TagsChipInput.vue @@ -1,19 +1,6 @@ diff --git a/src/admin/src/views/FeaturesView.vue b/src/admin/src/views/FeaturesView.vue index 543a2d7..cd3eb0b 100755 --- a/src/admin/src/views/FeaturesView.vue +++ b/src/admin/src/views/FeaturesView.vue @@ -73,7 +73,7 @@ …

diff --git a/src/admin/tests/unit/components/features/TagsChipInput.spec.ts b/src/admin/tests/unit/components/features/TagsChipInput.spec.ts index 40c48e1..f29a75d 100755 --- a/src/admin/tests/unit/components/features/TagsChipInput.spec.ts +++ b/src/admin/tests/unit/components/features/TagsChipInput.spec.ts @@ -1,12 +1,13 @@ import { describe, it, expect, beforeEach, jest } from '@jest/globals'; -import { mount, flushPromises, type VueWrapper } from '@vue/test-utils'; +import { mount, flushPromises, DOMWrapper, type VueWrapper } from '@vue/test-utils'; import { setActivePinia, createPinia } from 'pinia'; import { createRouter, createMemoryHistory } from 'vue-router'; import TagsChipInput from '@/components/features/TagsChipInput.vue'; import { useContextStore } from '@/stores/context'; -import type { ProjectResponse, TagResponse } from '@/types/api'; +import type { ProjectResponse, TagResponse, FeatureResponse } from '@/types/api'; jest.mock('@/api/tags'); +jest.mock('@/api/features'); jest.mock('@/api/client', () => ({ setAuthTokens: jest.fn(), clearAuthTokens: jest.fn(), @@ -14,18 +15,25 @@ jest.mock('@/api/client', () => ({ })); import * as tagsApi from '@/api/tags'; +import * as featuresApi from '@/api/features'; const mockProject: ProjectResponse = { id: 10, name: 'Website', organizationId: 1, hideDisabledFlags: false, createdAt: '2026-01-01T00:00:00Z', }; -const mockTags: TagResponse[] = [ - { id: 1, label: 'backend', color: '#ff0000', projectId: 10 }, - { id: 2, label: 'frontend', color: '#0000ff', projectId: 10 }, -]; +const backendTag: TagResponse = { id: 1, label: 'backend', color: '#ff0000', projectId: 10 }; +const frontendTag: TagResponse = { id: 2, label: 'frontend', color: '#0000ff', projectId: 10 }; +const mockTags: TagResponse[] = [backendTag, frontendTag]; -function mountComponent() { +function mockFeature(tags: TagResponse[]): FeatureResponse { + return { + id: 1, name: 'dark_mode', type: 'STANDARD', initialValue: null, description: null, + defaultEnabled: false, projectId: 10, createdAt: '2026-01-01T00:00:00Z', tags, + }; +} + +function mountComponent(feature: FeatureResponse) { const router = createRouter({ history: createMemoryHistory(), routes: [{ path: '/', component: { template: '

' } }] }); - return mount(TagsChipInput, { global: { plugins: [router] } }); + return mount(TagsChipInput, { props: { feature }, global: { plugins: [router] } }); } describe('TagsChipInput', () => { @@ -40,100 +48,173 @@ describe('TagsChipInput', () => { describe('WhenComponentMounts', () => { it('ThenTagsAreFetchedFromApi', async () => { jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); - mountComponent(); + mountComponent(mockFeature([])); await flushPromises(); expect(tagsApi.listTags).toHaveBeenCalledWith(mockProject.id); }); - it('ThenExistingTagChipsAreRendered', async () => { + it('ThenAssignedTagChipsAreRendered', async () => { jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); - const wrapper = mountComponent(); + const wrapper = mountComponent(mockFeature([backendTag])); await flushPromises(); expect(wrapper.find('[data-testid="tag-chip-backend"]').exists()).toBe(true); - expect(wrapper.find('[data-testid="tag-chip-frontend"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="tag-chip-frontend"]').exists()).toBe(false); }); - it('ThenEmptyMessageIsShownWhenNoTagsExist', async () => { - jest.mocked(tagsApi.listTags).mockResolvedValue([]); - const wrapper = mountComponent(); + it('ThenEmptyMessageIsShownWhenNoTagsAssigned', async () => { + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + const wrapper = mountComponent(mockFeature([])); await flushPromises(); expect(wrapper.find('[data-testid="tags-empty-message"]').exists()).toBe(true); }); }); - describe('WhenCreateTagIsSubmitted', () => { - it('ThenCreateTagApiIsCalled', async () => { + describe('WhenAddingANewTagName', () => { + it('ThenCreateTagAndAssignAreBothCalled', async () => { const newTag: TagResponse = { id: 3, label: 'api', color: '#00ff00', projectId: 10 }; + const updatedFeature = mockFeature([newTag]); jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); jest.mocked(tagsApi.createTag).mockResolvedValue(newTag); + jest.mocked(featuresApi.assignFeatureTag).mockResolvedValue(updatedFeature); - const wrapper = mountComponent(); + const wrapper = mountComponent(mockFeature([])); await flushPromises(); - // Open the create form - await wrapper.find('[data-testid="create-tag-btn"]').trigger('click'); - await wrapper.vm.$nextTick(); - - // Fill in the label - await (wrapper.findComponent('[data-testid="new-tag-label-input"]') as VueWrapper).vm.$emit('update:modelValue', 'api'); - - // Submit - await wrapper.find('[data-testid="confirm-create-tag-btn"]').trigger('click'); + const input = wrapper.findComponent('[data-testid="tag-name-input"]') as VueWrapper; + await input.vm.$emit('update:modelValue', 'api'); + await wrapper.find('[data-testid="add-tag-btn"]').trigger('click'); await flushPromises(); - expect(tagsApi.createTag).toHaveBeenCalledWith( - mockProject.id, - expect.objectContaining({ label: 'api' }), - ); + expect(tagsApi.createTag).toHaveBeenCalledWith(mockProject.id, expect.objectContaining({ label: 'api' })); + expect(featuresApi.assignFeatureTag).toHaveBeenCalledWith(mockProject.id, 1, newTag.id); }); - it('ThenNewTagAppearsInTheList', async () => { + it('ThenUpdatedEventIsEmitted', async () => { const newTag: TagResponse = { id: 3, label: 'api', color: '#00ff00', projectId: 10 }; - jest.mocked(tagsApi.listTags).mockResolvedValue([]); + const updatedFeature = mockFeature([newTag]); + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); jest.mocked(tagsApi.createTag).mockResolvedValue(newTag); + jest.mocked(featuresApi.assignFeatureTag).mockResolvedValue(updatedFeature); - const wrapper = mountComponent(); + const wrapper = mountComponent(mockFeature([])); await flushPromises(); - await wrapper.find('[data-testid="create-tag-btn"]').trigger('click'); - await wrapper.vm.$nextTick(); - await (wrapper.findComponent('[data-testid="new-tag-label-input"]') as VueWrapper).vm.$emit('update:modelValue', 'api'); - await wrapper.find('[data-testid="confirm-create-tag-btn"]').trigger('click'); + const input = wrapper.findComponent('[data-testid="tag-name-input"]') as VueWrapper; + await input.vm.$emit('update:modelValue', 'api'); + await wrapper.find('[data-testid="add-tag-btn"]').trigger('click'); await flushPromises(); - expect(wrapper.find('[data-testid="tag-chip-api"]').exists()).toBe(true); + expect(wrapper.emitted('updated')?.[0]).toEqual([updatedFeature]); }); }); - describe('WhenTagChipIsDeleted', () => { - it('ThenDeleteTagApiIsCalled', async () => { + describe('WhenTypedNameMatchesAnExistingTag', () => { + it('ThenExistingTagIsAssignedInsteadOfCreatingANewOne', async () => { + const updatedFeature = mockFeature([backendTag]); jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); - jest.mocked(tagsApi.deleteTag).mockResolvedValue(undefined); + jest.mocked(featuresApi.assignFeatureTag).mockResolvedValue(updatedFeature); - const wrapper = mountComponent(); + const wrapper = mountComponent(mockFeature([])); await flushPromises(); - const chip = wrapper.findComponent('[data-testid="tag-chip-backend"]') as VueWrapper; - await chip.vm.$emit('click:close'); + const input = wrapper.findComponent('[data-testid="tag-name-input"]') as VueWrapper; + await input.vm.$emit('update:modelValue', 'backend'); + await wrapper.find('[data-testid="add-tag-btn"]').trigger('click'); await flushPromises(); - expect(tagsApi.deleteTag).toHaveBeenCalledWith(mockProject.id, mockTags[0].id); + expect(tagsApi.createTag).not.toHaveBeenCalled(); + expect(featuresApi.assignFeatureTag).toHaveBeenCalledWith(mockProject.id, 1, backendTag.id); + }); + }); + + describe('WhenTypingTwoOrMoreCharacters', () => { + it('ThenMatchingUnassignedTagsAreSuggested', async () => { + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + + const wrapper = mountComponent(mockFeature([])); + await flushPromises(); + + const input = wrapper.findComponent('[data-testid="tag-name-input"]') as VueWrapper; + await input.vm.$emit('update:modelValue', 'ba'); + await wrapper.vm.$nextTick(); + + expect(input.props('items')).toEqual(['backend']); }); - it('ThenDeletedTagIsRemovedFromTheList', async () => { + it('ThenNoSuggestionsAreShownBelowTwoCharacters', async () => { jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); - jest.mocked(tagsApi.deleteTag).mockResolvedValue(undefined); - const wrapper = mountComponent(); + const wrapper = mountComponent(mockFeature([])); + await flushPromises(); + + const input = wrapper.findComponent('[data-testid="tag-name-input"]') as VueWrapper; + await input.vm.$emit('update:modelValue', 'b'); + await wrapper.vm.$nextTick(); + + expect(input.props('items')).toEqual([]); + }); + }); + + describe('WhenChoosingATagColour', () => { + it('ThenSwatchClickOpensTheColourDialog', async () => { + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + const wrapper = mountComponent(mockFeature([])); + await flushPromises(); + + expect(wrapper.find('[data-testid="color-dialog"]').exists()).toBe(false); + await wrapper.find('[data-testid="tag-color-swatch"]').trigger('click'); + await flushPromises(); + + expect(wrapper.findComponent({ name: 'VDialog' }).props('modelValue')).toBe(true); + }); + + it('ThenFifteenPresetColoursAreOffered', async () => { + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + const wrapper = mountComponent(mockFeature([])); + await flushPromises(); + + await wrapper.find('[data-testid="tag-color-swatch"]').trigger('click'); + await flushPromises(); + + const presets = new DOMWrapper(document.body).findAll('[data-testid^="preset-color-"]'); + const distinctColors = new Set(presets.map((p) => p.attributes('data-testid'))); + expect(distinctColors.size).toBe(15); + }); + + it('ThenSelectingAPresetUpdatesTheSwatchAndClosesTheDialog', async () => { + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + const wrapper = mountComponent(mockFeature([])); + await flushPromises(); + + await wrapper.find('[data-testid="tag-color-swatch"]').trigger('click'); + await flushPromises(); + + const presets = new DOMWrapper(document.body).findAll('[data-testid="preset-color-#4CAF50"]'); + await presets[presets.length - 1].trigger('click'); + await wrapper.vm.$nextTick(); + await flushPromises(); + + expect(wrapper.find('[data-testid="tag-color-swatch"]').attributes('style')).toContain('background: rgb(76, 175, 80)'); + }); + }); + + describe('WhenTagChipIsClosed', () => { + it('ThenRemoveFeatureTagApiIsCalled', async () => { + const updatedFeature = mockFeature([]); + jest.mocked(tagsApi.listTags).mockResolvedValue(mockTags); + jest.mocked(featuresApi.removeFeatureTag).mockResolvedValue(updatedFeature); + + const wrapper = mountComponent(mockFeature([backendTag])); await flushPromises(); const chip = wrapper.findComponent('[data-testid="tag-chip-backend"]') as VueWrapper; await chip.vm.$emit('click:close'); await flushPromises(); - expect(wrapper.find('[data-testid="tag-chip-backend"]').exists()).toBe(false); + expect(featuresApi.removeFeatureTag).toHaveBeenCalledWith(mockProject.id, 1, backendTag.id); + expect(wrapper.emitted('updated')?.[0]).toEqual([updatedFeature]); }); }); }); diff --git a/src/admin/tests/unit/views/FeaturesView.spec.ts b/src/admin/tests/unit/views/FeaturesView.spec.ts index 576950a..b420ed2 100755 --- a/src/admin/tests/unit/views/FeaturesView.spec.ts +++ b/src/admin/tests/unit/views/FeaturesView.spec.ts @@ -4,7 +4,7 @@ import { setActivePinia, createPinia } from 'pinia'; import { createRouter, createMemoryHistory } from 'vue-router'; import FeaturesView from '@/views/FeaturesView.vue'; import { useContextStore } from '@/stores/context'; -import type { ProjectResponse, EnvironmentResponse, FeatureResponse, FeatureStateResponse } from '@/types/api'; +import type { ProjectResponse, EnvironmentResponse, FeatureResponse, FeatureStateResponse, TagResponse } from '@/types/api'; jest.mock('@/api/features'); jest.mock('@/api/featureStates'); @@ -25,9 +25,13 @@ const mockEnv: EnvironmentResponse = { id: 100, name: 'Development', apiKey: 'env-dev', projectId: 10, createdAt: '2026-01-01T00:00:00Z', }; const mockFeatures: FeatureResponse[] = [ - { id: 1, name: 'dark_mode', type: 'STANDARD', initialValue: null, description: 'Dark mode toggle', defaultEnabled: false, projectId: 10, createdAt: '2026-01-01T00:00:00Z' }, - { id: 2, name: 'new_checkout', type: 'STANDARD', initialValue: null, description: null, defaultEnabled: true, projectId: 10, createdAt: '2026-01-01T00:00:00Z' }, + { id: 1, name: 'dark_mode', type: 'STANDARD', initialValue: null, description: 'Dark mode toggle', defaultEnabled: false, projectId: 10, createdAt: '2026-01-01T00:00:00Z', tags: [] }, + { id: 2, name: 'new_checkout', type: 'STANDARD', initialValue: null, description: null, defaultEnabled: true, projectId: 10, createdAt: '2026-01-01T00:00:00Z', tags: [] }, ]; + +function makeTag(id: number, label: string): TagResponse { + return { id, label, color: '#1565C0', projectId: 10, createdAt: '2026-01-01T00:00:00Z' }; +} const mockStates: FeatureStateResponse[] = [ { id: 101, featureId: 1, environmentId: 100, identityId: null, featureSegmentId: null, enabled: false, value: null, createdAt: '2026-01-01T00:00:00Z', updatedAt: '2026-01-01T00:00:00Z' }, { id: 102, featureId: 2, environmentId: 100, identityId: null, featureSegmentId: null, enabled: true, value: null, createdAt: '2026-01-01T00:00:00Z', updatedAt: '2026-01-01T00:00:00Z' }, @@ -85,6 +89,43 @@ describe('FeaturesView', () => { }); }); + describe('WhenFeatureHasTags', () => { + it('ThenUpToFiveTagChipsAreShownAndExtraAreEllipsised', async () => { + const sixTags = [1, 2, 3, 4, 5, 6].map((id) => makeTag(id, `tag-${id}`)); + const taggedFeature: FeatureResponse = { ...mockFeatures[0], tags: sixTags }; + jest.mocked(featuresApi.listFeatures).mockResolvedValue({ count: 1, next: null, previous: null, results: [taggedFeature] }); + jest.mocked(featureStatesApi.listFeatureStates).mockResolvedValue([]); + + const contextStore = useContextStore(); + contextStore.setOrganization({ id: 1, name: 'Acme', createdAt: '', isPrimary: false }); + contextStore.setProject(mockProject); + + const wrapper = mountView(); + await flushPromises(); + + const chips = sixTags.slice(0, 5).map((tag) => wrapper.find(`[data-testid="feature-tag-chip-${taggedFeature.id}-${tag.id}"]`)); + chips.forEach((chip) => expect(chip.exists()).toBe(true)); + expect(wrapper.find(`[data-testid="feature-tag-chip-${taggedFeature.id}-6"]`).exists()).toBe(false); + expect(wrapper.text()).toContain('…'); + }); + + it('ThenMissingTagsDoesNotThrow', async () => { + const untaggedFeature = { ...mockFeatures[0], tags: undefined } as unknown as FeatureResponse; + jest.mocked(featuresApi.listFeatures).mockResolvedValue({ count: 1, next: null, previous: null, results: [untaggedFeature] }); + jest.mocked(featureStatesApi.listFeatureStates).mockResolvedValue([]); + + const contextStore = useContextStore(); + contextStore.setOrganization({ id: 1, name: 'Acme', createdAt: '', isPrimary: false }); + contextStore.setProject(mockProject); + + const wrapper = mountView(); + await flushPromises(); + + expect(wrapper.find('[data-testid="features-table"]').exists()).toBe(true); + expect(wrapper.text()).toContain(untaggedFeature.name); + }); + }); + describe('WhenToggleEnabledIsClicked', () => { it('ThenPatchFeatureStateIsCalled', async () => { jest.mocked(featuresApi.listFeatures).mockResolvedValue({ count: 2, next: null, previous: null, results: mockFeatures });