From 9f4df9c0f4af43fae3836e02bd96108533565c2f Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Mon, 26 Sep 2022 10:57:55 +0200 Subject: [PATCH] Merge pull request #9636 from overleaf/ab-prevent-create-rename-tag-existing-name [web] Prevent creating/renaming a tag to an existing name GitOrigin-RevId: 44bb35a4152238ce21fa6e0d4d211cc5b25481e8 --- .../web/frontend/extracted-translations.json | 1 + .../components/modals/create-tag-modal.tsx | 6 +- .../components/modals/rename-tag-modal.tsx | 20 +++++- services/web/locales/en.json | 3 +- .../components/sidebar/tags-list.test.tsx | 62 +++++++++++++++++++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index fe60b1cd83..c19c67fadb 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -601,6 +601,7 @@ "tab_connecting": "", "tab_no_longer_connected": "", "tag_name_cannot_exceed_characters": "", + "tag_name_is_already_used": "", "tags": "", "tags_slash_folders": "", "take_short_survey": "", diff --git a/services/web/frontend/js/features/project-list/components/modals/create-tag-modal.tsx b/services/web/frontend/js/features/project-list/components/modals/create-tag-modal.tsx index 3b71974366..f5336dbdd5 100644 --- a/services/web/frontend/js/features/project-list/components/modals/create-tag-modal.tsx +++ b/services/web/frontend/js/features/project-list/components/modals/create-tag-modal.tsx @@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next' import { Tag } from '../../../../../../app/src/Features/Tags/types' import AccessibleModal from '../../../../shared/components/accessible-modal' import useAsync from '../../../../shared/hooks/use-async' +import { useProjectListContext } from '../../context/project-list-context' import { createTag } from '../../util/api' import { MAX_TAG_LENGTH } from '../../util/tag' @@ -20,6 +21,7 @@ export default function CreateTagModal({ onCreate, onClose, }: CreateTagModalProps) { + const { tags } = useProjectListContext() const { t } = useTranslation() const { isError, runAsync, status } = useAsync() @@ -47,10 +49,12 @@ export default function CreateTagModal({ setValidationError( t('tag_name_cannot_exceed_characters', { maxLength: MAX_TAG_LENGTH }) ) + } else if (tagName && tags.find(tag => tag.name === tagName)) { + setValidationError(t('tag_name_is_already_used', { tagName })) } else if (validationError) { setValidationError(undefined) } - }, [tagName, t, validationError]) + }, [tagName, tags, t, validationError]) if (!show) { return null diff --git a/services/web/frontend/js/features/project-list/components/modals/rename-tag-modal.tsx b/services/web/frontend/js/features/project-list/components/modals/rename-tag-modal.tsx index 57f8c8ad10..86854ab3de 100644 --- a/services/web/frontend/js/features/project-list/components/modals/rename-tag-modal.tsx +++ b/services/web/frontend/js/features/project-list/components/modals/rename-tag-modal.tsx @@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next' import { Tag } from '../../../../../../app/src/Features/Tags/types' import AccessibleModal from '../../../../shared/components/accessible-modal' import useAsync from '../../../../shared/hooks/use-async' +import { useProjectListContext } from '../../context/project-list-context' import { renameTag } from '../../util/api' import { MAX_TAG_LENGTH } from '../../util/tag' @@ -20,8 +21,9 @@ export default function RenameTagModal({ onRename, onClose, }: RenameTagModalProps) { + const { tags } = useProjectListContext() const { t } = useTranslation() - const { isLoading, isError, runAsync } = useAsync() + const { isLoading, isError, runAsync, status } = useAsync() const [newTagName, setNewTagName] = useState() const [validationError, setValidationError] = useState() @@ -52,10 +54,16 @@ export default function RenameTagModal({ setValidationError( t('tag_name_cannot_exceed_characters', { maxLength: MAX_TAG_LENGTH }) ) + } else if ( + newTagName && + newTagName !== tag?.name && + tags.find(tag => tag.name === newTagName) + ) { + setValidationError(t('tag_name_is_already_used', { tagName: newTagName })) } else if (validationError) { setValidationError(undefined) } - }, [newTagName, t, validationError]) + }, [newTagName, tags, tag?.name, t, validationError]) if (!tag) { return null @@ -100,7 +108,13 @@ export default function RenameTagModal({ diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 34970659f4..9a4cf62a05 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1885,9 +1885,8 @@ "create_first_project": "Create First Project", "you_dont_have_any_repositories": "You don’t have any repositories", "tag_name_cannot_exceed_characters": "Tag name cannot exceed __maxLength__ characters", + "tag_name_is_already_used": "Tag \"__tagName__\" already exists", "save_changes": "Save changes", - "tag_name_cannot_exceed_characters": "Tag name cannot exceed __maxLength__ characters", - "overleaf_labs": "Overleaf Labs", "labs_program_already_participating": "You are enrolled in Labs", "labs_program_not_participating": "You are not enrolled in Labs", "labs_program_benefits": "__appName__ is always looking for new ways to help users work more quickly and effectively. By joining Overleaf Labs, you can participate in experiments that explore innovative ideas in the space of collaborative writing and publishing.", diff --git a/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx b/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx index 799d202510..c80e81adcb 100644 --- a/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx +++ b/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx @@ -104,6 +104,34 @@ describe('', function () { expect(createButton.hasAttribute('disabled')).to.be.true }) + it('Create button is disabled with error message when tag name is too long', async function () { + const modal = screen.getAllByRole('dialog', { hidden: false })[0] + const input = within(modal).getByRole('textbox') + fireEvent.change(input, { + target: { + value: 'This is a very very very very very very long tag name', + }, + }) + + const createButton = within(modal).getByRole('button', { name: 'Create' }) + expect(createButton.hasAttribute('disabled')).to.be.true + screen.getByText('Tag name cannot exceed 50 characters') + }) + + it('Create button is disabled with error message when tag name is already used', async function () { + const modal = screen.getAllByRole('dialog', { hidden: false })[0] + const input = within(modal).getByRole('textbox') + fireEvent.change(input, { + target: { + value: 'Tag 1', + }, + }) + + const createButton = within(modal).getByRole('button', { name: 'Create' }) + expect(createButton.hasAttribute('disabled')).to.be.true + screen.getByText('Tag "Tag 1" already exists') + }) + it('filling the input and clicking Create sends a request', async function () { const modal = screen.getAllByRole('dialog', { hidden: false })[0] const input = within(modal).getByRole('textbox') @@ -157,6 +185,40 @@ describe('', function () { expect(renameButton.hasAttribute('disabled')).to.be.true }) + it('Rename button is disabled with error message when tag name is too long', async function () { + const modal = screen.getAllByRole('dialog', { hidden: false })[0] + const input = within(modal).getByRole('textbox') + fireEvent.change(input, { + target: { + value: 'This is a very very very very very very long tag name', + }, + }) + + const createButton = within(modal).getByRole('button', { name: 'Rename' }) + expect(createButton.hasAttribute('disabled')).to.be.true + screen.getByText('Tag name cannot exceed 50 characters') + }) + + it('Rename button is disabled with no error message when tag name is unchanged', async function () { + const modal = screen.getAllByRole('dialog', { hidden: false })[0] + const createButton = within(modal).getByRole('button', { name: 'Rename' }) + expect(createButton.hasAttribute('disabled')).to.be.true + }) + + it('Rename button is disabled with error message when tag name is already used', async function () { + const modal = screen.getAllByRole('dialog', { hidden: false })[0] + const input = within(modal).getByRole('textbox') + fireEvent.change(input, { + target: { + value: 'Another tag', + }, + }) + + const createButton = within(modal).getByRole('button', { name: 'Rename' }) + expect(createButton.hasAttribute('disabled')).to.be.true + screen.getByText('Tag "Another tag" already exists') + }) + it('filling the input and clicking Rename sends a request', async function () { const modal = screen.getAllByRole('dialog', { hidden: false })[0] const input = within(modal).getByRole('textbox')