Merge pull request #7725 from overleaf/ta-settings-fixes

[SettingsPage] Misc Fixes

GitOrigin-RevId: 56f58d2bb5830f7e0584a83c98efc9989ae2bd42

services/web/app/src/Features/User/UserController.js

@@ -73,7 +73,11 @@ async function changePassword(req, res, next) {
     req.body.currentPassword
   )
   if (!user) {
-    return HttpErrorHandler.badRequest(req, res, 'Your old password is wrong')
+    return HttpErrorHandler.badRequest(
+      req,
+      res,
+      req.i18n.translate('password_change_old_password_wrong')
+    )
   }
 
   if (req.body.newPassword1 !== req.body.newPassword2) {

services/web/app/src/Features/User/UserPagesController.js

@@ -82,8 +82,6 @@ async function settingsPage(req, res) {
         },
       },
       hasPassword: !!user.hashedPassword,
-      firstName: user.first_name,
-      lastName: user.last_name,
       shouldAllowEditingDetails,
       oauthProviders: UserPagesController._translateProviderDescriptions(
         oauthProviders,

services/web/app/views/user/settings-react.pug

@@ -1,4 +1,4 @@
-extends ../layout
+extends ../layout-marketing
 
 block entrypointVar
 	- entrypoint = 'pages/user/settings'
@@ -18,8 +18,6 @@ block append meta
 	meta(name="ol-thirdPartyIds", data-type="json", content=thirdPartyIds || {})
 	meta(name="ol-passwordStrengthOptions", data-type="json", content=settings.passwordStrengthOptions || {})
 	meta(name="ol-isExternalAuthenticationSystemUsed" data-type="boolean" content=externalAuthenticationSystemUsed())
-	meta(name="ol-firstName" content=firstName)
-	meta(name="ol-lastName" content=lastName)
 	meta(name="ol-user" data-type="json" content=user)
 	meta(name="ol-dropbox" data-type="json" content=dropbox)
 	meta(name="ol-github" data-type="json" content=github)

services/web/frontend/js/features/settings/components/account-info-section.tsx

@@ -11,6 +11,7 @@ import { postJSON } from '../../../infrastructure/fetch-json'
 import getMeta from '../../../utils/meta'
 import { ExposedSettings } from '../../../../../types/exposed-settings'
 import useAsync from '../../../shared/hooks/use-async'
+import { useUserContext } from '../../../shared/context/user-context'
 
 function AccountInfoSection() {
   const { t } = useTranslation()
@@ -23,15 +24,16 @@ function AccountInfoSection() {
   const shouldAllowEditingDetails = getMeta(
     'ol-shouldAllowEditingDetails'
   ) as boolean
+  const {
+    first_name: initialFirstName,
+    last_name: initialLastName,
+    email: initialEmail,
+  } = useUserContext()
 
-  const [email, setEmail] = useState(() => getMeta('ol-usersEmail') as string)
-  const [firstName, setFirstName] = useState(
-    () => getMeta('ol-firstName') as string
-  )
-  const [lastName, setLastName] = useState(
-    () => getMeta('ol-lastName') as string
-  )
-  const { isLoading, error, isSuccess, runAsync } = useAsync()
+  const [email, setEmail] = useState(initialEmail)
+  const [firstName, setFirstName] = useState(initialFirstName)
+  const [lastName, setLastName] = useState(initialLastName)
+  const { isLoading, isSuccess, isError, error, runAsync } = useAsync()
   const [isFormValid, setIsFormValid] = useState(true)
 
   const handleEmailChange = event => {
@@ -60,8 +62,8 @@ function AccountInfoSection() {
       postJSON('/user/settings', {
         body: {
           email: canUpdateEmail ? email : undefined,
-          firstName: canUpdateNames ? firstName : undefined,
-          lastName: canUpdateNames ? lastName : undefined,
+          first_name: canUpdateNames ? firstName : undefined,
+          last_name: canUpdateNames ? lastName : undefined,
         },
       })
     ).catch(() => {})
@@ -105,7 +107,7 @@ function AccountInfoSection() {
             <Alert bsStyle="success">{t('thanks_settings_updated')}</Alert>
           </FormGroup>
         ) : null}
-        {error ? (
+        {isError ? (
           <FormGroup>
             <Alert bsStyle="danger">{error.getUserFacingMessage()}</Alert>
           </FormGroup>

services/web/frontend/js/features/settings/components/leave-section.tsx

@@ -18,7 +18,7 @@ function LeaveSection() {
   return (
     <>
       {t('need_to_leave')}{' '}
-      <button className="btn btn-inline-link" onClick={handleOpen}>
+      <button className="btn btn-inline-link btn-danger" onClick={handleOpen}>
         {t('delete_your_account')}
       </button>
       <LeaveModal isOpen={isModalOpen} handleClose={handleClose} />

services/web/frontend/js/features/settings/components/linking/integration-widget.tsx

@@ -1,7 +1,8 @@
 import { useCallback, useState, ReactNode } from 'react'
 import { useTranslation } from 'react-i18next'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
-import { Modal } from 'react-bootstrap'
+import { Button, Modal } from 'react-bootstrap'
+import getMeta from '../../../../utils/meta'
 
 type IntegrationLinkingWidgetProps = {
   logo: ReactNode
@@ -135,6 +136,10 @@ function UnlinkConfirmationModal({
 }: UnlinkConfirmModalProps) {
   const { t } = useTranslation()
 
+  const handleCancel = event => {
+    event.preventDefault()
+    handleHide()
+  }
   return (
     <AccessibleModal show={show} onHide={handleHide}>
       <Modal.Header closeButton>
@@ -146,12 +151,13 @@ function UnlinkConfirmationModal({
       </Modal.Body>
 
       <Modal.Footer>
-        <button className="btn btn-default" onClick={handleHide}>
-          {t('cancel')}
-        </button>
-        <a href={unlinkPath} className="btn btn-danger text-capitalize">
-          {t('unlink')}
-        </a>
+        <form action={unlinkPath} method="POST" className="form-inline">
+          <input type="hidden" name="_csrf" value={getMeta('ol-csrfToken')} />
+          <Button onClick={handleCancel}>{t('cancel')}</Button>
+          <Button type="submit" bsStyle="danger">
+            {t('unlink')}
+          </Button>
+        </form>
       </Modal.Footer>
     </AccessibleModal>
   )

services/web/frontend/js/features/settings/components/password-section.tsx

@@ -58,7 +58,7 @@ function PasswordForm() {
   const [currentPassword, setCurrentPassword] = useState('')
   const [newPassword1, setNewPassword1] = useState('')
   const [newPassword2, setNewPassword2] = useState('')
-  const { isLoading, error, isSuccess, data, runAsync } = useAsync()
+  const { isLoading, isSuccess, isError, data, error, runAsync } = useAsync()
   const [isNewPasswordValid, setIsNewPasswordValid] = useState(false)
   const [isFormValid, setIsFormValid] = useState(false)
 
@@ -126,7 +126,7 @@ function PasswordForm() {
           <Alert bsStyle="success">{data.message.text}</Alert>
         </FormGroup>
       ) : null}
-      {error ? (
+      {isError ? (
         <FormGroup>
           <Alert bsStyle="danger">{error.getUserFacingMessage()}</Alert>
         </FormGroup>

services/web/frontend/stories/settings/account-info.stories.js

@@ -1,12 +1,17 @@
 import useFetchMock from '../hooks/use-fetch-mock'
 import AccountInfoSection from '../../js/features/settings/components/account-info-section'
 import { setDefaultMeta, defaultSetupMocks } from './helpers/account-info'
+import { UserProvider } from '../../js/shared/context/user-context'
 
 export const Success = args => {
   setDefaultMeta()
   useFetchMock(defaultSetupMocks)
 
-  return <AccountInfoSection {...args} />
+  return (
+    <UserProvider>
+      <AccountInfoSection {...args} />
+    </UserProvider>
+  )
 }
 
 export const ReadOnly = args => {
@@ -14,7 +19,11 @@ export const ReadOnly = args => {
   window.metaAttributesCache.set('ol-isExternalAuthenticationSystemUsed', true)
   window.metaAttributesCache.set('ol-shouldAllowEditingDetails', false)
 
-  return <AccountInfoSection {...args} />
+  return (
+    <UserProvider>
+      <AccountInfoSection {...args} />
+    </UserProvider>
+  )
 }
 
 export const NoEmailInput = args => {
@@ -24,14 +33,22 @@ export const NoEmailInput = args => {
   })
   useFetchMock(defaultSetupMocks)
 
-  return <AccountInfoSection {...args} />
+  return (
+    <UserProvider>
+      <AccountInfoSection {...args} />
+    </UserProvider>
+  )
 }
 
 export const Error = args => {
   setDefaultMeta()
   useFetchMock(fetchMock => fetchMock.post(/\/user\/settings/, 500))
 
-  return <AccountInfoSection {...args} />
+  return (
+    <UserProvider>
+      <AccountInfoSection {...args} />
+    </UserProvider>
+  )
 }
 
 export default {

services/web/frontend/stories/settings/helpers/account-info.js

@@ -8,9 +8,11 @@ export function defaultSetupMocks(fetchMock) {
 
 export function setDefaultMeta() {
   window.metaAttributesCache = window.metaAttributesCache || new Map()
-  window.metaAttributesCache.set('ol-usersEmail', 'sherlock@holmes.co.uk')
-  window.metaAttributesCache.set('ol-firstName', 'Sherlock')
-  window.metaAttributesCache.set('ol-lastName', 'Holmes')
+  window.metaAttributesCache.set('ol-user', {
+    email: 'sherlock@holmes.co.uk',
+    first_name: 'Sherlock',
+    last_name: 'Holmes',
+  })
   window.metaAttributesCache.set('ol-ExposedSettings', {
     hasAffiliationsFeature: false,
   })

services/web/frontend/stories/settings/page.stories.js

@@ -28,11 +28,13 @@ export const Root = args => {
   setDefaultPasswordMeta()
   setDefaultEmailsMeta()
   setDefaultLinkingMeta()
-  useFetchMock(defaultSetupLeaveMocks)
-  useFetchMock(defaultSetupAccountInfoMocks)
-  useFetchMock(defaultSetupPasswordMocks)
-  useFetchMock(defaultSetupEmailsMocks)
-  useFetchMock(defaultSetupLinkingMocks)
+  useFetchMock(fetchMock => {
+    defaultSetupLeaveMocks(fetchMock)
+    defaultSetupAccountInfoMocks(fetchMock)
+    defaultSetupPasswordMocks(fetchMock)
+    defaultSetupEmailsMocks(fetchMock)
+    defaultSetupLinkingMocks(fetchMock)
+  })
 
   return (
     <UserProvider>

services/web/frontend/stylesheets/components/buttons.less

@@ -147,6 +147,15 @@
       text-decoration: none;
     }
   }
+
+  &.btn-danger {
+    color: @ol-red;
+
+    &:hover,
+    &:focus {
+      color: @ol-dark-red;
+    }
+  }
 }
 
 .btn-inline-link {

services/web/locales/en.json

@@ -309,6 +309,7 @@
   "not_found_error_from_the_supplied_url": "The link to open this content on Overleaf pointed to a file that could not be found. If this keeps happening for links on a particular site, please report this to them.",
   "too_many_requests": "Too many requests were received in a short space of time. Please wait for a few moments and try again.",
   "password_change_passwords_do_not_match": "Passwords do not match",
+  "password_change_old_password_wrong": "Your old password is wrong",
   "github_for_link_shared_projects": "This project was accessed via link-sharing and won’t be synchronised with your GitHub unless you are invited via e-mail by the project owner.",
   "browsing_project_latest_for_pseudo_label": "Browsing your project’s current state",
   "history_label_project_current_state": "Current state",

services/web/test/frontend/features/settings/components/account-info-section.test.tsx

@@ -2,13 +2,22 @@ import { expect } from 'chai'
 import { fireEvent, screen, render } from '@testing-library/react'
 import fetchMock from 'fetch-mock'
 import AccountInfoSection from '../../../../../frontend/js/features/settings/components/account-info-section'
+import { UserProvider } from '../../../../../frontend/js/shared/context/user-context'
+
+function renderSectionWithUserProvider() {
+  render(<AccountInfoSection />, {
+    wrapper: ({ children }) => <UserProvider>{children}</UserProvider>,
+  })
+}
 
 describe('<AccountInfoSection />', function () {
   beforeEach(function () {
     window.metaAttributesCache = window.metaAttributesCache || new Map()
-    window.metaAttributesCache.set('ol-usersEmail', 'sherlock@holmes.co.uk')
-    window.metaAttributesCache.set('ol-firstName', 'Sherlock')
-    window.metaAttributesCache.set('ol-lastName', 'Holmes')
+    window.metaAttributesCache.set('ol-user', {
+      email: 'sherlock@holmes.co.uk',
+      first_name: 'Sherlock',
+      last_name: 'Holmes',
+    })
     window.metaAttributesCache.set('ol-ExposedSettings', {
       hasAffiliationsFeature: false,
     })
@@ -26,7 +35,7 @@ describe('<AccountInfoSection />', function () {
 
   it('submits all inputs', async function () {
     const updateMock = fetchMock.post('/user/settings', 200)
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.change(screen.getByLabelText('Email'), {
       target: { value: 'john@watson.co.uk' },
@@ -45,14 +54,14 @@ describe('<AccountInfoSection />', function () {
     expect(updateMock.called()).to.be.true
     expect(JSON.parse(updateMock.lastCall()[1].body)).to.deep.equal({
       email: 'john@watson.co.uk',
-      firstName: 'John',
-      lastName: 'Watson',
+      first_name: 'John',
+      last_name: 'Watson',
     })
   })
 
   it('disables button on invalid email', async function () {
     const updateMock = fetchMock.post('/user/settings', 200)
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.change(screen.getByLabelText('Email'), {
       target: { value: 'john' },
@@ -73,7 +82,7 @@ describe('<AccountInfoSection />', function () {
       '/user/settings',
       new Promise(resolve => (finishUpdateCall = resolve))
     )
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.click(
       screen.getByRole('button', {
@@ -91,7 +100,7 @@ describe('<AccountInfoSection />', function () {
 
   it('shows server error', async function () {
     fetchMock.post('/user/settings', 500)
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.click(
       screen.getByRole('button', {
@@ -105,7 +114,7 @@ describe('<AccountInfoSection />', function () {
 
   it('shows invalid error', async function () {
     fetchMock.post('/user/settings', 400)
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.click(
       screen.getByRole('button', {
@@ -124,7 +133,7 @@ describe('<AccountInfoSection />', function () {
         message: 'This email is already registered',
       },
     })
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
 
     fireEvent.click(
       screen.getByRole('button', {
@@ -140,7 +149,7 @@ describe('<AccountInfoSection />', function () {
     })
     const updateMock = fetchMock.post('/user/settings', 200)
 
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
     expect(screen.queryByLabelText('Email')).to.not.exist
 
     fireEvent.click(
@@ -149,8 +158,8 @@ describe('<AccountInfoSection />', function () {
       })
     )
     expect(JSON.parse(updateMock.lastCall()[1].body)).to.deep.equal({
-      firstName: 'Sherlock',
-      lastName: 'Holmes',
+      first_name: 'Sherlock',
+      last_name: 'Holmes',
     })
   })
 
@@ -161,7 +170,7 @@ describe('<AccountInfoSection />', function () {
     )
     const updateMock = fetchMock.post('/user/settings', 200)
 
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
     expect(screen.getByLabelText('Email').readOnly).to.be.true
     expect(screen.getByLabelText('First Name').readOnly).to.be.false
     expect(screen.getByLabelText('Last Name').readOnly).to.be.false
@@ -172,8 +181,8 @@ describe('<AccountInfoSection />', function () {
       })
     )
     expect(JSON.parse(updateMock.lastCall()[1].body)).to.deep.equal({
-      firstName: 'Sherlock',
-      lastName: 'Holmes',
+      first_name: 'Sherlock',
+      last_name: 'Holmes',
     })
   })
 
@@ -181,7 +190,7 @@ describe('<AccountInfoSection />', function () {
     window.metaAttributesCache.set('ol-shouldAllowEditingDetails', false)
     const updateMock = fetchMock.post('/user/settings', 200)
 
-    render(<AccountInfoSection />)
+    renderSectionWithUserProvider()
     expect(screen.getByLabelText('Email').readOnly).to.be.false
     expect(screen.getByLabelText('First Name').readOnly).to.be.true
     expect(screen.getByLabelText('Last Name').readOnly).to.be.true

services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx

@@ -79,9 +79,8 @@ describe('<IntegrationLinkingWidgetTest/>', function () {
       fireEvent.click(screen.getByRole('button', { name: 'Unlink' }))
       screen.getByText('confirm unlink')
       screen.getByText('you will be unlinked')
-      expect(
-        screen.getByRole('link', { name: 'Unlink' }).getAttribute('href')
-      ).to.equal('/unlink')
+      screen.getByRole('button', { name: 'Cancel' })
+      screen.getByRole('button', { name: 'Unlink' })
     })
 
     it('should cancel unlinking when clicking "cancel" in the confirmation modal', async function () {

services/web/test/unit/src/User/UserControllerTests.js

@@ -714,7 +714,7 @@ describe('UserController', function () {
           expect(this.HttpErrorHandler.badRequest).to.have.been.calledWith(
             this.req,
             this.res,
-            'Your old password is wrong'
+            'password_change_old_password_wrong'
           )
           this.AuthenticationManager.promises.authenticate.should.have.been.calledWith(
             { _id: this.user._id },