Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RHOAIENG-1154]: Apply modal to alert user error #3512

Merged
merged 1 commit into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export class Modal {
}

find(): Cypress.Chainable<JQuery<HTMLElement>> {
// 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 });
Expand Down
23 changes: 23 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/loginDialog.ts
Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
Expand Up @@ -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<null>) &
interceptOdh: ((type: 'GET /oauth/sign_out') => Cypress.Chainable<null>) &
((
type: 'POST /api/accelerator-profiles',
response?: OdhResponse,
) => Cypress.Chainable<null>) &
((
type: 'DELETE /api/accelerator-profiles/:name',
options: { path: { name: string } },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test can also intercept the call to /oauth/sign_out when the user clicks the Log in button.

// 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');
});
});
10 changes: 9 additions & 1 deletion frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 <SessionExpiredModal />;
}

// Default error handling for other cases
return (
<Page>
<PageSection hasBodyWrapper={false}>
Expand Down
39 changes: 39 additions & 0 deletions frontend/src/app/SessionExpiredModal.tsx
Original file line number Diff line number Diff line change
@@ -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 = () => (
<Modal
data-testid="session-expired-modal"
variant={ModalVariant.small}
isOpen
aria-labelledby="session-expired-modal-title"
>
<ModalHeader
title="Session Expired"
titleIconVariant="warning"
labelId="session-expired-modal-title"
/>
<ModalBody>Your session timed out. To continue working, log in.</ModalBody>
<ModalFooter>
<Button
data-testid="modal-login-button"
key="confirm"
variant="primary"
onClick={() => logout().then(() => window.location.reload())}
>
Log in
</Button>
</ModalFooter>
</Modal>
);

export default SessionExpiredModal;
7 changes: 4 additions & 3 deletions frontend/src/app/useApplicationSettings.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<Error>();
const [loadError, setLoadError] = React.useState<AxiosError>();
const [dashboardConfig, setDashboardConfig] = React.useState<DashboardConfigKind | null>(null);
const setRefreshMarker = useTimeBasedRefresh();

Expand All @@ -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 */
Expand Down
7 changes: 1 addition & 6 deletions frontend/src/services/dashboardConfigService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,5 @@ import { DashboardConfigKind } from '~/k8sTypes';

export const fetchDashboardConfig = (): Promise<DashboardConfigKind> => {
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);
};
Loading