From eaa833e096eb1ca2262c02c845c381afc37c4211 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 27 Mar 2026 16:26:00 -0400 Subject: [PATCH 1/6] feat: rename tags --- src/taxonomy/data/api.ts | 1 + src/taxonomy/data/apiHooks.ts | 24 ++ src/taxonomy/data/types.ts | 1 + src/taxonomy/tag-list/TagListTable.test.jsx | 419 ++++++++++++++++---- src/taxonomy/tag-list/TagListTable.tsx | 12 +- src/taxonomy/tag-list/hooks.test.tsx | 9 +- src/taxonomy/tag-list/hooks.ts | 35 +- src/taxonomy/tag-list/messages.ts | 8 + src/taxonomy/tag-list/tagColumns.tsx | 65 ++- src/taxonomy/tree-table/CreateRow.tsx | 147 +++++-- src/taxonomy/tree-table/NestedRows.test.tsx | 18 +- src/taxonomy/tree-table/NestedRows.tsx | 82 +++- src/taxonomy/tree-table/TableBody.tsx | 48 ++- src/taxonomy/tree-table/TableView.test.tsx | 4 + src/taxonomy/tree-table/TableView.tsx | 15 +- 15 files changed, 721 insertions(+), 167 deletions(-) diff --git a/src/taxonomy/data/api.ts b/src/taxonomy/data/api.ts index 063971ee08..f9ade6ccbe 100644 --- a/src/taxonomy/data/api.ts +++ b/src/taxonomy/data/api.ts @@ -90,6 +90,7 @@ export const apiUrls = { /** URL to plan (preview what would happen) a taxonomy import */ tagsPlanImport: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/import/plan/`), createTag: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/`), + updateTag: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/`), } satisfies Record string>; /** diff --git a/src/taxonomy/data/apiHooks.ts b/src/taxonomy/data/apiHooks.ts index 753503cf3f..dd0b5cf2de 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -234,3 +234,27 @@ export const useCreateTag = (taxonomyId: number) => { }, }); }; + +export const useUpdateTag = (taxonomyId: number) => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: async ({ value, originalValue }: { value: string, originalValue: string }) => { + try { + await getAuthenticatedHttpClient().patch( + apiUrls.updateTag(taxonomyId), + { tag: originalValue, updated_tag_value: value }, + ); + } catch (err) { + throw new Error(getApiErrorMessage(err)); + } + }, + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: taxonomyQueryKeys.taxonomyTagList(taxonomyId), + }); + // In the metadata, 'tagsCount' (and possibly other fields) will have changed: + queryClient.invalidateQueries({ queryKey: taxonomyQueryKeys.taxonomyMetadata(taxonomyId) }); + }, + }); +}; diff --git a/src/taxonomy/data/types.ts b/src/taxonomy/data/types.ts index d8ca63d1e7..4fd3c41167 100644 --- a/src/taxonomy/data/types.ts +++ b/src/taxonomy/data/types.ts @@ -59,6 +59,7 @@ export interface TagListData { next: string; numPages: number; previous: string; + canAddTag?: boolean; results: TagData[]; start: number; } diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index a7616a2ab7..53018661e2 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -98,6 +98,15 @@ const mockTagsResponse = { }, ], }; + +const mockTagResponseDisallowingEdits = { + ...mockTagsResponse, + results: mockTagsResponse.results.map(tag => ({ + ...tag, + can_change_tag: false, + can_delete_tag: false, + })), +}; const mockTagsPaginationResponse = { next: null, previous: null, @@ -161,6 +170,15 @@ const openTopLevelDraftRow = async () => { }; const openActionsMenuForTag = (tagName, actionButtonName = /actions/i) => { + if (!screen.queryAllByText(tagName)?.length) { + // expand all + const expandButton = screen.queryAllByText('Expand All')?.[0]; + act(() => { + if (expandButton) { + fireEvent.click(expandButton); + } + }); + } const row = screen.getByText(tagName).closest('tr'); const actionsButton = within(row).getByRole('button', { name: actionButtonName }); act(() => { @@ -183,6 +201,22 @@ const openSubtagDraftRow = async ({ return { rows, draftRow, input }; }; +const openRenameDraftRow = async (tagName = 'root tag 1') => { + openActionsMenuForTag(tagName); + fireEvent.click(screen.getByText('Rename')); + const input = screen.getByRole('textbox'); + const row = input.closest('tr'); + expect(row).toBeInTheDocument(); + const saveButton = within(row).getByText('Save'); + const cancelButton = within(row).getByText('Cancel'); + return { + row, + input, + saveButton, + cancelButton, + }; +}; + describe('', () => { beforeAll(async () => { initializeMockApp({ @@ -191,8 +225,13 @@ describe('', () => { axiosMock = new MockAdapter(getAuthenticatedHttpClient()); }); beforeEach(async () => { + initializeMockApp({ + authenticatedUser: adminUser, + }); store = initializeStore(); + queryClient.clear(); axiosMock.reset(); + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); @@ -255,7 +294,26 @@ describe('', () => { }); describe('Create a new top-level tag', () => { + it('should disable tag creation buttons if the taxonomy includes `can_add_tag: false`', async () => { + axiosMock.onGet(rootTagsListUrl).reply(200, { + ...mockTagsResponse, + can_add_tag: false, + }); + cleanup(); + queryClient.clear(); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag('root tag 1'); + expect(screen.getByText('Add Subtag')).toBeDisabled(); + expect(screen.getByLabelText('Create Tag')).toBeDisabled(); + }); + describe('with editable user and loaded taxonomy', () => { + beforeEach(() => { + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); + }); + it('should add draft row when top-level "Add tag" button is clicked', async () => { const { creatingRow } = await openTopLevelDraftRow(); @@ -429,6 +487,8 @@ describe('', () => { // a bit flaky when ran together with other tests - any way to improve this? it('should allow adding multiple tags consecutively without a page refresh', async () => { + // clear axios mock history + axiosMock.reset(); axiosMock.onPost(createTagUrl).reply(config => { const requestData = JSON.parse(config.data); return [201, { @@ -470,7 +530,7 @@ describe('', () => { expect(tagBRowIndex).toBeLessThan(tagARowIndex); // no additional get requests should have been made, that is, the table should not have been refreshed - expect(axiosMock.history.get.length).toBe(1); + expect(axiosMock.history.get.length).toBe(0); }); it('should disable the Save button when the input is empty', async () => { @@ -596,108 +656,106 @@ describe('', () => { }); describe('Create a new subtag', () => { - describe('with editable user and loaded taxonomy', () => { - it('should show an Add sub-tag option in the parent tag actions', async () => { - expect(screen.queryAllByText('Add Subtag').length).toBe(0); - // user clicks on row actions for root tag 1 - openActionsMenuForTag('root tag 1'); - expect(screen.getByText('Add Subtag')).toBeInTheDocument(); - }); + it('should show an Add sub-tag option in the parent tag actions', async () => { + expect(screen.queryAllByText('Add Subtag').length).toBe(0); + // user clicks on row actions for root tag 1 + openActionsMenuForTag('root tag 1'); + expect(screen.getByText('Add Subtag')).toBeInTheDocument(); + }); - it('should render an inline add-subtag row with input, placeholder, and action buttons', async () => { - const { rows } = await openSubtagDraftRow({ tagName: 'root tag 1' }); - const draftRows = rows.filter(tableRow => tableRow.querySelector('input')); - expect(draftRows[0].querySelector('input')).toBeInTheDocument(); - // expect the draft row to be directly beneath the parent tag row - const parentRowIndex = rows.findIndex(tableRow => within(tableRow).queryByText('root tag 1')); - const draftRowIndex = rows.findIndex(tableRow => tableRow.querySelector('input')); - expect(draftRowIndex).toBe(parentRowIndex + 1); - expect(draftRows[0].querySelector('input')).toBeInTheDocument(); - expect(within(draftRows[0]).getByText('Cancel')).toBeInTheDocument(); - expect(within(draftRows[0]).getByText('Save')).toBeInTheDocument(); - }); + it('should render an inline add-subtag row with input, placeholder, and action buttons', async () => { + const { rows } = await openSubtagDraftRow({ tagName: 'root tag 1' }); + const draftRows = rows.filter(tableRow => tableRow.querySelector('input')); + expect(draftRows[0].querySelector('input')).toBeInTheDocument(); + // expect the draft row to be directly beneath the parent tag row + const parentRowIndex = rows.findIndex(tableRow => within(tableRow).queryByText('root tag 1')); + const draftRowIndex = rows.findIndex(tableRow => tableRow.querySelector('input')); + expect(draftRowIndex).toBe(parentRowIndex + 1); + expect(draftRows[0].querySelector('input')).toBeInTheDocument(); + expect(within(draftRows[0]).getByText('Cancel')).toBeInTheDocument(); + expect(within(draftRows[0]).getByText('Save')).toBeInTheDocument(); + }); - it('should remove add-subtag row and avoid create request when cancelled', async () => { - const { draftRow, input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); - fireEvent.change(input, { target: { value: 'new subtag' } }); - fireEvent.click(within(draftRow).getByText('Cancel')); + it('should remove add-subtag row and avoid create request when cancelled', async () => { + const { draftRow, input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); + fireEvent.change(input, { target: { value: 'new subtag' } }); + fireEvent.click(within(draftRow).getByText('Cancel')); - await waitFor(() => { - expect(axiosMock.history.post.length).toBe(0); - expectNoDraftRows(); - }); + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(0); + expectNoDraftRows(); }); + }); - it('should remove add-subtag row and avoid create request on escape key', async () => { - const { input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); + it('should remove add-subtag row and avoid create request on escape key', async () => { + const { input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); - fireEvent.change(input, { target: { value: 'new subtag' } }); - fireEvent.keyDown(input, { key: 'Escape', code: 'Escape' }); + fireEvent.change(input, { target: { value: 'new subtag' } }); + fireEvent.keyDown(input, { key: 'Escape', code: 'Escape' }); - await waitFor(() => { - expect(axiosMock.history.post.length).toBe(0); - expectNoDraftRows(); - }); + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(0); + expectNoDraftRows(); }); + }); - it('should disable Save and show required-name inline error for empty sub-tag input', async () => { - openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); - const rows = await screen.findAllByRole('row'); - const draftRow = rows.find(tableRow => tableRow.querySelector('input')); - const saveButton = within(draftRow).getByText('Save'); - const input = draftRow.querySelector('input'); - act(() => { - fireEvent.change(input, { target: { value: ' ' } }); - }); - - expect(saveButton).toBeDisabled(); - expect(within(draftRow).getByText(/Name is required/i)).toBeInTheDocument(); + it('should disable Save and show required-name inline error for empty sub-tag input', async () => { + openActionsMenuForTag('root tag 1'); + fireEvent.click(screen.getByText('Add Subtag')); + const rows = await screen.findAllByRole('row'); + const draftRow = rows.find(tableRow => tableRow.querySelector('input')); + const saveButton = within(draftRow).getByText('Save'); + const input = draftRow.querySelector('input'); + act(() => { + fireEvent.change(input, { target: { value: ' ' } }); }); - it('should keep Save disabled for whitespace-only sub-tag input', async () => { - openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); - const rows = await screen.findAllByRole('row'); - const draftRow = rows.find(tableRow => tableRow.querySelector('input')); - const input = draftRow.querySelector('input'); - const saveButton = within(draftRow).getByText('Save'); + expect(saveButton).toBeDisabled(); + expect(within(draftRow).getByText(/Name is required/i)).toBeInTheDocument(); + }); - fireEvent.change(input, { target: { value: ' ' } }); - expect(saveButton).toBeDisabled(); - }); + it('should keep Save disabled for whitespace-only sub-tag input', async () => { + openActionsMenuForTag('root tag 1'); + fireEvent.click(screen.getByText('Add Subtag')); + const rows = await screen.findAllByRole('row'); + const draftRow = rows.find(tableRow => tableRow.querySelector('input')); + const input = draftRow.querySelector('input'); + const saveButton = within(draftRow).getByText('Save'); + + fireEvent.change(input, { target: { value: ' ' } }); + expect(saveButton).toBeDisabled(); + }); - it('should disable Save and show invalid-character error for sub-tag input', async () => { - openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); - const rows = await screen.findAllByRole('row'); - const draftRow = rows.find(row => row.querySelector('input')); - const input = draftRow.querySelector('input'); - const saveButton = within(draftRow).getByText('Save'); + it('should disable Save and show invalid-character error for sub-tag input', async () => { + openActionsMenuForTag('root tag 1'); + fireEvent.click(screen.getByText('Add Subtag')); + const rows = await screen.findAllByRole('row'); + const draftRow = rows.find(row => row.querySelector('input')); + const input = draftRow.querySelector('input'); + const saveButton = within(draftRow).getByText('Save'); - fireEvent.change(input, { target: { value: 'invalid;name' } }); - expect(saveButton).toBeDisabled(); - expect(within(draftRow).getByText(/invalid character/i)).toBeInTheDocument(); - }); + fireEvent.change(input, { target: { value: 'invalid;name' } }); + expect(saveButton).toBeDisabled(); + expect(within(draftRow).getByText(/invalid character/i)).toBeInTheDocument(); + }); - it('should keep inline row and show failure feedback when sub-tag save fails', async () => { - axiosMock.onPost(createTagUrl).reply(500, { - error: 'Internal server error', - }); + it('should keep inline row and show failure feedback when sub-tag save fails', async () => { + axiosMock.onPost(createTagUrl).reply(500, { + error: 'Internal server error', + }); - openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); - const rows = await screen.findAllByRole('row'); - const draftRow = rows.find(row => row.querySelector('input')); - const input = draftRow.querySelector('input'); - fireEvent.change(input, { target: { value: 'subtag fail' } }); - fireEvent.click(within(draftRow).getByText('Save')); + openActionsMenuForTag('root tag 1'); + fireEvent.click(screen.getByText('Add Subtag')); + const rows = await screen.findAllByRole('row'); + const draftRow = rows.find(row => row.querySelector('input')); + const input = draftRow.querySelector('input'); + fireEvent.change(input, { target: { value: 'subtag fail' } }); + fireEvent.click(within(draftRow).getByText('Save')); - await waitFor(() => { - expect(getDraftRows().length).toBe(1); - }); - expect(await screen.findByText(/Error creating tag:/i)).toBeInTheDocument(); + await waitFor(() => { + expect(getDraftRows().length).toBe(1); }); + expect(await screen.findByText(/Error creating tag:/i)).toBeInTheDocument(); }); it('should hide or disable Add sub-tag actions when user lacks edit permissions', async () => { @@ -713,6 +771,192 @@ describe('', () => { }); }); + describe('Tag Rename Errors', () => { + it('should keep the inline row and show a failure toast when save request fails', async () => { + axiosMock.onPatch().reply(500, { + error: 'Internal server error', + }); + const { input } = await openRenameDraftRow('root tag 1'); + + fireEvent.change(input, { target: { value: 'will fail' } }); + act(() => { + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); + }); + + let draftRow; + await waitFor(() => { + const rows = screen.getAllByRole('row'); + const draftRows = rows.filter(row => row.querySelector('input')); + expect(draftRows.length).toBe(1); + draftRow = draftRows[0]; // eslint-disable-line prefer-destructuring + }); + + // Banner error message should be shown at the top of the table + expect(await screen.findByText('Error saving changes')).toBeInTheDocument(); + + // Toast message to indicate that the save failed + expect(await screen.findByText('Error saving changes')).toBeInTheDocument(); + expect(await screen.findByText('Internal server error')).toBeInTheDocument(); + + // expect the input to retain the value that was entered before + expect(draftRow.querySelector('input').value).toEqual('will fail'); + // expect the new tag to not be in the document outside the input field + expect(screen.queryByText('will fail')).not.toBeInTheDocument(); + }); + }); + + const tagDepthScenarios = [ + { + description: 'Rename a top-level tag', + tagName: 'root tag 1', + }, + { description: 'Rename a sub-tag', tagName: 'the child tag' }, + { description: 'Rename a grandchild tag', tagName: 'the grandchild tag' }, + ]; + + tagDepthScenarios.forEach(({ description, tagName }) => { + describe(description, () => { + beforeEach(async () => { + axiosMock.resetHistory(); + }); + it('should disable tag edit buttons if the taxonomy includes `can_add_tag: false`', async () => { + axiosMock.onGet(rootTagsListUrl).reply(200, { + ...mockTagsResponse, + can_add_tag: false, + }); + cleanup(); + queryClient.clear(); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag(tagName); + expect(screen.getByText('Rename')).toBeInTheDocument(); + expect(screen.getByText('Rename')).toBeDisabled(); + }); + it('should disable tag edit buttons if tag includes `can_edit: false`', async () => { + axiosMock.reset(); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + cleanup(); + queryClient.clear(); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag(tagName); + expect(screen.getByText('Rename')).toBeInTheDocument(); + expect(screen.getByText('Rename')).toBeDisabled(); + }); + + it('should show tag actions menu', async () => { + openActionsMenuForTag(tagName); + expect(screen.getByText('Add Subtag')).toBeInTheDocument(); + expect(screen.getByText('Rename')).toBeInTheDocument(); + }); + it('should show editable input and action buttons when Rename is selected from actions menu', async () => { + const { row } = await openRenameDraftRow(tagName); + expect(within(row).getByRole('textbox')).toBeInTheDocument(); + // expect the input to be pre-filled with the current tag name + expect(within(row).getByRole('textbox').value).toEqual(tagName); + expect(within(row).getByText('Save')).toBeInTheDocument(); + expect(within(row).getByText('Cancel')).toBeInTheDocument(); + }); + it('should disable Save button until the tag name is changed', async () => { + const { input, saveButton } = await openRenameDraftRow(tagName); + + expect(saveButton).toBeDisabled(); + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + expect(saveButton).not.toBeDisabled(); + }); + it('should save changes and show success toast when Enter is pressed', async () => { + axiosMock.onPatch(/.*/).reply(200, {}); + const { input } = await openRenameDraftRow(tagName); + + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + act(() => { + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); + }); + + await waitFor(() => { + expect(axiosMock.history.patch.length).toBe(1); + expect(axiosMock.history.patch[0].data).toEqual(JSON.stringify({ + tag: tagName, + updated_tag_value: `${tagName} updated`, + })); + }); + expect(await screen.findByText(`Tag "${tagName} updated" updated successfully`)).toBeInTheDocument(); + }); + it('should save changes and show success toast when Save is clicked', async () => { + axiosMock.onPatch(/.*/).reply(200, {}); + const { input, saveButton } = await openRenameDraftRow(tagName); + + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + fireEvent.click(saveButton); + + await waitFor(() => { + expect(axiosMock.history.patch.length).toBe(1); + expect(axiosMock.history.patch[0].data).toEqual(JSON.stringify({ + tag: tagName, + updated_tag_value: `${tagName} updated`, + })); + }); + expect(await screen.findByText(`Tag "${tagName} updated" updated successfully`)).toBeInTheDocument(); + }); + it('should cancel editing and revert to original name when Esc is pressed', async () => { + const { input } = await openRenameDraftRow(tagName); + + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + fireEvent.keyDown(input, { key: 'Escape', code: 'Escape' }); + + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + expect(screen.getByText(tagName)).toBeInTheDocument(); + expect(axiosMock.history.patch.length).toBe(0); + }); + it('should cancel editing and revert to original name when Cancel is clicked', async () => { + const { input, cancelButton } = await openRenameDraftRow(tagName); + + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + fireEvent.click(cancelButton); + + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + expect(screen.getByText(tagName)).toBeInTheDocument(); + expect(axiosMock.history.patch.length).toBe(0); + }); + }); + }); + + describe('Nested behavior', () => { + beforeEach(async () => { + axiosMock.resetHistory(); + }); + + it('should keep the parent-child relationships in the updated tree data when renaming a parent tag', async () => { + // this only tests that the frontend is updated correctly before reloading data; + // the rest is covered by the backend tests for the rename endpoint + + axiosMock.onPatch(/.*/).reply(200, {}); + const { input, saveButton } = await openRenameDraftRow('root tag 1'); + + fireEvent.change(input, { target: { value: 'root tag 1 updated' } }); + fireEvent.click(saveButton); + + await waitFor(() => { + expect(axiosMock.history.patch.length).toBe(1); + expect(axiosMock.history.patch[0].data).toEqual(JSON.stringify({ + tag: 'root tag 1', + updated_tag_value: 'root tag 1 updated', + })); + }); + // make sure rows are not already expanded by checking that the child tag is not visible before expanding + expect(screen.queryAllByText('the child tag')?.length).toBeFalsy(); + fireEvent.click(await screen.findByLabelText('Show Subtags')); + // expect the child tag to still be present under the renamed parent tag + expect(await screen.findByText('the child tag')).toBeInTheDocument(); + // expect the grandchild tag to still be present under the child tag + openActionsMenuForTag('the child tag'); + fireEvent.click(await screen.findByLabelText('Show Subtags')); + expect(await screen.findByText('the grandchild tag')).toBeInTheDocument(); + }); + }); + describe('At smaller max depth', () => { beforeEach(async () => { const maxDepth = 2; @@ -743,7 +987,10 @@ describe('', () => { // depth 2 is innermost when maxDepth=2, so no add-subtag action should be shown const grandchildTagRow = screen.getByText('the grandchild tag').closest('tr'); - expect(within(grandchildTagRow).queryByRole('button', { name: /actions/i })).not.toBeInTheDocument(); + expect(within(grandchildTagRow).queryByRole('button', { name: /actions/i })).toBeInTheDocument(); + openActionsMenuForTag('the grandchild tag'); + expect(within(grandchildTagRow).getByText('Rename')).toBeInTheDocument(); + expect(within(grandchildTagRow).getByText('Add Subtag')).toBeDisabled(); }); }); }); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 01795b8917..e8b7ebac0b 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -5,7 +5,7 @@ import React, { } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import type { PaginationState } from '@tanstack/react-table'; -import { useTagListData, useCreateTag } from '../data/apiHooks'; +import { useTagListData, useCreateTag, useUpdateTag } from '../data/apiHooks'; import { TagTree } from './tagTree'; import { TableView } from '../tree-table'; import type { @@ -80,6 +80,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { enabled: tableMode === TABLE_MODES.VIEW, }); const createTagMutation = useCreateTag(taxonomyId); + const updateTagMutation = useUpdateTag(taxonomyId); const pageCount = tagList?.numPages ?? -1; // TODO: to make this more readable, introduce a React context for the TagListTable instead of passing props. @@ -90,6 +91,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setTagTree, setDraftError, createTagMutation, + updateTagMutation, enterPreviewMode, setToast, setIsCreatingTopTag, @@ -107,19 +109,19 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { onStartDraft: enterDraftMode, setActiveActionMenuRowId, hasOpenDraft, + canAddTag: tagList?.canAddTag !== false, draftError, setDraftError, isSavingDraft: createTagMutation.isPending, maxDepth, - creatingParentId, }), [ isCreatingTopTag, - editingRowId, tableMode, activeActionMenuRowId, hasOpenDraft, creatingParentId, + tagList?.canAddTag, draftError, createTagMutation.isPending, maxDepth, @@ -157,7 +159,9 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { isCreatingTopRow: isCreatingTopTag, draftError, createRowMutation: createTagMutation, + updateRowMutation: updateTagMutation, handleCreateRow: handleCreateTag, + handleUpdateRow: handleUpdateTag, toast, setToast, setIsCreatingTopRow: setIsCreatingTopTag, @@ -166,6 +170,8 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setCreatingParentId, setDraftError, validate, + editingRowId, + setEditingRowId, }} /> ); diff --git a/src/taxonomy/tag-list/hooks.test.tsx b/src/taxonomy/tag-list/hooks.test.tsx index 7d799afceb..fc3dbdaebb 100644 --- a/src/taxonomy/tag-list/hooks.test.tsx +++ b/src/taxonomy/tag-list/hooks.test.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { act, renderHook } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; import { TagTree } from './tagTree'; import { useEditActions, useTableModes } from './hooks'; @@ -44,6 +44,8 @@ describe('useTableModes', () => { describe('useEditActions', () => { const buildActions = (overrides = {}) => { const createTagMutation = { mutateAsync: jest.fn() }; + // mock updateTagMutation to have a function `mutateAsync` that returns a resolved promise + const updateTagMutation = { mutateAsync: jest.fn() }; const setTagTree = jest.fn(); const setDraftError = jest.fn(); const enterPreviewMode = jest.fn(); @@ -63,6 +65,7 @@ describe('useEditActions', () => { setCreatingParentId, exitDraftWithoutSave, setEditingRowId, + updateTagMutation: updateTagMutation as any, ...(overrides as any), }; @@ -139,7 +142,9 @@ describe('useEditActions', () => { await actions.handleUpdateTag('updated', 'original'); }); - expect(enterPreviewMode).toHaveBeenCalled(); + await waitFor(() => { + expect(enterPreviewMode).toHaveBeenCalled(); + }); expect(setToast).toHaveBeenCalledWith({ show: true, message: 'Tag "updated" updated successfully', diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index a350e3eabe..f92531bae4 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -1,7 +1,7 @@ import { useReducer } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { useCreateTag } from '../data/apiHooks'; +import { useCreateTag, useUpdateTag } from '../data/apiHooks'; import { TagTree } from './tagTree'; import { TagListTableError } from './errors'; import type { RowId } from '../tree-table/types'; @@ -45,6 +45,7 @@ interface UseEditActionsParams { setCreatingParentId: React.Dispatch>; exitDraftWithoutSave: () => void; setEditingRowId: React.Dispatch>; + updateTagMutation: ReturnType; } const getInlineValidationMessage = (value: string, intl: ReturnType): string => { @@ -111,9 +112,20 @@ const useEditActions = ({ setToast, setIsCreatingTopTag, setCreatingParentId, + exitDraftWithoutSave, setEditingRowId, + updateTagMutation, }: UseEditActionsParams) => { const intl = useIntl(); + + const updateTableAfterRename = (oldValue: string, newValue: string) => { + setTagTree((currentTagTree) => { + const nextTree = currentTagTree || new TagTree([]); + nextTree.editTagValue(oldValue, newValue); + return nextTree; + }); + }; + const updateTableWithoutDataReload = (value: string, parentTagValue: string | null = null) => { setTagTree((currentTagTree) => { const nextTree = currentTagTree || new TagTree([]); @@ -179,14 +191,31 @@ const useEditActions = ({ const handleUpdateTag = async (value: string, originalValue: string) => { const trimmed = value.trim(); - if (trimmed && trimmed !== originalValue) { + if (!validate(trimmed, 'soft')) { + return; + } + + if (trimmed === originalValue) { + setEditingRowId(null); + exitDraftWithoutSave(); + return; + } + + try { + setDraftError(''); + await updateTagMutation.mutateAsync({ value: trimmed, originalValue }); + updateTableAfterRename(originalValue, trimmed); enterPreviewMode(); + setEditingRowId(null); setToast({ show: true, message: intl.formatMessage(messages.tagUpdateSuccessMessage, { name: trimmed }), }); + } catch (error) { + const message = intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage: (error as Error)?.message }); + setDraftError((error as Error)?.message || intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage: '' })); + setToast({ show: true, message }); } - setEditingRowId(null); }; return { diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index 2e9b7ecadb..f40f5f4d05 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -53,6 +53,14 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.hide-subtags.button-label', defaultMessage: 'Hide Subtags', }, + tagUpdateErrorMessage: { + id: 'course-authoring.tag-list.update-error', + defaultMessage: 'Error updating tag: {errorMessage}', + }, + renameTag: { + id: 'course-authoring.tag-list.rename-tag', + defaultMessage: 'Rename', + }, }); export default messages; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index c70b415a69..4710d92bc7 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -20,6 +20,8 @@ import type { } from '../tree-table/types'; import OptionalExpandLink from './OptionalExpandLink'; +const EDITABLE_COLUMNS = ['value']; + interface TagListRowData extends TreeRowData { depth: number; childCount: number; @@ -40,11 +42,11 @@ interface GetColumnsArgs { onStartDraft: () => void; setActiveActionMenuRowId: (id: RowId | null) => void; hasOpenDraft: boolean; + canAddTag: boolean; draftError: string; setDraftError: (error: string) => void; isSavingDraft: boolean; maxDepth: number; - creatingParentId: RowId | null; } interface ActionsHeaderProps { @@ -55,6 +57,7 @@ interface ActionsHeaderProps { setActiveActionMenuRowId: (id: RowId | null) => void; hasOpenDraft: boolean; draftInProgressHintId: string; + canAddTag: boolean; } const ActionsHeader = ({ @@ -64,6 +67,7 @@ const ActionsHeader = ({ setEditingRowId, setActiveActionMenuRowId, hasOpenDraft, + canAddTag, draftInProgressHintId, }: ActionsHeaderProps) => { const intl = useIntl(); @@ -82,7 +86,7 @@ const ActionsHeader = ({ setEditingRowId(null); setActiveActionMenuRowId(null); }} - disabled={hasOpenDraft} + disabled={hasOpenDraft || !canAddTag} aria-describedby={hasOpenDraft ? draftInProgressHintId : undefined} /> @@ -93,9 +97,21 @@ interface ActionsMenuProps { rowData: TagListRowData; startSubtagDraft: () => void; disableAddSubtag: boolean; + editTag: () => void; + disableEditTag: boolean; + reachedMaxDepth: (row: Row) => boolean; + row: Row; } -const ActionsMenu = ({ rowData, startSubtagDraft, disableAddSubtag }: ActionsMenuProps) => { +const ActionsMenu = ({ + rowData, + row, + startSubtagDraft, + disableAddSubtag, + editTag, + disableEditTag, + reachedMaxDepth, +}: ActionsMenuProps) => { const intl = useIntl(); return ( @@ -113,14 +129,21 @@ const ActionsMenu = ({ rowData, startSubtagDraft, disableAddSubtag }: ActionsMen {intl.formatMessage(messages.addSubtag)} + + {intl.formatMessage(messages.renameTag)} + ); -} +}; function getColumns({ setIsCreatingTopTag, @@ -129,11 +152,11 @@ function getColumns({ onStartDraft, setActiveActionMenuRowId, hasOpenDraft, + canAddTag, setDraftError, maxDepth, - creatingParentId, }: GetColumnsArgs): TreeColumnDef[] { - const canAddSubtag = (row: Row) => row.depth < maxDepth; + const reachedMaxDepth = (row: Row) => row.depth >= maxDepth; const draftInProgressHintId = 'tag-list-draft-in-progress-hint'; return [ @@ -164,16 +187,19 @@ function getColumns({ setActiveActionMenuRowId={setActiveActionMenuRowId} hasOpenDraft={hasOpenDraft} draftInProgressHintId={draftInProgressHintId} + canAddTag={canAddTag} /> ), cell: ({ row }) => { const rowData = asTagListRowData(row); - if (rowData.isNew || rowData.isEditing || !canAddSubtag(row)) { + if (rowData.isNew || rowData.isEditing) { return
; } - const disableAddSubtag = hasOpenDraft && creatingParentId !== rowData.id; + const disableAddSubtag = hasOpenDraft || !canAddTag; + const disableEditTag = hasOpenDraft || !canAddTag || row.original.canChangeTag === false; + const startSubtagDraft = () => { onStartDraft(); setDraftError(''); @@ -184,9 +210,26 @@ function getColumns({ row.toggleExpanded(true); }; + const editTag = () => { + onStartDraft(); + setDraftError(''); + setEditingRowId(`${rowData.id}:${rowData.value}`); + setCreatingParentId(null); + setIsCreatingTopTag(false); + setActiveActionMenuRowId(null); + }; + return (
- +
); }, @@ -194,4 +237,4 @@ function getColumns({ ]; } -export { getColumns }; +export { getColumns, EDITABLE_COLUMNS }; diff --git a/src/taxonomy/tree-table/CreateRow.tsx b/src/taxonomy/tree-table/CreateRow.tsx index 57ea7dcfd2..4091db0a56 100644 --- a/src/taxonomy/tree-table/CreateRow.tsx +++ b/src/taxonomy/tree-table/CreateRow.tsx @@ -6,6 +6,20 @@ import { EditableCell } from './EditableCell'; import type { CreateRowMutationState, TreeColumnDef } from './types'; import messages from './messages'; +interface DraftRowProps { + draftError: string; + initialValue?: string; + onSave: (value: string) => void; + onCancel: () => void; + mutationState: CreateRowMutationState; + columns: TreeColumnDef[]; + indent?: number; + validate: (value: string, mode?: 'soft' | 'hard') => boolean; + requireValueChangeToEnableSave?: boolean; + rowTestId?: string; + rowId?: string; +} + interface CreateRowProps { draftError: string; setDraftError: (error: string) => void; @@ -18,57 +32,73 @@ interface CreateRowProps { validate: (value: string, mode?: 'soft' | 'hard') => boolean; } -const CreateRow: React.FC = ({ +interface EditRowProps { + draftError: string; + setDraftError: (error: string) => void; + initialValue: string; + handleUpdateRow: (value: string) => void; + cancelEditRow: () => void; + updateRowMutation: CreateRowMutationState; + columns: TreeColumnDef[]; + indent?: number; + validate: (value: string, mode?: 'soft' | 'hard') => boolean; +} + +const DraftRow: React.FC = ({ draftError, - setDraftError, - handleCreateRow, - setIsCreatingTopRow, - exitDraftWithoutSave, - createRowMutation, + initialValue = '', + onSave, + onCancel, + mutationState, columns, indent = 0, validate, + requireValueChangeToEnableSave = false, + rowTestId, + rowId, }) => { - const [newRowValue, setNewRowValue] = useState(''); - const intl = useIntl(); + const [rowValue, setRowValue] = useState(initialValue); const [saveDisabled, setSaveDisabled] = useState(true); + const intl = useIntl(); - const handleValueChange = (e: React.ChangeEvent) => { - const { value } = e.target; - setNewRowValue(value); + const updateSaveDisabled = (value: string) => { + const trimmedValue = value.trim(); const isValid = validate(value, 'soft'); - setSaveDisabled(!isValid || createRowMutation.isPending || false); + const isUnchanged = requireValueChangeToEnableSave && trimmedValue === initialValue.trim(); + setSaveDisabled(!isValid || !trimmedValue || isUnchanged || mutationState.isPending || false); }; - const handleCancel = () => { - setDraftError(''); - setNewRowValue(''); - setIsCreatingTopRow(false); - exitDraftWithoutSave(); + const handleValueChange = (e: React.ChangeEvent) => { + const { value } = e.target; + setRowValue(value); + updateSaveDisabled(value); }; const handleSave = () => { - handleCreateRow(newRowValue.trim()); + onSave(rowValue.trim()); }; const handleValueCellKeyPress = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' && newRowValue.trim() && !createRowMutation.isPending && !draftError) { + if (e.key === 'Enter' && !saveDisabled && !draftError) { e.preventDefault(); handleSave(); - } else if (e.key === 'Escape') { + return; + } + + if (e.key === 'Escape') { e.preventDefault(); - handleCancel(); + onCancel(); } }; return ( - - +
= ({ @@ -94,7 +124,7 @@ const CreateRow: React.FC = ({ {intl.formatMessage(messages.saveButtonLabel)} - {createRowMutation.isPending && ( + {mutationState.isPending && ( = ({ ); }; -export { CreateRow }; +const CreateRow: React.FC = ({ + draftError, + setDraftError, + handleCreateRow, + setIsCreatingTopRow, + exitDraftWithoutSave, + createRowMutation, + columns, + indent = 0, + validate, +}) => { + const handleCancel = () => { + setDraftError(''); + setIsCreatingTopRow(false); + exitDraftWithoutSave(); + }; + + return ( + + ); +}; + +const EditRow: React.FC = ({ + draftError, + setDraftError, + initialValue, + handleUpdateRow, + cancelEditRow, + updateRowMutation, + columns, + indent = 0, + validate, +}) => { + const handleCancel = () => { + setDraftError(''); + cancelEditRow(); + }; + + return ( + + ); +}; + +export { CreateRow, EditRow }; diff --git a/src/taxonomy/tree-table/NestedRows.test.tsx b/src/taxonomy/tree-table/NestedRows.test.tsx index 1eda32fe41..0909ad5a86 100644 --- a/src/taxonomy/tree-table/NestedRows.test.tsx +++ b/src/taxonomy/tree-table/NestedRows.test.tsx @@ -8,6 +8,17 @@ const wrapper = ({ children }: { children: React.ReactNode }) => ( {children} ); +const defaultRequiredProps = { + setIsCreatingTopRow: jest.fn(), + createRowMutation: {}, + updateRowMutation: {}, + handleUpdateRow: jest.fn(), + editingRowId: null, + setEditingRowId: jest.fn(), + exitDraftWithoutSave: jest.fn(), + validate: () => true, +}; + const makeCell = (id: string, content: string) => ({ id, column: { columnDef: { cell: () => content } }, @@ -41,9 +52,7 @@ describe('NestedRows', () => { true} + {...defaultRequiredProps} /> , @@ -74,9 +83,8 @@ describe('NestedRows', () => { creatingParentId={2} setCreatingParentId={setCreatingParentId} onCancelCreation={onCancelCreation} - setIsCreatingTopRow={jest.fn()} + {...defaultRequiredProps} createRowMutation={{ isPending: false }} - validate={() => true} /> , diff --git a/src/taxonomy/tree-table/NestedRows.tsx b/src/taxonomy/tree-table/NestedRows.tsx index 8facb825b7..216bcfbf18 100644 --- a/src/taxonomy/tree-table/NestedRows.tsx +++ b/src/taxonomy/tree-table/NestedRows.tsx @@ -4,9 +4,10 @@ import { flexRender } from '@tanstack/react-table'; import type { RowId, TreeRow, + TreeColumnDef, CreateRowMutationState, } from './types'; -import { CreateRow } from './CreateRow'; +import { CreateRow, EditRow } from './CreateRow'; interface NestedRowsProps { /** The parent row object from TanStack React Table */ @@ -35,6 +36,18 @@ interface NestedRowsProps { setIsCreatingTopRow: (isCreating: boolean) => void; /** State object for the row creation mutation (isPending, isError, error) */ createRowMutation: CreateRowMutationState; + /** State object for the row update mutation (isPending, isError, error) */ + updateRowMutation: CreateRowMutationState; + /** Callback when an existing row is updated (receives new value and original value) */ + handleUpdateRow: (value: string, originalValue: string) => void; + /** ID of the row currently in edit mode */ + editingRowId: RowId | null; + /** Setter function for which row is in edit mode */ + setEditingRowId: (id: RowId | null) => void; + /** Callback to exit draft mode without saving changes */ + exitDraftWithoutSave: () => void; + /** Column definitions for rendering cells in edit mode */ + columns?: TreeColumnDef[]; /** Validation function for new row values (receives value and optional 'soft' or 'hard' mode; * in 'hard' mode an exception is thrown on validation failure) */ validate: (value: string, mode?: 'soft' | 'hard') => boolean; @@ -67,6 +80,12 @@ const NestedRows = ({ setCreatingParentId = () => {}, setIsCreatingTopRow, createRowMutation, + updateRowMutation, + handleUpdateRow, + editingRowId, + setEditingRowId, + exitDraftWithoutSave, + columns = [], validate, }: NestedRowsProps) => { if (!parentRow.getIsExpanded()) { @@ -93,26 +112,43 @@ const NestedRows = ({ const rowData = row.original || row; return ( - - {row.getVisibleCells() - .map((cell, index) => { - const content = flexRender(cell.column.columnDef.cell, cell.getContext()); - const isFirstColumn = index === 0; + {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( + handleUpdateRow(value, String(row.original.value))} + cancelEditRow={() => { + setEditingRowId(null); + exitDraftWithoutSave(); + }} + updateRowMutation={updateRowMutation} + columns={columns} + indent={indent} + validate={validate} + /> + ) : ( + + {row.getVisibleCells() + .map((cell, index) => { + const content = flexRender(cell.column.columnDef.cell, cell.getContext()); + const isFirstColumn = index === 0; - return ( - - {isFirstColumn ? ( -
{content}
- ) : ( - content - )} - - ); - })} - + return ( + + {isFirstColumn ? ( +
{content}
+ ) : ( + content + )} + + ); + })} + + )} diff --git a/src/taxonomy/tree-table/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index 77a9b7af21..6e1819a289 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -13,7 +13,7 @@ import type { TreeColumnDef, TreeTable, } from './types'; -import { CreateRow } from './CreateRow'; +import { CreateRow, EditRow } from './CreateRow'; interface TableBodyProps { columns: TreeColumnDef[]; @@ -26,9 +26,13 @@ interface TableBodyProps { setCreatingParentId: (id: RowId | null) => void; setDraftError: (error: string) => void; createRowMutation: CreateRowMutationState; + updateRowMutation: CreateRowMutationState; table: TreeTable; isLoading: boolean; validate: (value: string, mode?: 'soft' | 'hard') => boolean; + handleUpdateRow: (value: string, originalValue: string) => void; + editingRowId: RowId | null; + setEditingRowId: (id: RowId | null) => void; } const TableBody = ({ @@ -42,9 +46,13 @@ const TableBody = ({ setCreatingParentId, setDraftError, createRowMutation, + updateRowMutation, table, isLoading, validate, + handleUpdateRow, + editingRowId, + setEditingRowId, }: TableBodyProps) => { const intl = useIntl(); @@ -85,14 +93,30 @@ const TableBody = ({ {table.getRowModel().rows.filter(row => row.depth === 0).map(row => ( - - {row.getVisibleCells() - .map((cell, index) => ( - - {flexRender(cell.column.columnDef.cell, cell.getContext())} - - ))} - + {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( + handleUpdateRow(value, String(row.original.value))} + cancelEditRow={() => { + setEditingRowId(null); + exitDraftWithoutSave(); + }} + updateRowMutation={updateRowMutation} + columns={columns} + validate={validate} + /> + ) : ( + + {row.getVisibleCells() + .map((cell, index) => ( + + {flexRender(cell.column.columnDef.cell, cell.getContext())} + + ))} + + )} ))} diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index 47a4720b7f..f1382eeea5 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -29,6 +29,7 @@ const baseProps = () => ({ isCreatingTopRow: false, draftError: '', createRowMutation: { isPending: false, isError: false }, + updateRowMutation: { isPending: false, isError: false }, toast: { show: false, message: '', variant: 'success' }, setToast: jest.fn(), setIsCreatingTopRow: jest.fn(), @@ -38,6 +39,9 @@ const baseProps = () => ({ setCreatingParentId: jest.fn(), setDraftError: jest.fn(), validate: jest.fn(() => true), + handleUpdateRow: jest.fn(), + editingRowId: null, + setEditingRowId: jest.fn(), }); describe('TableView', () => { diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 19f347b06d..79aa625259 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -42,6 +42,7 @@ interface TableViewProps { isCreatingTopRow: boolean; draftError: string; createRowMutation: CreateRowMutationState; + updateRowMutation: CreateRowMutationState; toast: ToastState; setToast: React.Dispatch>; setIsCreatingTopRow: (isCreating: boolean) => void; @@ -51,6 +52,9 @@ interface TableViewProps { setCreatingParentId: (id: RowId | null) => void; setDraftError: (error: string) => void; validate: (value: string, mode?: 'soft' | 'hard') => boolean; + handleUpdateRow: (value: string, originalValue: string) => void; + editingRowId: RowId | null; + setEditingRowId: (id: RowId | null) => void; } const TableView = ({ @@ -64,6 +68,7 @@ const TableView = ({ isCreatingTopRow, draftError, createRowMutation, + updateRowMutation, handleCreateRow, toast, setToast, @@ -73,6 +78,9 @@ const TableView = ({ setCreatingParentId, setDraftError, validate, + handleUpdateRow, + editingRowId, + setEditingRowId, }: TableViewProps) => { const intl = useIntl(); @@ -93,11 +101,12 @@ const TableView = ({ const currentPageIndex = table.getState().pagination.pageIndex + 1; const { isError } = createRowMutation; + const { isError: isUpdateError } = updateRowMutation; const [showError, setShowError] = React.useState(true); return ( <> - {isError && showError && ( + {(isError || isUpdateError) && showError && ( setShowError(false)}> {intl.formatMessage(messages.errorSavingTitle)} @@ -157,9 +166,13 @@ const TableView = ({ setCreatingParentId={setCreatingParentId} setDraftError={setDraftError} createRowMutation={createRowMutation} + updateRowMutation={updateRowMutation} table={table} isLoading={isLoading} validate={validate} + handleUpdateRow={handleUpdateRow} + editingRowId={editingRowId} + setEditingRowId={setEditingRowId} /> From 285e329a8721fda3721d36db8cd88d4aa2de192e Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 31 Mar 2026 13:16:44 -0400 Subject: [PATCH 2/6] fix: PR comments --- src/taxonomy/tag-list/TagListTable.test.jsx | 110 +++++++++----------- src/taxonomy/tag-list/tagColumns.tsx | 3 - 2 files changed, 48 insertions(+), 65 deletions(-) diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index 53018661e2..1da70cedd4 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -160,9 +160,7 @@ const expectNoDraftRows = () => { const openTopLevelDraftRow = async () => { const addButton = await screen.findByLabelText('Create Tag'); - await act(async () => { - fireEvent.click(addButton); - }); + fireEvent.click(addButton); const creatingRow = await screen.findByTestId('creating-top-row'); const input = creatingRow.querySelector('input'); expect(input).toBeInTheDocument(); @@ -172,18 +170,14 @@ const openTopLevelDraftRow = async () => { const openActionsMenuForTag = (tagName, actionButtonName = /actions/i) => { if (!screen.queryAllByText(tagName)?.length) { // expand all - const expandButton = screen.queryAllByText('Expand All')?.[0]; - act(() => { - if (expandButton) { - fireEvent.click(expandButton); - } - }); + const expandButton = screen.queryByRole('button', { name: 'Expand All' }); + if (expandButton) { + fireEvent.click(expandButton); + } } const row = screen.getByText(tagName).closest('tr'); const actionsButton = within(row).getByRole('button', { name: actionButtonName }); - act(() => { - fireEvent.click(actionsButton); - }); + fireEvent.click(actionsButton); return row; }; @@ -203,8 +197,8 @@ const openSubtagDraftRow = async ({ const openRenameDraftRow = async (tagName = 'root tag 1') => { openActionsMenuForTag(tagName); - fireEvent.click(screen.getByText('Rename')); - const input = screen.getByRole('textbox'); + fireEvent.click(screen.getByRole('button', { name: /Rename/i })); + const input = screen.getByRole('textbox', { name: /type tag name/i }); const row = input.closest('tr'); expect(row).toBeInTheDocument(); const saveButton = within(row).getByText('Save'); @@ -305,7 +299,7 @@ describe('', () => { await waitForRootTag(); openActionsMenuForTag('root tag 1'); - expect(screen.getByText('Add Subtag')).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Add Subtag' })).toHaveAttribute('aria-disabled', 'true'); expect(screen.getByLabelText('Create Tag')).toBeDisabled(); }); @@ -332,7 +326,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'a new tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); await waitFor(() => { expect(axiosMock.history.post.length).toBe(1); @@ -381,7 +375,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'a new tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); const spinner = await screen.findByRole('status'); expect(spinner.textContent).toEqual('Saving...'); @@ -398,7 +392,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'a new tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); let newTag; await waitFor(() => { @@ -425,7 +419,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'a new tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); const toast = await screen.findByText('Tag "a new tag" created successfully'); expect(toast).toBeInTheDocument(); @@ -442,7 +436,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'xyz tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); // no input row should be in the document await waitFor(() => { @@ -464,7 +458,7 @@ describe('', () => { const { creatingRow, input } = await openTopLevelDraftRow(); fireEvent.change(input, { target: { value: 'xyz tag' } }); - const saveButton = within(creatingRow).getByText('Save'); + const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); const temporaryRow = await screen.findByText('xyz tag'); // temporaryRow should be at the top of the table, that is, the first row after the header @@ -506,7 +500,7 @@ describe('', () => { expect(input).toBeInTheDocument(); fireEvent.change(input, { target: { value: 'Tag A' } }); - let saveButton = within(creatingRow).getByText('Save'); + let saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); const tagA = await screen.findByText('Tag A'); expect(tagA).toBeInTheDocument(); @@ -518,7 +512,7 @@ describe('', () => { expect(input).toBeInTheDocument(); fireEvent.change(input, { target: { value: 'Tag B' } }); - saveButton = within(creatingRow).getByText('Save'); + saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); fireEvent.click(saveButton); const tagB = await screen.findByText('Tag B'); expect(tagB).toBeInTheDocument(); @@ -539,7 +533,7 @@ describe('', () => { const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); expect(input).toBeInTheDocument(); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); expect(saveButton).toBeDisabled(); fireEvent.change(input, { target: { value: 'a new tag' } }); expect(saveButton).not.toBeDisabled(); @@ -551,7 +545,7 @@ describe('', () => { const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); expect(input).toBeInTheDocument(); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); expect(saveButton).toBeDisabled(); fireEvent.change(input, { target: { value: ' ' } }); expect(saveButton).toBeDisabled(); @@ -571,7 +565,7 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Create Tag')); const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); fireEvent.change(input, { target: { value: ' Tag A ' } }); fireEvent.click(saveButton); @@ -586,11 +580,9 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Create Tag')); const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); - await act(async () => { - fireEvent.change(input, { target: { value: 'invalid;tag' } }); - }); + fireEvent.change(input, { target: { value: 'invalid;tag' } }); expect(saveButton).toBeDisabled(); expect(screen.getByText(/invalid character/i)).toBeInTheDocument(); @@ -602,7 +594,7 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Create Tag')); const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); fireEvent.change(input, { target: { value: 'root tag 1' } }); fireEvent.click(saveButton); @@ -618,7 +610,7 @@ describe('', () => { fireEvent.click(await screen.findByLabelText('Create Tag')); const draftRow = await screen.findAllByRole('row'); const input = draftRow[1].querySelector('input'); - const saveButton = within(draftRow[1]).getByText('Save'); + const saveButton = within(draftRow[1]).getByRole('button', { name: 'Save' }); fireEvent.change(input, { target: { value: 'will fail' } }); fireEvent.click(saveButton); @@ -660,7 +652,7 @@ describe('', () => { expect(screen.queryAllByText('Add Subtag').length).toBe(0); // user clicks on row actions for root tag 1 openActionsMenuForTag('root tag 1'); - expect(screen.getByText('Add Subtag')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Add Subtag' })).toBeInTheDocument(); }); it('should render an inline add-subtag row with input, placeholder, and action buttons', async () => { @@ -672,14 +664,14 @@ describe('', () => { const draftRowIndex = rows.findIndex(tableRow => tableRow.querySelector('input')); expect(draftRowIndex).toBe(parentRowIndex + 1); expect(draftRows[0].querySelector('input')).toBeInTheDocument(); - expect(within(draftRows[0]).getByText('Cancel')).toBeInTheDocument(); - expect(within(draftRows[0]).getByText('Save')).toBeInTheDocument(); + expect(within(draftRows[0]).getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + expect(within(draftRows[0]).getByRole('button', { name: 'Save' })).toBeInTheDocument(); }); it('should remove add-subtag row and avoid create request when cancelled', async () => { const { draftRow, input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); fireEvent.change(input, { target: { value: 'new subtag' } }); - fireEvent.click(within(draftRow).getByText('Cancel')); + fireEvent.click(within(draftRow).getByRole('button', { name: 'Cancel' })); await waitFor(() => { expect(axiosMock.history.post.length).toBe(0); @@ -701,14 +693,12 @@ describe('', () => { it('should disable Save and show required-name inline error for empty sub-tag input', async () => { openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); + fireEvent.click(screen.getByRole('button', { name: 'Add Subtag' })); const rows = await screen.findAllByRole('row'); const draftRow = rows.find(tableRow => tableRow.querySelector('input')); - const saveButton = within(draftRow).getByText('Save'); + const saveButton = within(draftRow).getByRole('button', { name: 'Save' }); const input = draftRow.querySelector('input'); - act(() => { - fireEvent.change(input, { target: { value: ' ' } }); - }); + fireEvent.change(input, { target: { value: ' ' } }); expect(saveButton).toBeDisabled(); expect(within(draftRow).getByText(/Name is required/i)).toBeInTheDocument(); @@ -716,11 +706,11 @@ describe('', () => { it('should keep Save disabled for whitespace-only sub-tag input', async () => { openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); + fireEvent.click(screen.getByRole('button', { name: 'Add Subtag' })); const rows = await screen.findAllByRole('row'); const draftRow = rows.find(tableRow => tableRow.querySelector('input')); const input = draftRow.querySelector('input'); - const saveButton = within(draftRow).getByText('Save'); + const saveButton = within(draftRow).getByRole('button', { name: 'Save' }); fireEvent.change(input, { target: { value: ' ' } }); expect(saveButton).toBeDisabled(); @@ -728,11 +718,11 @@ describe('', () => { it('should disable Save and show invalid-character error for sub-tag input', async () => { openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); + fireEvent.click(screen.getByRole('button', { name: 'Add Subtag' })); const rows = await screen.findAllByRole('row'); const draftRow = rows.find(row => row.querySelector('input')); const input = draftRow.querySelector('input'); - const saveButton = within(draftRow).getByText('Save'); + const saveButton = within(draftRow).getByRole('button', { name: 'Save' }); fireEvent.change(input, { target: { value: 'invalid;name' } }); expect(saveButton).toBeDisabled(); @@ -745,12 +735,12 @@ describe('', () => { }); openActionsMenuForTag('root tag 1'); - fireEvent.click(screen.getByText('Add Subtag')); + fireEvent.click(screen.getByRole('button', { name: 'Add Subtag' })); const rows = await screen.findAllByRole('row'); const draftRow = rows.find(row => row.querySelector('input')); const input = draftRow.querySelector('input'); fireEvent.change(input, { target: { value: 'subtag fail' } }); - fireEvent.click(within(draftRow).getByText('Save')); + fireEvent.click(within(draftRow).getByRole('button', { name: 'Save' })); await waitFor(() => { expect(getDraftRows().length).toBe(1); @@ -779,9 +769,7 @@ describe('', () => { const { input } = await openRenameDraftRow('root tag 1'); fireEvent.change(input, { target: { value: 'will fail' } }); - act(() => { - fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); - }); + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); let draftRow; await waitFor(() => { @@ -830,8 +818,8 @@ describe('', () => { await waitForRootTag(); openActionsMenuForTag(tagName); - expect(screen.getByText('Rename')).toBeInTheDocument(); - expect(screen.getByText('Rename')).toBeDisabled(); + expect(screen.getByRole('button', { name: /Rename/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Rename/i })).toHaveAttribute('aria-disabled', 'true'); }); it('should disable tag edit buttons if tag includes `can_edit: false`', async () => { axiosMock.reset(); @@ -842,22 +830,22 @@ describe('', () => { await waitForRootTag(); openActionsMenuForTag(tagName); - expect(screen.getByText('Rename')).toBeInTheDocument(); - expect(screen.getByText('Rename')).toBeDisabled(); + expect(screen.getByRole('button', { name: /Rename/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Rename/i })).toHaveAttribute('aria-disabled', 'true'); }); it('should show tag actions menu', async () => { openActionsMenuForTag(tagName); expect(screen.getByText('Add Subtag')).toBeInTheDocument(); - expect(screen.getByText('Rename')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Rename/i })).toBeInTheDocument(); }); it('should show editable input and action buttons when Rename is selected from actions menu', async () => { const { row } = await openRenameDraftRow(tagName); expect(within(row).getByRole('textbox')).toBeInTheDocument(); // expect the input to be pre-filled with the current tag name expect(within(row).getByRole('textbox').value).toEqual(tagName); - expect(within(row).getByText('Save')).toBeInTheDocument(); - expect(within(row).getByText('Cancel')).toBeInTheDocument(); + expect(within(row).getByRole('button', { name: /Save/i })).toBeInTheDocument(); + expect(within(row).getByRole('button', { name: /Cancel/i })).toBeInTheDocument(); }); it('should disable Save button until the tag name is changed', async () => { const { input, saveButton } = await openRenameDraftRow(tagName); @@ -871,9 +859,7 @@ describe('', () => { const { input } = await openRenameDraftRow(tagName); fireEvent.change(input, { target: { value: `${tagName} updated` } }); - act(() => { - fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); - }); + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); await waitFor(() => { expect(axiosMock.history.patch.length).toBe(1); @@ -989,8 +975,8 @@ describe('', () => { const grandchildTagRow = screen.getByText('the grandchild tag').closest('tr'); expect(within(grandchildTagRow).queryByRole('button', { name: /actions/i })).toBeInTheDocument(); openActionsMenuForTag('the grandchild tag'); - expect(within(grandchildTagRow).getByText('Rename')).toBeInTheDocument(); - expect(within(grandchildTagRow).getByText('Add Subtag')).toBeDisabled(); + expect(within(grandchildTagRow).getByRole('button', { name: /Rename/i })).toBeInTheDocument(); + expect(within(grandchildTagRow).getByText('Add Subtag')).toHaveAttribute('aria-disabled', 'true'); }); }); }); diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 4710d92bc7..594ec6fe6e 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -1,5 +1,4 @@ import { - Button, Icon, IconButton, IconButtonWithTooltip, @@ -127,14 +126,12 @@ const ActionsMenu = ({ /> {intl.formatMessage(messages.addSubtag)} From e82b4205d56627fbba6c0d7bafae987dd7f95e74 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 31 Mar 2026 13:26:31 -0400 Subject: [PATCH 3/6] fix: PR comments --- src/taxonomy/tag-list/TagListTable.test.jsx | 84 ++++++--------------- 1 file changed, 23 insertions(+), 61 deletions(-) diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index 1da70cedd4..036c9ca0d9 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -1,25 +1,14 @@ import React from 'react'; -import PropTypes from 'prop-types'; -import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { initializeMockApp } from '@edx/frontend-platform'; -import { AppProvider } from '@edx/frontend-platform/react'; import { render, waitFor, waitForElementToBeRemoved, screen, within, - fireEvent, act, cleanup, -} from '@testing-library/react'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import MockAdapter from 'axios-mock-adapter'; - -import initializeStore from '../../store'; + fireEvent, act, cleanup, initializeMocks, +} from '@src/testUtils'; import * as apiHooksModule from '../data/apiHooks'; import * as hooksModule from './hooks'; import * as treeTableModule from '../tree-table'; import TagListTable from './TagListTable'; -let store; let axiosMock; -const queryClient = new QueryClient(); const adminUser = { userId: 3, username: 'abc123', @@ -31,20 +20,6 @@ const nonAdminUser = { administrator: false, }; -const RootWrapper = ({ maxDepth = 3 }) => ( - - - - - - - -); - -RootWrapper.propTypes = { - maxDepth: PropTypes.number, -}; - const tagDefaults = { depth: 0, external_id: '', parent_value: null }; const mockTagsResponse = { next: null, @@ -138,7 +113,7 @@ const subTagsResponse = { const subTagsUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?full_depth_threshold=10000&parent_tag=root+tag+1'; const createTagUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/'; -const renderTagListTable = (maxDepth = 3) => render(); +const renderTagListTable = (maxDepth = 3) => render(); const flushReactUpdates = async () => { await act(async () => { @@ -212,20 +187,8 @@ const openRenameDraftRow = async (tagName = 'root tag 1') => { }; describe('', () => { - beforeAll(async () => { - initializeMockApp({ - authenticatedUser: adminUser, - }); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - }); beforeEach(async () => { - initializeMockApp({ - authenticatedUser: adminUser, - }); - store = initializeStore(); - queryClient.clear(); - axiosMock.reset(); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + ({ axiosMock } = initializeMocks({ user: adminUser })); axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); @@ -294,7 +257,12 @@ describe('', () => { can_add_tag: false, }); cleanup(); - queryClient.clear(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, { + ...mockTagsResponse, + can_add_tag: false, + }); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); await waitForRootTag(); @@ -640,7 +608,7 @@ describe('', () => { }); it('should hide Add Tag for users without taxonomy edit permissions', async () => { - initializeMockApp({ authenticatedUser: nonAdminUser }); + initializeMocks({ user: nonAdminUser }); axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); expect(screen.queryByText('Add Tag')).not.toBeInTheDocument(); @@ -749,7 +717,7 @@ describe('', () => { }); it('should hide or disable Add sub-tag actions when user lacks edit permissions', async () => { - initializeMockApp({ authenticatedUser: nonAdminUser }); + initializeMocks({ user: nonAdminUser }); const addSubtagActions = screen.queryAllByText('Add Subtag'); if (addSubtagActions.length === 0) { expect(addSubtagActions.length).toBe(0); @@ -813,7 +781,12 @@ describe('', () => { can_add_tag: false, }); cleanup(); - queryClient.clear(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, { + ...mockTagsResponse, + can_add_tag: false, + }); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); await waitForRootTag(); @@ -825,7 +798,9 @@ describe('', () => { axiosMock.reset(); axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); cleanup(); - queryClient.clear(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); await waitForRootTag(); @@ -984,20 +959,8 @@ describe('', () => { // These async creation flows are intentionally isolated because they pass individually // but can be flaky when interleaved with the larger suite's async/query timing. describe(' isolated async subtag tests', () => { - beforeAll(async () => { - initializeMockApp({ - authenticatedUser: adminUser, - }); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - }); - beforeEach(async () => { - initializeMockApp({ - authenticatedUser: adminUser, - }); - store = initializeStore(); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - queryClient.clear(); + ({ axiosMock } = initializeMocks({ user: adminUser })); }); it('shows the spinner before the query is complete', async () => { @@ -1196,8 +1159,7 @@ describe(' pagination transition behavior', () => { beforeEach(() => { tableViewProps = null; mockEnterViewMode.mockReset(); - store = initializeStore(); - queryClient.clear(); + initializeMocks({ user: adminUser }); jest.spyOn(apiHooksModule, 'useTagListData').mockReturnValue({ isLoading: false, From 569029698d6e114070e274e603dbabafcf4e96a6 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 31 Mar 2026 18:59:13 -0400 Subject: [PATCH 4/6] fix: PR comments --- src/taxonomy/tree-table/NestedRows.test.tsx | 33 ++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/taxonomy/tree-table/NestedRows.test.tsx b/src/taxonomy/tree-table/NestedRows.test.tsx index 0909ad5a86..7d68f5f15b 100644 --- a/src/taxonomy/tree-table/NestedRows.test.tsx +++ b/src/taxonomy/tree-table/NestedRows.test.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; -import { fireEvent, render, screen } from '@testing-library/react'; +import { fireEvent, render, screen, within } from '@testing-library/react'; import NestedRows from './NestedRows'; @@ -96,4 +96,35 @@ describe('NestedRows', () => { expect(setCreatingParentId).toHaveBeenCalledWith(null); expect(onCancelCreation).toHaveBeenCalled(); }); + + it('renders EditRow when editingRowId matches the child row id and value', () => { + const nestedChild = makeRow({ id: 2, value: 'child', expanded: true }); + const parent = makeRow({ + id: 1, + value: 'parent', + expanded: true, + subRows: [nestedChild], + }); + + render( + + + + +
, + { wrapper }, + ); + + const childInput = screen.getByDisplayValue('child'); + const cancelButton = screen.getByRole('button', { name: /Cancel/i }); + + expect(childInput).toBeInTheDocument(); + expect(cancelButton).toBeInTheDocument(); + }); }); From fe3231f341745f2877cf7b9926dcf5249d789a4d Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 1 Apr 2026 18:24:36 -0400 Subject: [PATCH 5/6] fix: autofocus --- src/taxonomy/tree-table/EditableCell.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/taxonomy/tree-table/EditableCell.tsx b/src/taxonomy/tree-table/EditableCell.tsx index 09a660c159..8cdc10aabc 100644 --- a/src/taxonomy/tree-table/EditableCell.tsx +++ b/src/taxonomy/tree-table/EditableCell.tsx @@ -48,7 +48,6 @@ const EditableCell = ({ useEffect(() => { if (autoFocus && inputRef.current) { inputRef.current.focus(); - inputRef.current.select(); } }, [inputRef.current]); // autoFocus explicitly not a dependency, to avoid unexpected focus change. From 0ae30acc268b021988cbce6bd073ff23310ebbb8 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 1 Apr 2026 18:24:49 -0400 Subject: [PATCH 6/6] fix: max depth --- src/taxonomy/taxonomy-detail/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/taxonomy/taxonomy-detail/constants.ts b/src/taxonomy/taxonomy-detail/constants.ts index 442573c7c3..fedec20613 100644 --- a/src/taxonomy/taxonomy-detail/constants.ts +++ b/src/taxonomy/taxonomy-detail/constants.ts @@ -6,6 +6,6 @@ * tags exceeding this depth, they will still be rendered, but further nesting will be blocked. * * **Sync Required**: This must match `TAXONOMY_MAX_DEPTH` in the openedx-core backend. */ -const TAXONOMY_MAX_DEPTH = 3; +const TAXONOMY_MAX_DEPTH = 5; export { TAXONOMY_MAX_DEPTH };