From 2f5eb9c1d469ecf9560a8319d5642e1d447747f8 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Wed, 4 Jan 2023 11:16:42 +0000 Subject: [PATCH] Merge pull request #11016 from overleaf/ii-dashboard-wrong-action-buttons [web] Project dashboard wrong action buttons GitOrigin-RevId: d76c64616a34c63b60bc499b96c6507c1196e666 --- .../context/project-list-context.tsx | 4 +- .../components/project-list-root.test.tsx | 317 ++++++++++-------- .../project-tools/project-tools.test.tsx | 2 +- 3 files changed, 180 insertions(+), 143 deletions(-) diff --git a/services/web/frontend/js/features/project-list/context/project-list-context.tsx b/services/web/frontend/js/features/project-list/context/project-list-context.tsx index 9079c71579..904d21e6d5 100644 --- a/services/web/frontend/js/features/project-list/context/project-list-context.tsx +++ b/services/web/frontend/js/features/project-list/context/project-list-context.tsx @@ -186,7 +186,6 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { tag?.project_ids?.includes(project.id) ) } else { - setFilter('all') setSelectedTagId(undefined) } } @@ -290,9 +289,10 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { const selectTag = useCallback( (tagId: string) => { + setFilter('all') setSelectedTagId(tagId) }, - [setSelectedTagId] + [setSelectedTagId, setFilter] ) const addTag = useCallback((tag: Tag) => { diff --git a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx index 78b0305c7b..61d5fa560a 100644 --- a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx +++ b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx @@ -665,159 +665,196 @@ describe('', function () { }) }) - describe('project tools "More" dropdown', function () { - beforeEach(async function () { - const filterButton = screen.getAllByText('All Projects')[0] - fireEvent.click(filterButton) - allCheckboxes = screen.getAllByRole('checkbox') - // first one is the select all checkbox - fireEvent.click(allCheckboxes[2]) + describe('project tools', function () { + it('renders download, archive, trash buttons followed by tags and "more" dropdowns when selecting an archived or trashed filter before selecting tag filter', function () { + const assertToolbarButtonsExists = () => { + within(actionsToolbar).getByLabelText(/download/i) + within(actionsToolbar).getByLabelText(/archive/i) + within(actionsToolbar).getByLabelText(/trash/i) + within(actionsToolbar).getByLabelText(/tags/i) + within(actionsToolbar).getByText(/more/i) + } + + // Select archived projects + const [archivedProjectsButton] = + screen.getAllByText(/archived projects/i) + fireEvent.click(archivedProjectsButton) + const [tag] = screen.getAllByText(this.tagName) + fireEvent.click(tag) + + allCheckboxes = screen.getAllByRole('checkbox') + fireEvent.click(allCheckboxes[1]) actionsToolbar = screen.getAllByRole('toolbar')[0] + + assertToolbarButtonsExists() + + // select trashed projects + const [trashedProjectsButton] = + screen.getAllByText(/trashed projects/i) + fireEvent.click(trashedProjectsButton) + fireEvent.click(tag) + + allCheckboxes = screen.getAllByRole('checkbox') + fireEvent.click(allCheckboxes[1]) + actionsToolbar = screen.getAllByRole('toolbar')[0] + + assertToolbarButtonsExists() }) - it('does not show the dropdown when more than 1 project is selected', async function () { - await waitFor(() => { - within(actionsToolbar).getByText('More') - }) - fireEvent.click(allCheckboxes[0]) - expect(within(actionsToolbar).queryByText('More')).to.be - .null - }) - - it('validates the project name', async function () { - const moreDropdown = await within( - actionsToolbar - ).findByText('More') - fireEvent.click(moreDropdown) - - const renameButton = - screen.getAllByText('Rename')[1] // first one is for the tag in the sidebar - fireEvent.click(renameButton) - - const modals = await screen.findAllByRole('dialog') - const modal = modals[0] - - expect(sendSpy).to.be.calledOnce - expect(sendSpy).calledWith('project-list-page-interaction') - - // same name - let confirmButton = - within(modal).getByText('Rename') - expect(confirmButton.disabled).to.be.true - let input = screen.getByLabelText('New Name') as HTMLButtonElement - - // no name - input = screen.getByLabelText('New Name') as HTMLButtonElement - fireEvent.change(input, { - target: { value: '' }, - }) - confirmButton = within(modal).getByText('Rename') - expect(confirmButton.disabled).to.be.true - }) - - it('opens the rename modal, and can rename the project, and view updated', async function () { - const renameProjectMock = fetchMock.post( - `express:/project/:id/rename`, - { - status: 200, - } - ) - const moreDropdown = await within( - actionsToolbar - ).findByText('More') - fireEvent.click(moreDropdown) - - const renameButton = - within(actionsToolbar).getByText('Rename') // first one is for the tag in the sidebar - fireEvent.click(renameButton) - - const modals = await screen.findAllByRole('dialog') - const modal = modals[0] - - // a valid name - const newProjectName = 'A new project name' - const input = (await within(modal).findByLabelText( - 'New Name' - )) as HTMLButtonElement - const oldName = input.value - fireEvent.change(input, { - target: { value: newProjectName }, + describe('"More" dropdown', function () { + beforeEach(async function () { + const filterButton = screen.getAllByText('All Projects')[0] + fireEvent.click(filterButton) + allCheckboxes = screen.getAllByRole('checkbox') + // first one is the select all checkbox + fireEvent.click(allCheckboxes[2]) + actionsToolbar = screen.getAllByRole('toolbar')[0] }) - const confirmButton = - within(modal).getByText('Rename') - expect(confirmButton.disabled).to.be.false - fireEvent.click(confirmButton) - - await fetchMock.flush(true) - - expect( - renameProjectMock.called(`/project/${projectsData[1].id}/rename`) - ).to.be.true - - const table = await screen.findByRole('table') - within(table).getByText(newProjectName) - expect(within(table).queryByText(oldName)).to.be.null - - const allCheckboxes = await within( - table - ).findAllByRole('checkbox') - const allCheckboxesChecked = allCheckboxes.filter(c => c.checked) - expect(allCheckboxesChecked.length).to.equal(0) - }) - - it('opens the copy modal, can copy the project, and view updated', async function () { - const tableRows = screen.getAllByRole('row') - const linkForProjectToCopy = within(tableRows[1]).getByRole('link') - const projectNameToCopy = linkForProjectToCopy.textContent || '' // needed for type checking - screen.getByText(projectNameToCopy) // make sure not just empty string - const copiedProjectName = `${projectNameToCopy} (Copy)` - const cloneProjectMock = fetchMock.post( - `express:/project/:id/clone`, - { - status: 200, - body: { - name: copiedProjectName, - lastUpdated: new Date(), - project_id: userId, - owner_ref: userId, - owner, - id: '6328e14abec0df019fce0be5', - lastUpdatedBy: owner, - accessLevel: 'owner', - source: 'owner', - trashed: false, - archived: false, - }, - } - ) - - await waitFor(() => { - const moreDropdown = + it('does not show the dropdown when more than 1 project is selected', async function () { + await waitFor(() => { within(actionsToolbar).getByText('More') - fireEvent.click(moreDropdown) + }) + fireEvent.click(allCheckboxes[0]) + expect(within(actionsToolbar).queryByText('More')).to + .be.null }) - const copyButton = - within(actionsToolbar).getByText('Make a copy') - fireEvent.click(copyButton) + it('validates the project name', async function () { + const moreDropdown = await within( + actionsToolbar + ).findByText('More') + fireEvent.click(moreDropdown) - // confirm in modal - const copyConfirmButton = document.querySelector( - 'button[type="submit"]' - ) as HTMLElement - fireEvent.click(copyConfirmButton) + const renameButton = + screen.getAllByText('Rename')[1] // first one is for the tag in the sidebar + fireEvent.click(renameButton) - await fetchMock.flush(true) + const modals = await screen.findAllByRole('dialog') + const modal = modals[0] - expect( - cloneProjectMock.called(`/project/${projectsData[1].id}/clone`) - ).to.be.true + expect(sendSpy).to.be.calledOnce + expect(sendSpy).calledWith('project-list-page-interaction') - expect(sendSpy).to.be.calledOnce - expect(sendSpy).calledWith('project-list-page-interaction') + // same name + let confirmButton = + within(modal).getByText('Rename') + expect(confirmButton.disabled).to.be.true + let input = screen.getByLabelText('New Name') as HTMLButtonElement - screen.getByText(copiedProjectName) + // no name + input = screen.getByLabelText('New Name') as HTMLButtonElement + fireEvent.change(input, { + target: { value: '' }, + }) + confirmButton = within(modal).getByText('Rename') + expect(confirmButton.disabled).to.be.true + }) + + it('opens the rename modal, and can rename the project, and view updated', async function () { + const renameProjectMock = fetchMock.post( + `express:/project/:id/rename`, + { + status: 200, + } + ) + const moreDropdown = await within( + actionsToolbar + ).findByText('More') + fireEvent.click(moreDropdown) + + const renameButton = + within(actionsToolbar).getByText('Rename') // first one is for the tag in the sidebar + fireEvent.click(renameButton) + + const modals = await screen.findAllByRole('dialog') + const modal = modals[0] + + // a valid name + const newProjectName = 'A new project name' + const input = (await within(modal).findByLabelText( + 'New Name' + )) as HTMLButtonElement + const oldName = input.value + fireEvent.change(input, { + target: { value: newProjectName }, + }) + + const confirmButton = + within(modal).getByText('Rename') + expect(confirmButton.disabled).to.be.false + fireEvent.click(confirmButton) + + await fetchMock.flush(true) + + expect( + renameProjectMock.called(`/project/${projectsData[1].id}/rename`) + ).to.be.true + + const table = await screen.findByRole('table') + within(table).getByText(newProjectName) + expect(within(table).queryByText(oldName)).to.be.null + + const allCheckboxes = await within( + table + ).findAllByRole('checkbox') + const allCheckboxesChecked = allCheckboxes.filter(c => c.checked) + expect(allCheckboxesChecked.length).to.equal(0) + }) + + it('opens the copy modal, can copy the project, and view updated', async function () { + const tableRows = screen.getAllByRole('row') + const linkForProjectToCopy = within(tableRows[1]).getByRole('link') + const projectNameToCopy = linkForProjectToCopy.textContent || '' // needed for type checking + screen.getByText(projectNameToCopy) // make sure not just empty string + const copiedProjectName = `${projectNameToCopy} (Copy)` + const cloneProjectMock = fetchMock.post( + `express:/project/:id/clone`, + { + status: 200, + body: { + name: copiedProjectName, + lastUpdated: new Date(), + project_id: userId, + owner_ref: userId, + owner, + id: '6328e14abec0df019fce0be5', + lastUpdatedBy: owner, + accessLevel: 'owner', + source: 'owner', + trashed: false, + archived: false, + }, + } + ) + + await waitFor(() => { + const moreDropdown = + within(actionsToolbar).getByText('More') + fireEvent.click(moreDropdown) + }) + + const copyButton = + within(actionsToolbar).getByText('Make a copy') + fireEvent.click(copyButton) + + // confirm in modal + const copyConfirmButton = document.querySelector( + 'button[type="submit"]' + ) as HTMLElement + fireEvent.click(copyConfirmButton) + + await fetchMock.flush(true) + + expect( + cloneProjectMock.called(`/project/${projectsData[1].id}/clone`) + ).to.be.true + + expect(sendSpy).to.be.calledOnce + expect(sendSpy).calledWith('project-list-page-interaction') + + screen.getByText(copiedProjectName) + }) }) }) diff --git a/services/web/test/frontend/features/project-list/components/table/project-tools/project-tools.test.tsx b/services/web/test/frontend/features/project-list/components/table/project-tools/project-tools.test.tsx index f321f094c6..5fed1e962c 100644 --- a/services/web/test/frontend/features/project-list/components/table/project-tools/project-tools.test.tsx +++ b/services/web/test/frontend/features/project-list/components/table/project-tools/project-tools.test.tsx @@ -6,7 +6,7 @@ import { resetProjectListContextFetch, } from '../../../helpers/render-with-context' -describe('', function () { +describe('', function () { afterEach(function () { resetProjectListContextFetch() })