From 317d882235dd429a2b81142689192fffebc71e1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Alby?= Date: Wed, 18 May 2022 15:45:31 +0200 Subject: [PATCH] Merge pull request #7975 from overleaf/ta-settings-fixes-6 [SettingsPage] Small Fixes 6 GitOrigin-RevId: 19ad9a195a401909ac3dcc2be79d380cb61078da --- .../components/emails/actions/remove.tsx | 18 ++++--------- .../components/leavers-survey-alert.tsx | 15 ++++++++++- .../settings/context/user-email-context.tsx | 14 +++++----- .../stories/settings/helpers/emails.js | 5 ++++ .../settings/leavers-survey-alert.stories.tsx | 3 ++- .../emails/emails-section-actions.test.tsx | 14 +++------- .../components/leavers-survey-alert.test.tsx | 27 ++++++++++--------- 7 files changed, 50 insertions(+), 46 deletions(-) diff --git a/services/web/frontend/js/features/settings/components/emails/actions/remove.tsx b/services/web/frontend/js/features/settings/components/emails/actions/remove.tsx index 976e5b28db..53cf32ce51 100644 --- a/services/web/frontend/js/features/settings/components/emails/actions/remove.tsx +++ b/services/web/frontend/js/features/settings/components/emails/actions/remove.tsx @@ -8,6 +8,8 @@ import { postJSON } from '../../../../../infrastructure/fetch-json' import { UseAsyncReturnType } from '../../../../../shared/hooks/use-async' function DeleteButton({ children, disabled, onClick }: Button.ButtonProps) { + const { t } = useTranslation() + return ( ) } @@ -47,7 +49,7 @@ function Remove({ userEmailData, deleteEmailAsync }: RemoveProps) { } if (deleteEmailAsync.isLoading) { - return {t('deleting')}... + return } return ( @@ -64,17 +66,7 @@ function Remove({ userEmailData, deleteEmailAsync }: RemoveProps) { - - + /> ) diff --git a/services/web/frontend/js/features/settings/components/leavers-survey-alert.tsx b/services/web/frontend/js/features/settings/components/leavers-survey-alert.tsx index dfa2e407b7..c4ece7dc12 100644 --- a/services/web/frontend/js/features/settings/components/leavers-survey-alert.tsx +++ b/services/web/frontend/js/features/settings/components/leavers-survey-alert.tsx @@ -7,16 +7,29 @@ export function LeaversSurveyAlert() { const [expirationDate, setExpirationDate] = usePersistedState( 'showInstitutionalLeaversSurveyUntil', - 0 + 0, + true ) + + const [hide, setHide] = usePersistedState( + 'hideInstitutionalLeaversSurvey', + false, + true + ) + function handleDismiss() { setExpirationDate(0) + setHide(true) } if (Date.now() > expirationDate) { return null } + if (hide) { + return null + } + return (

diff --git a/services/web/frontend/js/features/settings/context/user-email-context.tsx b/services/web/frontend/js/features/settings/context/user-email-context.tsx index 9c87dc6385..b364752ad7 100644 --- a/services/web/frontend/js/features/settings/context/user-email-context.tsx +++ b/services/web/frontend/js/features/settings/context/user-email-context.tsx @@ -220,23 +220,21 @@ function useUserEmails() { const resetLeaversSurveyExpiration = useCallback( (deletedEmail: UserEmailData) => { - if (!data) { - return - } - const emailData = data as UserEmailData[] if ( deletedEmail.emailHasInstitutionLicence || deletedEmail.affiliation?.pastReconfirmDate ) { - const stillHasLicenseAccess = emailData.some( - userEmail => userEmail.emailHasInstitutionLicence + const stillHasLicenseAccess = Object.values(state.data.byId).some( + userEmail => + userEmail.email !== deletedEmail.email && + userEmail.emailHasInstitutionLicence ) - if (stillHasLicenseAccess) { + if (!stillHasLicenseAccess) { setExpirationDate(Date.now() + ONE_WEEK_IN_MS) } } }, - [data, setExpirationDate] + [state, setExpirationDate] ) return { diff --git a/services/web/frontend/stories/settings/helpers/emails.js b/services/web/frontend/stories/settings/helpers/emails.js index 0d2991a623..370a8e5383 100644 --- a/services/web/frontend/stories/settings/helpers/emails.js +++ b/services/web/frontend/stories/settings/helpers/emails.js @@ -13,6 +13,7 @@ const fakeUsersData = [ confirmedAt: '2022-03-09T10:59:44.139Z', email: 'foo@overleaf.com', default: true, + emailHasInstitutionLicence: true, }, { confirmedAt: '2022-03-10T10:59:44.139Z', @@ -115,4 +116,8 @@ export function setDefaultMeta() { hasSamlFeature: true, samlInitPath: 'saml/init', }) + localStorage.setItem( + 'showInstitutionalLeaversSurveyUntil', + (Date.now() - 1000 * 60 * 60).toString() + ) } diff --git a/services/web/frontend/stories/settings/leavers-survey-alert.stories.tsx b/services/web/frontend/stories/settings/leavers-survey-alert.stories.tsx index b793913f9d..9a57aa9ee0 100644 --- a/services/web/frontend/stories/settings/leavers-survey-alert.stories.tsx +++ b/services/web/frontend/stories/settings/leavers-survey-alert.stories.tsx @@ -1,10 +1,11 @@ import EmailsSection from '../../js/features/settings/components/emails-section' import { LeaversSurveyAlert } from '../../js/features/settings/components/leavers-survey-alert' +import localStorage from '../../js/infrastructure/local-storage' export const SurveyAlert = () => { localStorage.setItem( 'showInstitutionalLeaversSurveyUntil', - (Date.now() + 1000 * 60 * 60).toString() + Date.now() + 1000 * 60 * 60 ) return } diff --git a/services/web/test/frontend/features/settings/components/emails/emails-section-actions.test.tsx b/services/web/test/frontend/features/settings/components/emails/emails-section-actions.test.tsx index 6a88d52ddc..698d894d43 100644 --- a/services/web/test/frontend/features/settings/components/emails/emails-section-actions.test.tsx +++ b/services/web/test/frontend/features/settings/components/emails/emails-section-actions.test.tsx @@ -92,16 +92,12 @@ describe('email actions - delete', function () { const button = await screen.findByRole('button', { name: /remove/i }) fireEvent.click(button) - expect(screen.queryByRole('button', { name: /remove/i })).to.be.null - await waitForElementToBeRemoved(() => - screen.getByRole('button', { name: /deleting/i }) + screen.queryByText(userEmailData.email) ) - - expect(screen.queryByText(userEmailData.email)).to.be.null }) - it('shows error when making email primary', async function () { + it('shows error when deleting', async function () { fetchMock .get('/user/emails?ensureAffiliation=true', [userEmailData]) .post('/user/emails/delete', 503) @@ -110,11 +106,7 @@ describe('email actions - delete', function () { const button = await screen.findByRole('button', { name: /remove/i }) fireEvent.click(button) - await waitForElementToBeRemoved(() => - screen.getByRole('button', { name: /deleting/i }) - ) - - screen.getByText(/sorry, something went wrong/i) + await screen.queryByText(/sorry, something went wrong/i) screen.getByRole('button', { name: /remove/i }) }) }) diff --git a/services/web/test/frontend/features/settings/components/leavers-survey-alert.test.tsx b/services/web/test/frontend/features/settings/components/leavers-survey-alert.test.tsx index 6af4bf7ba4..b2bebb0d20 100644 --- a/services/web/test/frontend/features/settings/components/leavers-survey-alert.test.tsx +++ b/services/web/test/frontend/features/settings/components/leavers-survey-alert.test.tsx @@ -1,14 +1,13 @@ import { expect } from 'chai' import { fireEvent, screen, render } from '@testing-library/react' import { LeaversSurveyAlert } from '../../../../../frontend/js/features/settings/components/leavers-survey-alert' +import localStorage from '../../../../../frontend/js/infrastructure/local-storage' describe('', function () { it('should render before the expiration date', function () { const tomorrow = Date.now() + 1000 * 60 * 60 * 24 - localStorage.setItem( - 'showInstitutionalLeaversSurveyUntil', - tomorrow.toString() - ) + localStorage.setItem('showInstitutionalLeaversSurveyUntil', tomorrow) + localStorage.setItem('hideInstitutionalLeaversSurvey', false) render() screen.getByRole('alert') screen.getByText(/Provide some quick feedback/) @@ -17,20 +16,24 @@ describe('', function () { it('should not render after the expiration date', function () { const yesterday = Date.now() - 1000 * 60 * 60 * 24 - localStorage.setItem( - 'showInstitutionalLeaversSurveyUntil', - yesterday.toString() - ) + localStorage.setItem('showInstitutionalLeaversSurveyUntil', yesterday) + localStorage.setItem('hideInstitutionalLeaversSurvey', false) + render() + expect(screen.queryByRole('alert')).to.be.null + }) + + it('should not render if it has been hidden', function () { + const tomorrow = Date.now() + 1000 * 60 * 60 * 24 + localStorage.setItem('showInstitutionalLeaversSurveyUntil', tomorrow) + localStorage.setItem('hideInstitutionalLeaversSurvey', true) render() expect(screen.queryByRole('alert')).to.be.null }) it('should reset the expiration date when it is closed', function () { const tomorrow = Date.now() + 1000 * 60 * 60 * 24 - localStorage.setItem( - 'showInstitutionalLeaversSurveyUntil', - tomorrow.toString() - ) + localStorage.setItem('showInstitutionalLeaversSurveyUntil', tomorrow) + localStorage.setItem('hideInstitutionalLeaversSurvey', false) render() screen.getByRole('alert')