From f36c87b301a741e0a9922115f6992d58c8dbd64c Mon Sep 17 00:00:00 2001
From: Domagoj Kriskovic <dom.kriskovic@overleaf.com>
Date: Wed, 2 Apr 2025 12:27:11 +0200
Subject: [PATCH] Update share modal dropdown with a description for reviewers
 in a free project (#24571)

* Update collaborator select dropdown with a description for reviewers in free project

* Update share-project-modal test

* Fix saas-e2e tests

* fix server pro tests

* fix cypress multiple inputs selection

* fix testid case

GitOrigin-RevId: 5369828334596d80988aba168385f0a51eea998f
---
 .../test/create-and-compile-project.spec.ts   |  4 +-
 server-ce/test/git-bridge.spec.ts             |  4 +-
 server-ce/test/helpers/project.ts             | 14 +++--
 server-ce/test/project-sharing.spec.ts        |  6 +-
 .../web/frontend/extracted-translations.json  |  5 +-
 .../components/add-collaborators.jsx          | 58 +++++++++++++------
 .../components/member-privileges.jsx          |  6 +-
 .../components/select-collaborators.jsx       |  1 +
 .../frontend/js/shared/components/select.tsx  |  4 ++
 .../bootstrap-5/pages/editor/share.scss       | 11 ++++
 services/web/locales/en.json                  |  5 +-
 .../components/share-project-modal.test.jsx   | 36 ++++++++----
 12 files changed, 103 insertions(+), 51 deletions(-)

diff --git a/server-ce/test/create-and-compile-project.spec.ts b/server-ce/test/create-and-compile-project.spec.ts
index afee2072b5..20f8f0dd6d 100644
--- a/server-ce/test/create-and-compile-project.spec.ts
+++ b/server-ce/test/create-and-compile-project.spec.ts
@@ -95,7 +95,9 @@ describe('Project creation and compilation', function () {
 
     cy.findByText('Share').click()
     cy.findByRole('dialog').within(() => {
-      cy.get('input').type('collaborator@example.com,')
+      cy.findByTestId('collaborator-email-input').type(
+        'collaborator@example.com,'
+      )
       cy.findByText('Invite').click({ force: true })
       cy.findByText('Invite not yet accepted.')
     })
diff --git a/server-ce/test/git-bridge.spec.ts b/server-ce/test/git-bridge.spec.ts
index 010b8ccf74..071091bdfd 100644
--- a/server-ce/test/git-bridge.spec.ts
+++ b/server-ce/test/git-bridge.spec.ts
@@ -136,7 +136,7 @@ describe('git-bridge', function () {
         shareProjectByEmailAndAcceptInviteViaDash(
           projectName,
           'collaborator-rw@example.com',
-          'Can edit'
+          'Editor'
         )
         maybeClearAllTokens()
         openProjectByName(projectName)
@@ -147,7 +147,7 @@ describe('git-bridge', function () {
         shareProjectByEmailAndAcceptInviteViaDash(
           projectName,
           'collaborator-ro@example.com',
-          'Can view'
+          'Viewer'
         )
         maybeClearAllTokens()
         openProjectByName(projectName)
diff --git a/server-ce/test/helpers/project.ts b/server-ce/test/helpers/project.ts
index 3b43e3d959..662327d6f2 100644
--- a/server-ce/test/helpers/project.ts
+++ b/server-ce/test/helpers/project.ts
@@ -100,7 +100,7 @@ export function openProjectViaInviteNotification(projectName: string) {
 function shareProjectByEmail(
   projectName: string,
   email: string,
-  level: 'Can view' | 'Can edit'
+  level: 'Viewer' | 'Editor'
 ) {
   openProjectByName(projectName)
   cy.findByText('Share').click()
@@ -108,7 +108,13 @@ function shareProjectByEmail(
     cy.findByLabelText('Add people', { selector: 'input' }).type(`${email},`)
     cy.findByLabelText('Add people', { selector: 'input' })
       .parents('form')
-      .within(() => cy.findByText('Can edit').parent().select(level))
+      .within(() => {
+        cy.findByTestId('add-collaborator-select')
+          .click()
+          .then(() => {
+            cy.findByText(level).click()
+          })
+      })
     cy.findByText('Invite').click({ force: true })
     cy.findByText('Invite not yet accepted.')
   })
@@ -117,7 +123,7 @@ function shareProjectByEmail(
 export function shareProjectByEmailAndAcceptInviteViaDash(
   projectName: string,
   email: string,
-  level: 'Can view' | 'Can edit'
+  level: 'Viewer' | 'Editor'
 ) {
   shareProjectByEmail(projectName, email, level)
 
@@ -128,7 +134,7 @@ export function shareProjectByEmailAndAcceptInviteViaDash(
 export function shareProjectByEmailAndAcceptInviteViaEmail(
   projectName: string,
   email: string,
-  level: 'Can view' | 'Can edit'
+  level: 'Viewer' | 'Editor'
 ) {
   shareProjectByEmail(projectName, email, level)
 
diff --git a/server-ce/test/project-sharing.spec.ts b/server-ce/test/project-sharing.spec.ts
index 44887f2fd2..e26439264b 100644
--- a/server-ce/test/project-sharing.spec.ts
+++ b/server-ce/test/project-sharing.spec.ts
@@ -154,7 +154,7 @@ describe('Project Sharing', function () {
 
     beforeEach(function () {
       login('user@example.com')
-      shareProjectByEmailAndAcceptInviteViaEmail(projectName, email, 'Can view')
+      shareProjectByEmailAndAcceptInviteViaEmail(projectName, email, 'Viewer')
     })
 
     it('should grant the collaborator read access', () => {
@@ -169,7 +169,7 @@ describe('Project Sharing', function () {
 
     beforeWithReRunOnTestRetry(function () {
       login('user@example.com')
-      shareProjectByEmailAndAcceptInviteViaDash(projectName, email, 'Can view')
+      shareProjectByEmailAndAcceptInviteViaDash(projectName, email, 'Viewer')
     })
 
     it('should grant the collaborator read access', () => {
@@ -186,7 +186,7 @@ describe('Project Sharing', function () {
 
     beforeWithReRunOnTestRetry(function () {
       login('user@example.com')
-      shareProjectByEmailAndAcceptInviteViaDash(projectName, email, 'Can edit')
+      shareProjectByEmailAndAcceptInviteViaDash(projectName, email, 'Editor')
     })
 
     it('should grant the collaborator write access', () => {
diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json
index 058e8e77bf..12f86f93eb 100644
--- a/services/web/frontend/extracted-translations.json
+++ b/services/web/frontend/extracted-translations.json
@@ -183,13 +183,10 @@
   "blog": "",
   "browser": "",
   "by_subscribing_you_agree_to_our_terms_of_service": "",
-  "can_edit": "",
   "can_edit_content": "",
   "can_link_institution_email_acct_to_institution_acct": "",
   "can_link_your_institution_acct_2": "",
   "can_now_relink_dropbox": "",
-  "can_review": "",
-  "can_view": "",
   "can_view_content": "",
   "cancel": "",
   "cancel_add_on": "",
@@ -268,6 +265,7 @@
   "column_width_is_x_click_to_resize": "",
   "comment": "",
   "comment_only": "",
+  "comment_only_upgrade_for_track_changes": "",
   "common": "",
   "common_causes_of_compile_timeouts_include": "",
   "commons_plan_tooltip": "",
@@ -1273,7 +1271,6 @@
   "read_lines_from_path": "",
   "read_more": "",
   "read_more_about_free_compile_timeouts_servers": "",
-  "read_only": "",
   "read_only_token": "",
   "read_write_token": "",
   "ready_to_join_x": "",
diff --git a/services/web/frontend/js/features/share-project-modal/components/add-collaborators.jsx b/services/web/frontend/js/features/share-project-modal/components/add-collaborators.jsx
index 9e86d5dcac..aa5948cb6f 100644
--- a/services/web/frontend/js/features/share-project-modal/components/add-collaborators.jsx
+++ b/services/web/frontend/js/features/share-project-modal/components/add-collaborators.jsx
@@ -12,7 +12,7 @@ import ClickableElementEnhancer from '@/shared/components/clickable-element-enha
 import PropTypes from 'prop-types'
 import OLForm from '@/features/ui/components/ol/ol-form'
 import OLFormGroup from '@/features/ui/components/ol/ol-form-group'
-import OLFormSelect from '@/features/ui/components/ol/ol-form-select'
+import { Select } from '@/shared/components/select'
 import OLButton from '@/features/ui/components/ol/ol-button'
 import getMeta from '@/utils/meta'
 
@@ -27,7 +27,7 @@ export default function AddCollaborators({ readOnly }) {
 
   const { updateProject, setInFlight, setError } = useShareProjectContext()
 
-  const { _id: projectId, members, invites } = useProjectContext()
+  const { _id: projectId, members, invites, features } = useProjectContext()
 
   const currentMemberEmails = useMemo(
     () => (members || []).map(member => member.email).sort(),
@@ -149,6 +149,32 @@ export default function AddCollaborators({ readOnly }) {
     updateProject,
   ])
 
+  const privilegeOptions = useMemo(() => {
+    const options = [
+      {
+        key: 'readAndWrite',
+        label: t('editor'),
+      },
+    ]
+
+    if (getMeta('ol-isReviewerRoleEnabled')) {
+      options.push({
+        key: 'review',
+        label: t('reviewer'),
+        description: !features.trackChanges
+          ? t('comment_only_upgrade_for_track_changes')
+          : null,
+      })
+    }
+
+    options.push({
+      key: 'readOnly',
+      label: t('viewer'),
+    })
+
+    return options
+  }, [features.trackChanges, t])
+
   return (
     <OLForm className="add-collabs">
       <OLFormGroup>
@@ -161,23 +187,19 @@ export default function AddCollaborators({ readOnly }) {
       </OLFormGroup>
 
       <OLFormGroup>
-        <div className="pull-right">
-          <OLFormSelect
-            className="privileges"
-            value={privileges}
-            onChange={event => setPrivileges(event.target.value)}
-          >
-            <option disabled={readOnly} value="readAndWrite">
-              {t('can_edit')}
-            </option>
-            {getMeta('ol-isReviewerRoleEnabled') && (
-              <option disabled={readOnly} value="review">
-                {t('can_review')}
-              </option>
+        <div className="pull-right add-collaborator-controls">
+          <Select
+            dataTestId="add-collaborator-select"
+            items={privilegeOptions}
+            itemToKey={item => item.key}
+            itemToString={item => item.label}
+            itemToSubtitle={item => item.description || ''}
+            itemToDisabled={item => readOnly && item.key === 'readAndWrite'}
+            selected={privilegeOptions.find(
+              option => option.key === privileges
             )}
-            <option value="readOnly">{t('can_view')}</option>
-          </OLFormSelect>
-          <span>&nbsp;&nbsp;</span>
+            onSelectedItemChanged={item => setPrivileges(item.key)}
+          />
           <ClickableElementEnhancer
             as={OLButton}
             onClick={handleSubmit}
diff --git a/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx b/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx
index a968bfc5fd..bae354858a 100644
--- a/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx
+++ b/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx
@@ -6,13 +6,13 @@ export default function MemberPrivileges({ privileges }) {
 
   switch (privileges) {
     case 'readAndWrite':
-      return t('can_edit')
+      return t('editor')
 
     case 'readOnly':
-      return t('read_only')
+      return t('viewer')
 
     case 'review':
-      return t('can_review')
+      return t('reviewer')
 
     default:
       return null
diff --git a/services/web/frontend/js/features/share-project-modal/components/select-collaborators.jsx b/services/web/frontend/js/features/share-project-modal/components/select-collaborators.jsx
index 05544b440f..ffaadb6b01 100644
--- a/services/web/frontend/js/features/share-project-modal/components/select-collaborators.jsx
+++ b/services/web/frontend/js/features/share-project-modal/components/select-collaborators.jsx
@@ -188,6 +188,7 @@ export default function SelectCollaborators({
           ))}
 
           <input
+            data-testid="collaborator-email-input"
             {...getInputProps(
               getDropdownProps({
                 className: classnames('input', {
diff --git a/services/web/frontend/js/shared/components/select.tsx b/services/web/frontend/js/shared/components/select.tsx
index bad73d19ce..66b360480f 100644
--- a/services/web/frontend/js/shared/components/select.tsx
+++ b/services/web/frontend/js/shared/components/select.tsx
@@ -52,6 +52,8 @@ export type SelectProps<T> = {
   loading?: boolean
   // Show a checkmark next to the selected item
   selectedIcon?: boolean
+  // testId for the input element
+  dataTestId?: string
 }
 
 export const Select = <T,>({
@@ -70,6 +72,7 @@ export const Select = <T,>({
   optionalLabel = false,
   loading = false,
   selectedIcon = false,
+  dataTestId,
 }: SelectProps<T>) => {
   const [selectedItem, setSelectedItem] = useState<T | undefined | null>(
     defaultItem
@@ -247,6 +250,7 @@ export const Select = <T,>({
             </Form.Label>
           ) : null}
           <FormControl
+            data-testid={dataTestId}
             {...getToggleButtonProps({
               disabled,
               onKeyDown,
diff --git a/services/web/frontend/stylesheets/bootstrap-5/pages/editor/share.scss b/services/web/frontend/stylesheets/bootstrap-5/pages/editor/share.scss
index 25c1f443d0..a7ce16f739 100644
--- a/services/web/frontend/stylesheets/bootstrap-5/pages/editor/share.scss
+++ b/services/web/frontend/stylesheets/bootstrap-5/pages/editor/share.scss
@@ -195,4 +195,15 @@
       max-width: 175px;
     }
   }
+
+  .add-collaborator-controls {
+    display: flex;
+    align-items: center;
+    justify-content: flex-end;
+    gap: var(--spacing-03);
+
+    .select-wrapper {
+      max-width: 130px;
+    }
+  }
 }
diff --git a/services/web/locales/en.json b/services/web/locales/en.json
index 1e5a89c93b..eac5332bf1 100644
--- a/services/web/locales/en.json
+++ b/services/web/locales/en.json
@@ -244,15 +244,12 @@
   "by_joining_labs": "By joining Labs, you agree to receive occasional emails and updates from Overleaf—for example, to request your feedback. You also agree to our <0>terms of service</0> and <1>privacy notice</1>.",
   "by_registering_you_agree_to_our_terms_of_service": "By registering, you agree to our <0>terms of service</0> and <1>privacy notice</1>.",
   "by_subscribing_you_agree_to_our_terms_of_service": "By subscribing, you agree to our <0>terms of service</0>.",
-  "can_edit": "Can edit",
   "can_edit_content": "Can edit content",
   "can_link_institution_email_acct_to_institution_acct": "You can now link your <b>__email__</b> <b>__appName__</b> account to your <b>__institutionName__</b> institutional account.",
   "can_link_institution_email_by_clicking": "You can link your <b>__email__</b> <b>__appName__</b> account to your  <b>__institutionName__</b> account by clicking <b>__clickText__</b>.",
   "can_link_institution_email_to_login": "You can link your <b>__email__</b> <b>__appName__</b> account to your <b>__institutionName__</b> account, which will allow you to log in to <b>__appName__</b> through your institution and will reconfirm your institutional email address.",
   "can_link_your_institution_acct_2": "You can now <0>link</0> your <0>__appName__</0> account to your <0>__institutionName__</0> institutional account.",
   "can_now_relink_dropbox": "You can now <0>relink your Dropbox account</0>.",
-  "can_review": "Can review",
-  "can_view": "Can view",
   "can_view_content": "Can view content",
   "cancel": "Cancel",
   "cancel_add_on": "Cancel add-on",
@@ -349,6 +346,7 @@
   "column_width_is_x_click_to_resize": "Column width is __width__. Click to resize",
   "comment": "Comment",
   "comment_only": "Comment only",
+  "comment_only_upgrade_for_track_changes": "Comment only. Upgrade for track changes.",
   "common": "Common",
   "common_causes_of_compile_timeouts_include": "Common causes of compile timeouts include",
   "commons_plan_tooltip": "You’re on the __plan__ plan because of your affiliation with __institution__. Click to find out how to make the most of your Overleaf premium features.",
@@ -1689,7 +1687,6 @@
   "read_lines_from_path": "Read lines from __path__",
   "read_more": "Read more",
   "read_more_about_free_compile_timeouts_servers": "Read more about changes to free compile timeouts and servers",
-  "read_only": "Read only",
   "read_only_token": "Read-Only Token",
   "read_write_token": "Read-Write Token",
   "ready_to_join_x": "You’re ready to join __inviterName__",
diff --git a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx
index 69f325a9e7..9e02710275 100644
--- a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx
+++ b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.jsx
@@ -638,8 +638,9 @@ describe('<ShareProjectModal/>', function () {
       },
     })
 
-    const privilegesElement = screen.getByDisplayValue('Can edit')
-    fireEvent.change(privilegesElement, { target: { value: 'readOnly' } })
+    const user = userEvent.setup()
+    await user.click(screen.getByTestId('add-collaborator-select'))
+    await user.click(screen.getByText('Viewer'))
 
     const submitButton = screen.getByRole('button', { name: 'Invite' })
     await userEvent.click(submitButton)
@@ -691,11 +692,16 @@ describe('<ShareProjectModal/>', function () {
     })
 
     await screen.findByText('Add more editors')
-    expect(screen.getByRole('option', { name: 'Can edit' }).disabled).to.be.true
-    expect(screen.getByRole('option', { name: 'Can review' }).disabled).to.be
-      .true
-    expect(screen.getByRole('option', { name: 'Can view' }).disabled).to.be
-      .false
+
+    const user = userEvent.setup()
+    await user.click(screen.getByTestId('add-collaborator-select'))
+    const editorOption = screen.getByText('Editor').closest('button')
+    const reviewerOption = screen.getByText('Reviewer').closest('button')
+    const viewerOption = screen.getByText('Viewer').closest('button')
+
+    expect(editorOption.classList.contains('disabled')).to.be.true
+    expect(reviewerOption.classList.contains('disabled')).to.be.false
+    expect(viewerOption.classList.contains('disabled')).to.be.false
 
     screen.getByText(
       /Upgrade to add more editors and access collaboration features like track changes and full project history/
@@ -722,11 +728,17 @@ describe('<ShareProjectModal/>', function () {
     })
 
     await screen.findByText('Add more editors')
-    expect(screen.getByRole('option', { name: 'Can edit' }).disabled).to.be.true
-    expect(screen.getByRole('option', { name: 'Can review' }).disabled).to.be
-      .true
-    expect(screen.getByRole('option', { name: 'Can view' }).disabled).to.be
-      .false
+
+    const user = userEvent.setup()
+    await user.click(screen.getByTestId('add-collaborator-select'))
+
+    const editorOption = screen.getByText('Editor').closest('button')
+    const reviewerOption = screen.getByText('Reviewer').closest('button')
+    const viewerOption = screen.getByText('Viewer').closest('button')
+
+    expect(editorOption.classList.contains('disabled')).to.be.true
+    expect(reviewerOption.classList.contains('disabled')).to.be.false
+    expect(viewerOption.classList.contains('disabled')).to.be.false
 
     screen.getByText(
       /Upgrade to add more editors and access collaboration features like track changes and full project history/