Przeglądaj źródła

Merge pull request #7792 from overleaf/ta-settings-fixes-4

[SettingsPage] Wording and Error Handling

GitOrigin-RevId: 1e2445a68e0d32cbec558832892f2ce5a051d729
Timothée Alby 4 lat temu
rodzic
commit
c043db0ed9

+ 5 - 1
services/web/app/src/Features/User/UserPagesController.js

@@ -18,6 +18,10 @@ async function settingsPage(req, res) {
   if (ssoError) {
   if (ssoError) {
     delete req.session.ssoError
     delete req.session.ssoError
   }
   }
+  const ssoErrorMessage = req.session.ssoErrorMessage
+  if (ssoErrorMessage) {
+    delete req.session.ssoErrorMessage
+  }
   // Institution SSO
   // Institution SSO
   let institutionLinked = _.get(req.session, ['saml', 'linked'])
   let institutionLinked = _.get(req.session, ['saml', 'linked'])
   if (institutionLinked) {
   if (institutionLinked) {
@@ -97,7 +101,7 @@ async function settingsPage(req, res) {
       reconfirmedViaSAML,
       reconfirmedViaSAML,
       reconfirmationRemoveEmail,
       reconfirmationRemoveEmail,
       samlBeta: req.session.samlBeta,
       samlBeta: req.session.samlBeta,
-      ssoError: ssoError,
+      ssoErrorMessage,
       thirdPartyIds: UserPagesController._restructureThirdPartyIds(user),
       thirdPartyIds: UserPagesController._restructureThirdPartyIds(user),
     })
     })
   } else {
   } else {

+ 1 - 1
services/web/app/views/project/list/notifications.pug

@@ -126,7 +126,7 @@ include ../../_mixins/reconfirm_affiliation
 					.notification-body
 					.notification-body
 						if user.features.dropbox
 						if user.features.dropbox
 							| !{translate("dropbox_unlinked_premium_feature", {}, ['strong'])}
 							| !{translate("dropbox_unlinked_premium_feature", {}, ['strong'])}
-							| !{translate("can_now_relink_dropbox", {}, [{name: 'a', attrs: {href: '/user/settings#dropboxSettings' }}])}
+							| !{translate("can_now_relink_dropbox", {}, [{name: 'a', attrs: {href: '/user/settings#project-sync' }}])}
 						else
 						else
 							| !{translate("dropbox_unlinked_premium_feature", {}, ['strong'])}
 							| !{translate("dropbox_unlinked_premium_feature", {}, ['strong'])}
 							| !{translate("confirm_affiliation_to_relink_dropbox")}
 							| !{translate("confirm_affiliation_to_relink_dropbox")}

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

@@ -14,7 +14,7 @@ block append meta
 	meta(name="ol-reconfirmedViaSAML", content=reconfirmedViaSAML)
 	meta(name="ol-reconfirmedViaSAML", content=reconfirmedViaSAML)
 	meta(name="ol-reconfirmationRemoveEmail", content=reconfirmationRemoveEmail)
 	meta(name="ol-reconfirmationRemoveEmail", content=reconfirmationRemoveEmail)
 	meta(name="ol-samlBeta", content=samlBeta)
 	meta(name="ol-samlBeta", content=samlBeta)
-	meta(name="ol-ssoError", content=ssoError)
+	meta(name="ol-ssoErrorMessage", content=ssoErrorMessage)
 	meta(name="ol-thirdPartyIds", data-type="json", content=thirdPartyIds || {})
 	meta(name="ol-thirdPartyIds", data-type="json", content=thirdPartyIds || {})
 	meta(name="ol-passwordStrengthOptions", data-type="json", content=settings.passwordStrengthOptions || {})
 	meta(name="ol-passwordStrengthOptions", data-type="json", content=settings.passwordStrengthOptions || {})
 	meta(name="ol-isExternalAuthenticationSystemUsed" data-type="boolean" content=externalAuthenticationSystemUsed())
 	meta(name="ol-isExternalAuthenticationSystemUsed" data-type="boolean" content=externalAuthenticationSystemUsed())

+ 7 - 1
services/web/frontend/js/features/settings/components/linking-section.tsx

@@ -1,4 +1,5 @@
 import { useState } from 'react'
 import { useState } from 'react'
+import { Alert } from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
 import { useTranslation } from 'react-i18next'
 import importOverleafModules from '../../../../macros/import-overleaf-module.macro'
 import importOverleafModules from '../../../../macros/import-overleaf-module.macro'
 import { useSSOContext, SSOSubscription } from '../context/sso-context'
 import { useSSOContext, SSOSubscription } from '../context/sso-context'
@@ -8,7 +9,7 @@ import getMeta from '../../../utils/meta'
 function LinkingSection() {
 function LinkingSection() {
   const { t } = useTranslation()
   const { t } = useTranslation()
   const { subscriptions } = useSSOContext()
   const { subscriptions } = useSSOContext()
-
+  const ssoErrorMessage = getMeta('ol-ssoErrorMessage') as string
   const [integrationLinkingWidgets] = useState(
   const [integrationLinkingWidgets] = useState(
     () =>
     () =>
       getMeta('integrationLinkingWidgets') ||
       getMeta('integrationLinkingWidgets') ||
@@ -77,6 +78,11 @@ function LinkingSection() {
           <h3 id="linked-accounts" className="text-capitalize">
           <h3 id="linked-accounts" className="text-capitalize">
             {t('linked_accounts')}
             {t('linked_accounts')}
           </h3>
           </h3>
+          {ssoErrorMessage ? (
+            <Alert bsStyle="danger">
+              {t('sso_link_error')}: {ssoErrorMessage}
+            </Alert>
+          ) : null}
           <div className="settings-widgets-container">
           <div className="settings-widgets-container">
             {Object.values(subscriptions).map(
             {Object.values(subscriptions).map(
               (subscription, subscriptionIndex) => (
               (subscription, subscriptionIndex) => (

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

@@ -16,6 +16,7 @@ type IntegrationLinkingWidgetProps = {
   unlinkPath: string
   unlinkPath: string
   unlinkConfirmationTitle: string
   unlinkConfirmationTitle: string
   unlinkConfirmationText: string
   unlinkConfirmationText: string
+  disabled?: boolean
 }
 }
 
 
 export function IntegrationLinkingWidget({
 export function IntegrationLinkingWidget({
@@ -30,6 +31,7 @@ export function IntegrationLinkingWidget({
   unlinkPath,
   unlinkPath,
   unlinkConfirmationTitle,
   unlinkConfirmationTitle,
   unlinkConfirmationText,
   unlinkConfirmationText,
+  disabled,
 }: IntegrationLinkingWidgetProps) {
 }: IntegrationLinkingWidgetProps) {
   const { t } = useTranslation()
   const { t } = useTranslation()
 
 
@@ -59,7 +61,7 @@ export function IntegrationLinkingWidget({
             {t('learn_more')}
             {t('learn_more')}
           </a>
           </a>
         </p>
         </p>
-        {linked && statusIndicator}
+        {hasFeature && statusIndicator}
       </div>
       </div>
       <div>
       <div>
         <ActionButton
         <ActionButton
@@ -68,6 +70,7 @@ export function IntegrationLinkingWidget({
           linked={linked}
           linked={linked}
           handleUnlinkClick={handleUnlinkClick}
           handleUnlinkClick={handleUnlinkClick}
           linkPath={linkPath}
           linkPath={linkPath}
+          disabled={disabled}
         />
         />
       </div>
       </div>
       <UnlinkConfirmationModal
       <UnlinkConfirmationModal
@@ -87,6 +90,7 @@ type ActionButtonProps = {
   linked?: boolean
   linked?: boolean
   handleUnlinkClick: () => void
   handleUnlinkClick: () => void
   linkPath: string
   linkPath: string
+  disabled?: boolean
 }
 }
 
 
 function ActionButton({
 function ActionButton({
@@ -95,6 +99,7 @@ function ActionButton({
   linked,
   linked,
   handleUnlinkClick,
   handleUnlinkClick,
   linkPath,
   linkPath,
+  disabled,
 }: ActionButtonProps) {
 }: ActionButtonProps) {
   const { t } = useTranslation()
   const { t } = useTranslation()
 
 
@@ -106,13 +111,21 @@ function ActionButton({
     )
     )
   } else if (linked) {
   } else if (linked) {
     return (
     return (
-      <button className="btn btn-danger" onClick={handleUnlinkClick}>
+      <button
+        className="btn btn-danger"
+        onClick={handleUnlinkClick}
+        disabled={disabled}
+      >
         {t('unlink')}
         {t('unlink')}
       </button>
       </button>
     )
     )
   } else {
   } else {
     return (
     return (
-      <a href={linkPath} className="btn btn-primary text-capitalize">
+      <a
+        href={linkPath}
+        className="btn btn-primary text-capitalize"
+        disabled={disabled}
+      >
         {t('link')}
         {t('link')}
       </a>
       </a>
     )
     )

+ 8 - 3
services/web/frontend/js/features/settings/components/linking/sso-widget.tsx

@@ -1,10 +1,12 @@
 import { useCallback, useState } from 'react'
 import { useCallback, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { useTranslation } from 'react-i18next'
 import { Button, Modal } from 'react-bootstrap'
 import { Button, Modal } from 'react-bootstrap'
+import { FetchError } from '../../../../infrastructure/fetch-json'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import AccessibleModal from '../../../../shared/components/accessible-modal'
 import IEEELogo from '../../../../shared/svgs/ieee-logo'
 import IEEELogo from '../../../../shared/svgs/ieee-logo'
 import GoogleLogo from '../../../../shared/svgs/google-logo'
 import GoogleLogo from '../../../../shared/svgs/google-logo'
 import OrcidLogo from '../../../../shared/svgs/orcid-logo'
 import OrcidLogo from '../../../../shared/svgs/orcid-logo'
+import LinkingStatus from './status'
 
 
 const providerLogos = {
 const providerLogos = {
   collabratec: <IEEELogo />,
   collabratec: <IEEELogo />,
@@ -38,14 +40,15 @@ export function SSOLinkingWidget({
 
 
   const handleUnlinkClick = useCallback(() => {
   const handleUnlinkClick = useCallback(() => {
     setShowModal(true)
     setShowModal(true)
+    setErrorMessage('')
   }, [])
   }, [])
 
 
   const handleUnlinkConfirmationClick = useCallback(() => {
   const handleUnlinkConfirmationClick = useCallback(() => {
     setShowModal(false)
     setShowModal(false)
     setUnlinkRequestInflight(true)
     setUnlinkRequestInflight(true)
     onUnlink()
     onUnlink()
-      .catch((error: Error) => {
-        setErrorMessage(error.message)
+      .catch((error: FetchError) => {
+        setErrorMessage(error.getUserFacingMessage())
       })
       })
       .finally(() => {
       .finally(() => {
         setUnlinkRequestInflight(false)
         setUnlinkRequestInflight(false)
@@ -71,7 +74,9 @@ export function SSOLinkingWidget({
             </a>
             </a>
           ) : null}
           ) : null}
         </p>
         </p>
-        {errorMessage && <div>{errorMessage} </div>}
+        {errorMessage ? (
+          <LinkingStatus status="error" description={errorMessage} />
+        ) : null}
       </div>
       </div>
       <div>
       <div>
         <ActionButton
         <ActionButton

+ 57 - 0
services/web/frontend/js/features/settings/components/linking/status.tsx

@@ -0,0 +1,57 @@
+import { ReactNode } from 'react'
+import Icon from '../../../../shared/components/icon'
+
+type Status = 'pending' | 'success' | 'error'
+
+type LinkingStatusProps = {
+  status?: Status
+  description: string | ReactNode
+}
+
+export default function LinkingStatus({
+  status,
+  description,
+}: LinkingStatusProps) {
+  return (
+    <span>
+      <StatusIcon status={status} />
+      <span className="small"> {description}</span>
+    </span>
+  )
+}
+
+type StatusIconProps = {
+  status: Status
+}
+
+function StatusIcon({ status }: StatusIconProps) {
+  switch (status) {
+    case 'success':
+      return (
+        <Icon
+          type="check-circle"
+          fw
+          className="settings-widget-status-icon status-success"
+        />
+      )
+    case 'error':
+      return (
+        <Icon
+          type="times-circle"
+          fw
+          className="settings-widget-status-icon status-error"
+        />
+      )
+    case 'pending':
+      return (
+        <Icon
+          type="circle"
+          fw
+          className="settings-widget-status-icon status-pending"
+          spin
+        />
+      )
+    default:
+      return null
+  }
+}

+ 0 - 7
services/web/frontend/js/features/settings/components/root.tsx

@@ -1,5 +1,4 @@
 import { useEffect } from 'react'
 import { useEffect } from 'react'
-import { Alert } from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
 import { useTranslation } from 'react-i18next'
 import getMeta from '../../../utils/meta'
 import getMeta from '../../../utils/meta'
 import EmailsSection from './emails-section'
 import EmailsSection from './emails-section'
@@ -38,16 +37,10 @@ function SettingsPageRoot() {
 
 
 function SettingsPageContent() {
 function SettingsPageContent() {
   const { t } = useTranslation()
   const { t } = useTranslation()
-  const ssoError = getMeta('ol-ssoError') as string
   const { isOverleaf } = getMeta('ol-ExposedSettings') as ExposedSettings
   const { isOverleaf } = getMeta('ol-ExposedSettings') as ExposedSettings
 
 
   return (
   return (
     <UserProvider>
     <UserProvider>
-      {ssoError ? (
-        <Alert bsStyle="danger">
-          {t('sso_link_error')}: {t(ssoError)}
-        </Alert>
-      ) : null}
       <div className="card">
       <div className="card">
         <div className="page-header">
         <div className="page-header">
           <h1>{t('account_settings')}</h1>
           <h1>{t('account_settings')}</h1>

+ 1 - 0
services/web/frontend/stories/settings/helpers/linking.js

@@ -58,4 +58,5 @@ export function setDefaultMeta() {
   })
   })
   window.metaAttributesCache.delete('integrationLinkingWidgets')
   window.metaAttributesCache.delete('integrationLinkingWidgets')
   window.metaAttributesCache.delete('referenceLinkingWidgets')
   window.metaAttributesCache.delete('referenceLinkingWidgets')
+  window.metaAttributesCache.delete('ol-ssoErrorMessage')
 }
 }

+ 23 - 0
services/web/frontend/stories/settings/linking.stories.js

@@ -4,6 +4,8 @@ import { setDefaultMeta, defaultSetupMocks } from './helpers/linking'
 import { UserProvider } from '../../js/shared/context/user-context'
 import { UserProvider } from '../../js/shared/context/user-context'
 import { SSOProvider } from '../../js/features/settings/context/sso-context'
 import { SSOProvider } from '../../js/features/settings/context/sso-context'
 
 
+const MOCK_DELAY = 1000
+
 export const Section = args => {
 export const Section = args => {
   useFetchMock(defaultSetupMocks)
   useFetchMock(defaultSetupMocks)
   setDefaultMeta()
   setDefaultMeta()
@@ -40,6 +42,27 @@ export const SectionAllUnlinked = args => {
   )
   )
 }
 }
 
 
+export const SectionSSOErrors = args => {
+  useFetchMock(fetchMock =>
+    fetchMock.post('/user/oauth-unlink', 500, { delay: MOCK_DELAY })
+  )
+  setDefaultMeta()
+  window.metaAttributesCache.set('integrationLinkingWidgets', [])
+  window.metaAttributesCache.set('referenceLinkingWidgets', [])
+  window.metaAttributesCache.set(
+    'ol-ssoErrorMessage',
+    'Account already linked to another Overleaf user'
+  )
+
+  return (
+    <UserProvider>
+      <SSOProvider>
+        <LinkingSection {...args} />
+      </SSOProvider>
+    </UserProvider>
+  )
+}
+
 export default {
 export default {
   title: 'Account Settings / Linking',
   title: 'Account Settings / Linking',
   component: LinkingSection,
   component: LinkingSection,

+ 4 - 0
services/web/frontend/stylesheets/app/account-settings.less

@@ -107,17 +107,21 @@ tbody > tr.affiliations-table-warning-row > td {
   margin-bottom: (@line-height-computed / 2) - @table-cell-padding;
   margin-bottom: (@line-height-computed / 2) - @table-cell-padding;
 }
 }
 
 
+.settings-widget-status-icon,
 .dropbox-sync-icon {
 .dropbox-sync-icon {
   position: relative;
   position: relative;
   font-size: 1.3em;
   font-size: 1.3em;
   line-height: 1.3em;
   line-height: 1.3em;
   vertical-align: top;
   vertical-align: top;
+  &.status-error,
   &.dropbox-sync-icon-error {
   &.dropbox-sync-icon-error {
     color: @alert-danger-bg;
     color: @alert-danger-bg;
   }
   }
+  &.status-success,
   &.dropbox-sync-icon-success {
   &.dropbox-sync-icon-success {
     color: @alert-success-bg;
     color: @alert-success-bg;
   }
   }
+  &.status-pending,
   &.dropbox-sync-icon-updating {
   &.dropbox-sync-icon-updating {
     color: @alert-info-bg;
     color: @alert-info-bg;
     &::after {
     &::after {

+ 5 - 5
services/web/locales/en.json

@@ -635,7 +635,7 @@
   "tracked_change_deleted": "Deleted",
   "tracked_change_deleted": "Deleted",
   "show_all": "show all",
   "show_all": "show all",
   "show_less": "show less",
   "show_less": "show less",
-  "dropbox_sync_error": "Dropbox Sync Error",
+  "dropbox_sync_error": "Sorry, there was an error talking to our Dropbox service. Please try again in a few moments.",
   "send": "Send",
   "send": "Send",
   "sending": "Sending",
   "sending": "Sending",
   "invalid_password": "Invalid Password",
   "invalid_password": "Invalid Password",
@@ -762,7 +762,7 @@
   "mendeley_reference_loading_error_forbidden": "Could not load references from Mendeley, please re-link your account and try again",
   "mendeley_reference_loading_error_forbidden": "Could not load references from Mendeley, please re-link your account and try again",
   "zotero_reference_loading_error_forbidden": "Could not load references from Zotero, please re-link your account and try again",
   "zotero_reference_loading_error_forbidden": "Could not load references from Zotero, please re-link your account and try again",
   "mendeley_integration": "Mendeley Integration",
   "mendeley_integration": "Mendeley Integration",
-  "mendeley_sync_description": "With Mendeley integration you can import your references from Mendeley into your __appName__ projects.",
+  "mendeley_sync_description": "With the Mendeley integration you can import your references from Mendeley into your __appName__ projects.",
   "mendeley_is_premium": "Mendeley integration is a premium feature",
   "mendeley_is_premium": "Mendeley integration is a premium feature",
   "link_to_mendeley": "Link to Mendeley",
   "link_to_mendeley": "Link to Mendeley",
   "unlink_to_mendeley": "Unlink Mendeley",
   "unlink_to_mendeley": "Unlink Mendeley",
@@ -772,7 +772,7 @@
   "mendeley_groups_loading_error": "There was an error loading groups from Mendeley",
   "mendeley_groups_loading_error": "There was an error loading groups from Mendeley",
   "mendeley_groups_relink": "There was an error accessing your Mendeley data. This was likely caused by lack of permissions. Please re-link your account and try again.",
   "mendeley_groups_relink": "There was an error accessing your Mendeley data. This was likely caused by lack of permissions. Please re-link your account and try again.",
   "zotero_integration": "Zotero Integration",
   "zotero_integration": "Zotero Integration",
-  "zotero_sync_description": "With Zotero integration you can import your references from Zotero into your __appName__ projects.",
+  "zotero_sync_description": "With the Zotero integration you can import your references from Zotero into your __appName__ projects.",
   "zotero_is_premium": "Zotero integration is a premium feature",
   "zotero_is_premium": "Zotero integration is a premium feature",
   "link_to_zotero": "Link to Zotero",
   "link_to_zotero": "Link to Zotero",
   "unlink_to_zotero": "Unlink Zotero",
   "unlink_to_zotero": "Unlink Zotero",
@@ -1463,8 +1463,8 @@
   "about_us": "About Us",
   "about_us": "About Us",
   "loading_recent_github_commits": "Loading recent commits",
   "loading_recent_github_commits": "Loading recent commits",
   "no_new_commits_in_github": "No new commits in GitHub since last merge.",
   "no_new_commits_in_github": "No new commits in GitHub since last merge.",
-  "dropbox_sync_description": "Keep your __appName__ projects in sync with your Dropbox. Changes in __appName__ are automatically sent to your Dropbox, and the other way around.",
-  "github_sync_description": "With GitHub Sync you can link your __appName__ projects to GitHub repositories. Create new commits from __appName__, and merge with commits made offline or in GitHub.",
+  "dropbox_sync_description": "Keep your __appName__ projects in sync with your Dropbox account. Changes in __appName__ are automatically sent to your Dropbox account, and the other way around.",
+  "github_sync_description": "With GitHub Sync you can link your __appName__ projects to GitHub repositories, create new commits from __appName__, and merge commits from GitHub.",
   "github_import_description": "With GitHub Sync you can import your GitHub Repositories into __appName__. Create new commits from __appName__, and merge with commits made offline or in GitHub.",
   "github_import_description": "With GitHub Sync you can import your GitHub Repositories into __appName__. Create new commits from __appName__, and merge with commits made offline or in GitHub.",
   "link_to_github_description": "You need to authorise __appName__ to access your GitHub account to allow us to sync your projects.",
   "link_to_github_description": "You need to authorise __appName__ to access your GitHub account to allow us to sync your projects.",
   "link": "Link",
   "link": "Link",

+ 6 - 0
services/web/test/frontend/features/settings/components/linking-section.test.tsx

@@ -89,6 +89,12 @@ describe('<LinkingSection />', function () {
     expect(screen.queryByText('Twitter')).to.not.exist
     expect(screen.queryByText('Twitter')).to.not.exist
   })
   })
 
 
+  it('shows SSO error message', async function () {
+    window.metaAttributesCache.set('ol-ssoErrorMessage', 'You no SSO')
+    renderSectionWithProviders()
+    screen.getByText('Error linking SSO account: You no SSO')
+  })
+
   it('does not show providers section when empty', async function () {
   it('does not show providers section when empty', async function () {
     window.metaAttributesCache.delete('ol-oauthProviders')
     window.metaAttributesCache.delete('ol-oauthProviders')
     renderSectionWithProviders()
     renderSectionWithProviders()

+ 9 - 2
services/web/test/frontend/features/settings/components/linking/sso-widget.test.tsx

@@ -1,6 +1,7 @@
 import { expect } from 'chai'
 import { expect } from 'chai'
 import sinon from 'sinon'
 import sinon from 'sinon'
 import { screen, fireEvent, render, waitFor } from '@testing-library/react'
 import { screen, fireEvent, render, waitFor } from '@testing-library/react'
+import { FetchError } from '../../../../../../frontend/js/infrastructure/fetch-json'
 import { SSOLinkingWidget } from '../../../../../../frontend/js/features/settings/components/linking/sso-widget'
 import { SSOLinkingWidget } from '../../../../../../frontend/js/features/settings/components/linking/sso-widget'
 
 
 describe('<SSOLinkingWidget />', function () {
 describe('<SSOLinkingWidget />', function () {
@@ -103,7 +104,9 @@ describe('<SSOLinkingWidget />', function () {
 
 
   describe('when unlinking fails', function () {
   describe('when unlinking fails', function () {
     beforeEach(function () {
     beforeEach(function () {
-      const unlinkFunction = sinon.stub().rejects(new Error('unlinking failed'))
+      const unlinkFunction = sinon
+        .stub()
+        .rejects(new FetchError('unlinking failed', ''))
       render(
       render(
         <SSOLinkingWidget {...defaultProps} linked onUnlink={unlinkFunction} />
         <SSOLinkingWidget {...defaultProps} linked onUnlink={unlinkFunction} />
       )
       )
@@ -116,7 +119,11 @@ describe('<SSOLinkingWidget />', function () {
     })
     })
 
 
     it('should display an error message ', async function () {
     it('should display an error message ', async function () {
-      await waitFor(() => screen.getByText('unlinking failed'))
+      await waitFor(() =>
+        screen.getByText(
+          'Something went wrong talking to the server :(. Please try again.'
+        )
+      )
     })
     })
 
 
     it('should display the unlink button ', async function () {
     it('should display the unlink button ', async function () {