diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index 5ed1e022c6..5af414189f 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/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) { diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index 899d91287a..4f9433da9a 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/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, diff --git a/services/web/app/views/user/settings-react.pug b/services/web/app/views/user/settings-react.pug index 9270a7ee7d..2775dfb50c 100644 --- a/services/web/app/views/user/settings-react.pug +++ b/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) diff --git a/services/web/frontend/js/features/settings/components/account-info-section.tsx b/services/web/frontend/js/features/settings/components/account-info-section.tsx index ab5113eb92..b158e8d5f8 100644 --- a/services/web/frontend/js/features/settings/components/account-info-section.tsx +++ b/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() { {t('thanks_settings_updated')} ) : null} - {error ? ( + {isError ? ( {error.getUserFacingMessage()} diff --git a/services/web/frontend/js/features/settings/components/leave-section.tsx b/services/web/frontend/js/features/settings/components/leave-section.tsx index ceaf270b58..99e5c538bb 100644 --- a/services/web/frontend/js/features/settings/components/leave-section.tsx +++ b/services/web/frontend/js/features/settings/components/leave-section.tsx @@ -18,7 +18,7 @@ function LeaveSection() { return ( <> {t('need_to_leave')}{' '} - diff --git a/services/web/frontend/js/features/settings/components/linking/integration-widget.tsx b/services/web/frontend/js/features/settings/components/linking/integration-widget.tsx index 7e9bbade37..cd46c3b866 100644 --- a/services/web/frontend/js/features/settings/components/linking/integration-widget.tsx +++ b/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 ( @@ -146,12 +151,13 @@ function UnlinkConfirmationModal({ - - - {t('unlink')} - +
+ + + +
) diff --git a/services/web/frontend/js/features/settings/components/password-section.tsx b/services/web/frontend/js/features/settings/components/password-section.tsx index 779a700b92..31dbe432a4 100644 --- a/services/web/frontend/js/features/settings/components/password-section.tsx +++ b/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() { {data.message.text} ) : null} - {error ? ( + {isError ? ( {error.getUserFacingMessage()} diff --git a/services/web/frontend/stories/settings/account-info.stories.js b/services/web/frontend/stories/settings/account-info.stories.js index a2c8f22362..1f041f892d 100644 --- a/services/web/frontend/stories/settings/account-info.stories.js +++ b/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 + return ( + + + + ) } 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 + return ( + + + + ) } export const NoEmailInput = args => { @@ -24,14 +33,22 @@ export const NoEmailInput = args => { }) useFetchMock(defaultSetupMocks) - return + return ( + + + + ) } export const Error = args => { setDefaultMeta() useFetchMock(fetchMock => fetchMock.post(/\/user\/settings/, 500)) - return + return ( + + + + ) } export default { diff --git a/services/web/frontend/stories/settings/helpers/account-info.js b/services/web/frontend/stories/settings/helpers/account-info.js index 3774bb7b47..cebd81e7bc 100644 --- a/services/web/frontend/stories/settings/helpers/account-info.js +++ b/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, }) diff --git a/services/web/frontend/stories/settings/page.stories.js b/services/web/frontend/stories/settings/page.stories.js index 5e07a7e71f..a37073405e 100644 --- a/services/web/frontend/stories/settings/page.stories.js +++ b/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 ( diff --git a/services/web/frontend/stylesheets/components/buttons.less b/services/web/frontend/stylesheets/components/buttons.less index 1ae9857dcd..1dbaacd034 100755 --- a/services/web/frontend/stylesheets/components/buttons.less +++ b/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 { diff --git a/services/web/locales/en.json b/services/web/locales/en.json index d9a71d44b9..29a6f940a7 100644 --- a/services/web/locales/en.json +++ b/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", diff --git a/services/web/test/frontend/features/settings/components/account-info-section.test.tsx b/services/web/test/frontend/features/settings/components/account-info-section.test.tsx index 439410d37a..eb3a2be00a 100644 --- a/services/web/test/frontend/features/settings/components/account-info-section.test.tsx +++ b/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(, { + wrapper: ({ children }) => {children}, + }) +} describe('', 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('', function () { it('submits all inputs', async function () { const updateMock = fetchMock.post('/user/settings', 200) - render() + renderSectionWithUserProvider() fireEvent.change(screen.getByLabelText('Email'), { target: { value: 'john@watson.co.uk' }, @@ -45,14 +54,14 @@ describe('', 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() + renderSectionWithUserProvider() fireEvent.change(screen.getByLabelText('Email'), { target: { value: 'john' }, @@ -73,7 +82,7 @@ describe('', function () { '/user/settings', new Promise(resolve => (finishUpdateCall = resolve)) ) - render() + renderSectionWithUserProvider() fireEvent.click( screen.getByRole('button', { @@ -91,7 +100,7 @@ describe('', function () { it('shows server error', async function () { fetchMock.post('/user/settings', 500) - render() + renderSectionWithUserProvider() fireEvent.click( screen.getByRole('button', { @@ -105,7 +114,7 @@ describe('', function () { it('shows invalid error', async function () { fetchMock.post('/user/settings', 400) - render() + renderSectionWithUserProvider() fireEvent.click( screen.getByRole('button', { @@ -124,7 +133,7 @@ describe('', function () { message: 'This email is already registered', }, }) - render() + renderSectionWithUserProvider() fireEvent.click( screen.getByRole('button', { @@ -140,7 +149,7 @@ describe('', function () { }) const updateMock = fetchMock.post('/user/settings', 200) - render() + renderSectionWithUserProvider() expect(screen.queryByLabelText('Email')).to.not.exist fireEvent.click( @@ -149,8 +158,8 @@ describe('', 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('', function () { ) const updateMock = fetchMock.post('/user/settings', 200) - render() + 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('', 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('', function () { window.metaAttributesCache.set('ol-shouldAllowEditingDetails', false) const updateMock = fetchMock.post('/user/settings', 200) - render() + 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 diff --git a/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx b/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx index faf9cdaae6..6e6843f289 100644 --- a/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx +++ b/services/web/test/frontend/features/settings/components/linking/integration-widget.test.tsx @@ -79,9 +79,8 @@ describe('', 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 () { diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index aebbd9bd5e..52a813b906 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/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 },