From 52a99870e88459c2186c564c71a9bbda52d82cbe Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Fri, 22 Nov 2024 11:04:51 -0500 Subject: [PATCH] [RHOAIENG-1154]: Apply modal to alert user error check for 403 status code add cypress test update test title format format format remove it.only fix relative import PR feedback from Christian, WIP Cypress throwing error on shouldBeOpen fix tests, fix linting issues revert find() function revert find, update comment format apply PR comments from review format remove response param uncomment simulate login button remove it.only reorder apply PR feedback from Christian remove find --- .../cypress/cypress/pages/components/Modal.ts | 2 +- .../cypress/cypress/pages/loginDialog.ts | 23 +++++++++++ .../cypress/cypress/support/commands/odh.ts | 9 +++-- .../cypress/tests/mocked/application.cy.ts | 25 ++++++++++++ frontend/src/app/App.tsx | 10 ++++- frontend/src/app/SessionExpiredModal.tsx | 39 +++++++++++++++++++ frontend/src/app/useApplicationSettings.tsx | 7 ++-- .../src/services/dashboardConfigService.ts | 7 +--- 8 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 frontend/src/__tests__/cypress/cypress/pages/loginDialog.ts create mode 100644 frontend/src/app/SessionExpiredModal.tsx diff --git a/frontend/src/__tests__/cypress/cypress/pages/components/Modal.ts b/frontend/src/__tests__/cypress/cypress/pages/components/Modal.ts index a8efa38a3b..670dd6e3d9 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/components/Modal.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/components/Modal.ts @@ -12,7 +12,7 @@ export class Modal { } find(): Cypress.Chainable> { - // FIXME Remove `hidden: true` once issue with PF modals is fixed. + // FIXME Remove `hidden: true` once PF version is upgraded to 6.1.0. // https://issues.redhat.com/browse/RHOAIENG-11946 // https://github.com/patternfly/patternfly-react/issues/11041 return cy.findByRole('dialog', { name: this.title, hidden: true }); diff --git a/frontend/src/__tests__/cypress/cypress/pages/loginDialog.ts b/frontend/src/__tests__/cypress/cypress/pages/loginDialog.ts new file mode 100644 index 0000000000..34e6aa44ca --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/pages/loginDialog.ts @@ -0,0 +1,23 @@ +import { Modal } from '~/__tests__/cypress/cypress/pages/components/Modal'; + +class LoginDialog extends Modal { + constructor() { + super(/Session Expired/); + } + + // FIXME Remove once PF version is upgraded to 6.1.0. + // https://issues.redhat.com/browse/RHOAIENG-11946 + // https://github.com/patternfly/patternfly-react/issues/11041 + shouldBeOpen(open = true): void { + if (open) { + this.find(); + } else { + this.find().should('not.exist'); + } + } + + findLoginButton() { + return this.findFooter().findByTestId('modal-login-button'); + } +} +export const loginDialog = new LoginDialog(); diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts index 6db63923d4..dffed83cc5 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts @@ -79,10 +79,11 @@ type Options = { path?: Replacement; query?: Query; times?: number } | null; declare global { namespace Cypress { interface Chainable { - interceptOdh: (( - type: 'POST /api/accelerator-profiles', - response?: OdhResponse, - ) => Cypress.Chainable) & + interceptOdh: ((type: 'GET /oauth/sign_out') => Cypress.Chainable) & + (( + type: 'POST /api/accelerator-profiles', + response?: OdhResponse, + ) => Cypress.Chainable) & (( type: 'DELETE /api/accelerator-profiles/:name', options: { path: { name: string } }, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts index 5b0747ee19..2df5ae3080 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts @@ -7,6 +7,7 @@ import { import { mockDashboardConfig } from '~/__mocks__'; import { aboutDialog } from '~/__tests__/cypress/cypress/pages/aboutDialog'; import { mockConsoleLinks } from '~/__mocks__/mockConsoleLinks'; +import { loginDialog } from '~/__tests__/cypress/cypress/pages/loginDialog'; describe('Application', () => { it('should disallow access to the dashboard', () => { @@ -113,4 +114,28 @@ describe('Application', () => { aboutDialog.findText().should('contain.text', 'OpenShift'); aboutDialog.findProductName().should('contain.text', 'OpenShift AI'); }); + it('should show the login modal when receiving a 403 status code', () => { + // Mock the intercept to return a 403 status code + cy.interceptOdh('GET /api/config', { + statusCode: 403, + }).as('getData403'); + + // Set up the sign-out intercept before visiting the page + cy.interceptOdh('GET /oauth/sign_out').as('signOut'); + + // Visit the page where the request is triggered + cy.visit('/'); + + // Wait for the intercept to be triggered + cy.wait('@getData403'); + + // Verify that the login modal is displayed + loginDialog.shouldBeOpen(); + + // Simulate clicking the Log in button + loginDialog.findLoginButton().click(); + + // Wait for the sign out intercept to be triggered + cy.wait('@signOut'); + }); }); diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 614cc536a2..0282b8fd08 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -34,6 +34,7 @@ import TelemetrySetup from './TelemetrySetup'; import { logout } from './appUtils'; import QuickStarts from './QuickStarts'; import DevFeatureFlagsBanner from './DevFeatureFlagsBanner'; +import SessionExpiredModal from './SessionExpiredModal'; import './App.scss'; @@ -68,9 +69,16 @@ const App: React.FC = () => { [buildStatuses, dashboardConfig, storageClasses], ); + const isUnauthorized = fetchConfigError?.request?.status === 403; + // We lack the critical data to startup the app if (userError || fetchConfigError) { - // There was an error fetching critical data + // Check for unauthorized state + if (isUnauthorized) { + return ; + } + + // Default error handling for other cases return ( diff --git a/frontend/src/app/SessionExpiredModal.tsx b/frontend/src/app/SessionExpiredModal.tsx new file mode 100644 index 0000000000..fd16945348 --- /dev/null +++ b/frontend/src/app/SessionExpiredModal.tsx @@ -0,0 +1,39 @@ +import React from 'react'; + +import { + Modal, + ModalVariant, + ModalHeader, + ModalBody, + ModalFooter, + Button, +} from '@patternfly/react-core'; +import { logout } from './appUtils'; + +const SessionExpiredModal: React.FC = () => ( + + + Your session timed out. To continue working, log in. + + + + +); + +export default SessionExpiredModal; diff --git a/frontend/src/app/useApplicationSettings.tsx b/frontend/src/app/useApplicationSettings.tsx index 7797268d01..8e0cb736ff 100644 --- a/frontend/src/app/useApplicationSettings.tsx +++ b/frontend/src/app/useApplicationSettings.tsx @@ -1,4 +1,5 @@ import * as React from 'react'; +import { AxiosError } from 'axios'; import { DashboardConfigKind } from '~/k8sTypes'; import { POLL_INTERVAL } from '~/utilities/const'; import { useDeepCompareMemoize } from '~/utilities/useDeepCompareMemoize'; @@ -8,10 +9,10 @@ import useTimeBasedRefresh from './useTimeBasedRefresh'; export const useApplicationSettings = (): { dashboardConfig: DashboardConfigKind | null; loaded: boolean; - loadError: Error | undefined; + loadError: AxiosError | undefined; } => { const [loaded, setLoaded] = React.useState(false); - const [loadError, setLoadError] = React.useState(); + const [loadError, setLoadError] = React.useState(); const [dashboardConfig, setDashboardConfig] = React.useState(null); const setRefreshMarker = useTimeBasedRefresh(); @@ -29,7 +30,7 @@ export const useApplicationSettings = (): { setLoadError(undefined); }) .catch((e) => { - if (e?.message?.includes('Error getting Oauth Info for user')) { + if (e?.response?.data?.message?.includes('Error getting Oauth Info for user')) { // NOTE: this endpoint only requests oauth because of the security layer, this is not an ironclad use-case // Something went wrong on the server with the Oauth, let us just log them out and refresh for them /* eslint-disable-next-line no-console */ diff --git a/frontend/src/services/dashboardConfigService.ts b/frontend/src/services/dashboardConfigService.ts index 498c0a4d78..352fb861ba 100644 --- a/frontend/src/services/dashboardConfigService.ts +++ b/frontend/src/services/dashboardConfigService.ts @@ -3,10 +3,5 @@ import { DashboardConfigKind } from '~/k8sTypes'; export const fetchDashboardConfig = (): Promise => { const url = '/api/config'; - return axios - .get(url) - .then((response) => response.data) - .catch((e) => { - throw new Error(e.response.data.message); - }); + return axios.get(url).then((response) => response.data); };