diff --git a/src/taxonomy/data/api.ts b/src/taxonomy/data/api.ts index eb50241106..3a4545502e 100644 --- a/src/taxonomy/data/api.ts +++ b/src/taxonomy/data/api.ts @@ -91,6 +91,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 3207f5665a..479cc2c4db 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -240,3 +240,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 6074dc1e23..4fbf2a1b63 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, @@ -103,6 +78,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, @@ -134,7 +118,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 () => { @@ -156,9 +140,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(); @@ -166,11 +148,16 @@ const openTopLevelDraftRow = async () => { }; const openActionsMenuForTag = (tagName, actionButtonName = /actions/i) => { + if (!screen.queryAllByText(tagName)?.length) { + // expand all + 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; }; @@ -188,16 +175,25 @@ const openSubtagDraftRow = async ({ return { rows, draftRow, input }; }; +const openRenameDraftRow = async (tagName = 'root tag 1') => { + openActionsMenuForTag(tagName); + 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'); + const cancelButton = within(row).getByText('Cancel'); + return { + row, + input, + saveButton, + cancelButton, + }; +}; + describe('', () => { - beforeAll(async () => { - initializeMockApp({ - authenticatedUser: adminUser, - }); - axiosMock = new MockAdapter(getAuthenticatedHttpClient()); - }); beforeEach(async () => { - store = initializeStore(); - axiosMock.reset(); + ({ axiosMock } = initializeMocks({ user: adminUser })); axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse); axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); @@ -297,7 +293,31 @@ 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(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, { + ...mockTagsResponse, + can_add_tag: false, + }); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag('root tag 1'); + expect(screen.getByRole('button', { name: 'Add Subtag' })).toHaveAttribute('aria-disabled', 'true'); + 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(); @@ -316,7 +336,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); @@ -365,7 +385,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...'); @@ -382,7 +402,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(() => { @@ -409,7 +429,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(); @@ -426,7 +446,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(() => { @@ -448,7 +468,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 @@ -471,6 +491,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, { @@ -488,7 +510,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(); @@ -500,7 +522,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(); @@ -512,7 +534,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 () => { @@ -521,7 +543,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(); @@ -533,7 +555,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(); @@ -553,7 +575,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); @@ -568,11 +590,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(); @@ -584,7 +604,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); @@ -600,7 +620,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); @@ -630,7 +650,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(); @@ -638,120 +658,285 @@ 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.getByRole('button', { name: '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]).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).getByRole('button', { name: 'Cancel' })); + + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(0); + expectNoDraftRows(); }); + }); - 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 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' }); + + await waitFor(() => { + expect(axiosMock.history.post.length).toBe(0); + expectNoDraftRows(); }); + }); - 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 disable Save and show required-name inline error for empty sub-tag input', async () => { + openActionsMenuForTag('root tag 1'); + 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).getByRole('button', { name: 'Save' }); + const input = draftRow.querySelector('input'); + fireEvent.change(input, { target: { value: ' ' } }); + + expect(saveButton).toBeDisabled(); + expect(within(draftRow).getByText(/Name is required/i)).toBeInTheDocument(); + }); - await waitFor(() => { - expect(axiosMock.history.post.length).toBe(0); - expectNoDraftRows(); - }); + it('should keep Save disabled for whitespace-only sub-tag input', async () => { + openActionsMenuForTag('root tag 1'); + 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).getByRole('button', { name: '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.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).getByRole('button', { name: 'Save' }); + + 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 remove add-subtag row and avoid create request on escape key', async () => { - const { input } = await openSubtagDraftRow({ tagName: 'root tag 1' }); + openActionsMenuForTag('root tag 1'); + 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).getByRole('button', { name: 'Save' })); - fireEvent.change(input, { target: { value: 'new subtag' } }); - fireEvent.keyDown(input, { key: 'Escape', code: 'Escape' }); + await waitFor(() => { + expect(getDraftRows().length).toBe(1); + }); + expect(await screen.findByText(/Error creating tag:/i)).toBeInTheDocument(); + }); - await waitFor(() => { - expect(axiosMock.history.post.length).toBe(0); - expectNoDraftRows(); + it('should hide or disable Add sub-tag actions when user lacks edit permissions', async () => { + initializeMocks({ user: nonAdminUser }); + const addSubtagActions = screen.queryAllByText('Add Subtag'); + if (addSubtagActions.length === 0) { + expect(addSubtagActions.length).toBe(0); + } else { + addSubtagActions.forEach(action => { + expect(action).toBeDisabled(); }); + } + }); + }); + + 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'); - 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: ' ' } }); - }); + fireEvent.change(input, { target: { value: 'will fail' } }); + fireEvent.keyDown(input, { key: 'Enter', code: 'Enter' }); - expect(saveButton).toBeDisabled(); - expect(within(draftRow).getByText(/Name is required/i)).toBeInTheDocument(); + 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 }); - 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'); + // Banner error message should be shown at the top of the table + expect(await screen.findByText(/Error saving changes/i)).toBeInTheDocument(); - fireEvent.change(input, { target: { value: ' ' } }); - expect(saveButton).toBeDisabled(); + // Toast message to indicate that the save failed + expect(await screen.findByText(/Error updating tag:\s*.+/i)).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 tag includes `can_edit: false`', async () => { + axiosMock.reset(); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + cleanup(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag(tagName); + expect(screen.getByRole('button', { name: /Rename/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Rename/i })).toHaveAttribute('aria-disabled', 'true'); }); - 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 show tag actions menu', async () => { + openActionsMenuForTag(tagName); + expect(screen.getByText('Add Subtag')).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).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); - fireEvent.change(input, { target: { value: 'invalid;name' } }); expect(saveButton).toBeDisabled(); - expect(within(draftRow).getByText(/invalid character/i)).toBeInTheDocument(); + 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); - it('should keep inline row and show failure feedback when sub-tag save fails', async () => { - axiosMock.onPost(createTagUrl).reply(500, { - error: 'Internal server error', + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + 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); - 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')); + fireEvent.change(input, { target: { value: `${tagName} updated` } }); + fireEvent.click(saveButton); await waitFor(() => { - expect(getDraftRows().length).toBe(1); + 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(/Error creating tag:/i)).toBeInTheDocument(); + 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); }); }); + }); - it('should hide or disable Add sub-tag actions when user lacks edit permissions', async () => { - initializeMockApp({ authenticatedUser: nonAdminUser }); - const addSubtagActions = screen.queryAllByText('Add Subtag'); - if (addSubtagActions.length === 0) { - expect(addSubtagActions.length).toBe(0); - } else { - addSubtagActions.forEach(action => { - expect(action).toBeDisabled(); - }); - } + 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(); }); }); @@ -785,7 +970,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).getByRole('button', { name: /Rename/i })).toBeInTheDocument(); + expect(within(grandchildTagRow).getByText('Add Subtag')).toHaveAttribute('aria-disabled', 'true'); }); }); }); @@ -793,20 +981,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 () => { @@ -1005,8 +1181,7 @@ describe(' pagination transition behavior', () => { beforeEach(() => { tableViewProps = null; mockEnterViewMode.mockReset(); - store = initializeStore(); - queryClient.clear(); + initializeMocks({ user: adminUser }); jest.spyOn(apiHooksModule, 'useTagListData').mockReturnValue({ isLoading: false, diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 01bfdbfe11..c9c80adb08 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -4,7 +4,7 @@ import React, { useEffect, } from 'react'; 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 { @@ -78,6 +78,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. @@ -88,6 +89,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setTagTree, setDraftError, createTagMutation, + updateTagMutation, enterPreviewMode, setToast, setIsCreatingTopTag, @@ -105,19 +107,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, @@ -155,7 +157,9 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { isCreatingTopRow: isCreatingTopTag, draftError, createRowMutation: createTagMutation, + updateRowMutation: updateTagMutation, handleCreateRow: handleCreateTag, + handleUpdateRow: handleUpdateTag, toast, setToast, setIsCreatingTopRow: setIsCreatingTopTag, @@ -164,6 +168,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 b85c8a4696..69e74221ea 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -57,6 +57,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 2fad2e25c6..4f428fbdf5 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -1,6 +1,5 @@ import { Bubble, - Button, Icon, IconButton, IconButtonWithTooltip, @@ -21,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; @@ -42,11 +43,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; } const UsageCountDisplay = ({ row }: { row: Row }) => { @@ -68,6 +69,7 @@ interface ActionsHeaderProps { setActiveActionMenuRowId: (id: RowId | null) => void; hasOpenDraft: boolean; draftInProgressHintId: string; + canAddTag: boolean; } const ActionsHeader = ({ @@ -77,6 +79,7 @@ const ActionsHeader = ({ setEditingRowId, setActiveActionMenuRowId, hasOpenDraft, + canAddTag, draftInProgressHintId, }: ActionsHeaderProps) => { const intl = useIntl(); @@ -95,7 +98,7 @@ const ActionsHeader = ({ setEditingRowId(null); setActiveActionMenuRowId(null); }} - disabled={hasOpenDraft} + disabled={hasOpenDraft || !canAddTag} aria-describedby={hasOpenDraft ? draftInProgressHintId : undefined} /> @@ -106,9 +109,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 ( @@ -124,12 +139,17 @@ const ActionsMenu = ({ rowData, startSubtagDraft, disableAddSubtag }: ActionsMen /> {intl.formatMessage(messages.addSubtag)} + + {intl.formatMessage(messages.renameTag)} + ); @@ -142,13 +162,12 @@ 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'; - const intl = useIntl(); return [ { @@ -169,7 +188,7 @@ function getColumns({ }, { id: 'count', - header: intl.formatMessage(messages.tagListColumnCountHeader), + header: () => , cell: UsageCountDisplay, }, { @@ -183,16 +202,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 || row.original.canChangeTag === false; + const startSubtagDraft = () => { onStartDraft(); setDraftError(''); @@ -203,9 +225,26 @@ function getColumns({ row.toggleExpanded(true); }; + const editTag = () => { + onStartDraft(); + setDraftError(''); + setEditingRowId(`${rowData.id}:${rowData.value}`); + setCreatingParentId(null); + setIsCreatingTopTag(false); + setActiveActionMenuRowId(null); + }; + return (
- +
); }, @@ -213,4 +252,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..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'; @@ -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} /> , @@ -88,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(); + }); }); 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} />