Selaa lähdekoodia

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
Alexandre Bourdin 4 vuotta sitten
vanhempi
sitoutus
9f4df9c0f4

+ 1 - 0
services/web/frontend/extracted-translations.json

@@ -601,6 +601,7 @@
   "tab_connecting": "",
   "tab_connecting": "",
   "tab_no_longer_connected": "",
   "tab_no_longer_connected": "",
   "tag_name_cannot_exceed_characters": "",
   "tag_name_cannot_exceed_characters": "",
+  "tag_name_is_already_used": "",
   "tags": "",
   "tags": "",
   "tags_slash_folders": "",
   "tags_slash_folders": "",
   "take_short_survey": "",
   "take_short_survey": "",

+ 5 - 1
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 { Tag } from '../../../../../../app/src/Features/Tags/types'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import useAsync from '../../../../shared/hooks/use-async'
 import useAsync from '../../../../shared/hooks/use-async'
+import { useProjectListContext } from '../../context/project-list-context'
 import { createTag } from '../../util/api'
 import { createTag } from '../../util/api'
 import { MAX_TAG_LENGTH } from '../../util/tag'
 import { MAX_TAG_LENGTH } from '../../util/tag'
 
 
@@ -20,6 +21,7 @@ export default function CreateTagModal({
   onCreate,
   onCreate,
   onClose,
   onClose,
 }: CreateTagModalProps) {
 }: CreateTagModalProps) {
+  const { tags } = useProjectListContext()
   const { t } = useTranslation()
   const { t } = useTranslation()
   const { isError, runAsync, status } = useAsync<Tag>()
   const { isError, runAsync, status } = useAsync<Tag>()
 
 
@@ -47,10 +49,12 @@ export default function CreateTagModal({
       setValidationError(
       setValidationError(
         t('tag_name_cannot_exceed_characters', { maxLength: MAX_TAG_LENGTH })
         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) {
     } else if (validationError) {
       setValidationError(undefined)
       setValidationError(undefined)
     }
     }
-  }, [tagName, t, validationError])
+  }, [tagName, tags, t, validationError])
 
 
   if (!show) {
   if (!show) {
     return null
     return null

+ 17 - 3
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 { Tag } from '../../../../../../app/src/Features/Tags/types'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import useAsync from '../../../../shared/hooks/use-async'
 import useAsync from '../../../../shared/hooks/use-async'
+import { useProjectListContext } from '../../context/project-list-context'
 import { renameTag } from '../../util/api'
 import { renameTag } from '../../util/api'
 import { MAX_TAG_LENGTH } from '../../util/tag'
 import { MAX_TAG_LENGTH } from '../../util/tag'
 
 
@@ -20,8 +21,9 @@ export default function RenameTagModal({
   onRename,
   onRename,
   onClose,
   onClose,
 }: RenameTagModalProps) {
 }: RenameTagModalProps) {
+  const { tags } = useProjectListContext()
   const { t } = useTranslation()
   const { t } = useTranslation()
-  const { isLoading, isError, runAsync } = useAsync()
+  const { isLoading, isError, runAsync, status } = useAsync()
 
 
   const [newTagName, setNewTagName] = useState<string>()
   const [newTagName, setNewTagName] = useState<string>()
   const [validationError, setValidationError] = useState<string>()
   const [validationError, setValidationError] = useState<string>()
@@ -52,10 +54,16 @@ export default function RenameTagModal({
       setValidationError(
       setValidationError(
         t('tag_name_cannot_exceed_characters', { maxLength: MAX_TAG_LENGTH })
         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) {
     } else if (validationError) {
       setValidationError(undefined)
       setValidationError(undefined)
     }
     }
-  }, [newTagName, t, validationError])
+  }, [newTagName, tags, tag?.name, t, validationError])
 
 
   if (!tag) {
   if (!tag) {
     return null
     return null
@@ -100,7 +108,13 @@ export default function RenameTagModal({
         <Button
         <Button
           onClick={() => runRenameTag(tag._id)}
           onClick={() => runRenameTag(tag._id)}
           bsStyle="primary"
           bsStyle="primary"
-          disabled={isLoading || !newTagName?.length || !!validationError}
+          disabled={
+            isLoading ||
+            status === 'pending' ||
+            newTagName === tag?.name ||
+            !newTagName?.length ||
+            !!validationError
+          }
         >
         >
           {isLoading ? <>{t('renaming')} &hellip;</> : t('rename')}
           {isLoading ? <>{t('renaming')} &hellip;</> : t('rename')}
         </Button>
         </Button>

+ 1 - 2
services/web/locales/en.json

@@ -1885,9 +1885,8 @@
   "create_first_project": "Create First Project",
   "create_first_project": "Create First Project",
   "you_dont_have_any_repositories": "You don’t have any repositories",
   "you_dont_have_any_repositories": "You don’t have any repositories",
   "tag_name_cannot_exceed_characters": "Tag name cannot exceed __maxLength__ characters",
   "tag_name_cannot_exceed_characters": "Tag name cannot exceed __maxLength__ characters",
+  "tag_name_is_already_used": "Tag \"__tagName__\" already exists",
   "save_changes": "Save changes",
   "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_already_participating": "You are enrolled in Labs",
   "labs_program_not_participating": "You are not 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.",
   "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.",

+ 62 - 0
services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx

@@ -104,6 +104,34 @@ describe('<TagsList />', function () {
       expect(createButton.hasAttribute('disabled')).to.be.true
       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 () {
     it('filling the input and clicking Create sends a request', async function () {
       const modal = screen.getAllByRole('dialog', { hidden: false })[0]
       const modal = screen.getAllByRole('dialog', { hidden: false })[0]
       const input = within(modal).getByRole('textbox')
       const input = within(modal).getByRole('textbox')
@@ -157,6 +185,40 @@ describe('<TagsList />', function () {
       expect(renameButton.hasAttribute('disabled')).to.be.true
       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 () {
     it('filling the input and clicking Rename sends a request', async function () {
       const modal = screen.getAllByRole('dialog', { hidden: false })[0]
       const modal = screen.getAllByRole('dialog', { hidden: false })[0]
       const input = within(modal).getByRole('textbox')
       const input = within(modal).getByRole('textbox')