From 708adf56cc1c686987252f8ccb9788aa833d8017 Mon Sep 17 00:00:00 2001 From: Arman Jahanpour <77515879+rmanaem@users.noreply.github.com> Date: Wed, 17 Jul 2024 12:00:18 -0400 Subject: [PATCH] [ENH] Made authentication optional on the UI (#214) * Made authentication optional on UI * Set default visiblity of `AuthDialog` to `false` Renamed references of modal to dialog to avoid confusion with image modal * Updated `AuthDialog` component test * Implemented an e2e test for auth dialog and profile menu functionality * Updated `Navbar` component test * Updated e2e-test workflow config file * Addressed suggested changes from PR review --- .github/workflows/e2e-test.yaml | 2 +- cypress/component/AuthDialog.cy.tsx | 6 +-- cypress/component/Navbar.cy.tsx | 16 +++++++- cypress/component/ResultContainer.cy.tsx | 8 ++-- cypress/e2e/Auth.cy.ts | 17 +++++++++ src/App.tsx | 16 +++++++- src/components/AuthDialog.tsx | 19 ++++++++-- src/components/Navbar.tsx | 48 +++++++++++++++++------- src/components/ResultContainer.tsx | 8 ++-- 9 files changed, 106 insertions(+), 34 deletions(-) create mode 100644 cypress/e2e/Auth.cy.ts diff --git a/.github/workflows/e2e-test.yaml b/.github/workflows/e2e-test.yaml index 27235704..c3d46f69 100644 --- a/.github/workflows/e2e-test.yaml +++ b/.github/workflows/e2e-test.yaml @@ -21,7 +21,7 @@ jobs: - name: Create .env file run: | - echo -e "NB_API_QUERY_URL=https://federate.neurobagel.org/\nNB_IS_FEDERATION_API=true" > .env + echo -e "NB_API_QUERY_URL=https://federate.neurobagel.org/\nNB_IS_FEDERATION_API=true\nNB_ENABLE_AUTH=true\nNB_QUERY_CLIENT_ID=mockclientid" > .env - name: build run: npm install && npm run build diff --git a/cypress/component/AuthDialog.cy.tsx b/cypress/component/AuthDialog.cy.tsx index b6690b4f..d501de9a 100644 --- a/cypress/component/AuthDialog.cy.tsx +++ b/cypress/component/AuthDialog.cy.tsx @@ -2,16 +2,15 @@ import { GoogleOAuthProvider } from '@react-oauth/google'; import AuthDialog from '../../src/components/AuthDialog'; const props = { - isLoggedIn: false, onAuth: () => {}, + onClose: () => {}, }; describe('ContinuousField', () => { it('Displays a MUI dialog with the title and "sing in with google" button', () => { cy.mount( - {' '} - + ); cy.get('[data-cy="auth-dialog"]').should('be.visible'); @@ -19,5 +18,6 @@ describe('ContinuousField', () => { cy.get('[data-cy="auth-dialog"]').within(() => { cy.contains('Google'); }); + cy.get('[data-cy="close-auth-dialog-button"]').should('be.visible'); }); }); diff --git a/cypress/component/Navbar.cy.tsx b/cypress/component/Navbar.cy.tsx index 2982720e..18dc3b2f 100644 --- a/cypress/component/Navbar.cy.tsx +++ b/cypress/component/Navbar.cy.tsx @@ -1,14 +1,24 @@ import Navbar from '../../src/components/Navbar'; const props = { + isLoggedIn: true, name: 'john doe', profilePic: 'johndoe.png', onLogout: () => {}, + onLogin: () => {}, }; describe('Navbar', () => { it('Displays a MUI Toolbar with logo, title, subtitle, documentation link, and GitHub link', () => { - cy.mount(); + cy.mount( + + ); cy.get("[data-cy='navbar']").should('be.visible'); cy.get("[data-cy='navbar'] img").should('exist'); cy.get("[data-cy='navbar'] h5").should('contain', 'Neurobagel Query'); @@ -16,8 +26,10 @@ describe('Navbar', () => { 'contain', 'Define and find cohorts at the subject level' ); + // Check for the documentation and GitHub icon and links + cy.get("[data-cy='navbar'] a").eq(0).find('svg').should('be.visible'); cy.get("[data-cy='navbar'] a") - .should('contain', 'Documentation') + .eq(0) .should('have.attr', 'href', 'https://neurobagel.org/query_tool/'); cy.get("[data-cy='navbar'] a").eq(1).find('svg').should('be.visible'); cy.get("[data-cy='navbar'] a") diff --git a/cypress/component/ResultContainer.cy.tsx b/cypress/component/ResultContainer.cy.tsx index 3ed4d209..54a80890 100644 --- a/cypress/component/ResultContainer.cy.tsx +++ b/cypress/component/ResultContainer.cy.tsx @@ -2,7 +2,7 @@ import ResultContainer from '../../src/components/ResultContainer'; import { protectedResponse2 } from '../fixtures/mocked-responses'; describe('ResultContainer', () => { - it('Displays a set of Result Cards, select all checkbox, disabled download result buttons, summary stats, and how to get data modal button', () => { + it('Displays a set of Result Cards, select all checkbox, disabled download result buttons, summary stats, and how to get data dialog button', () => { cy.mount(); cy.get('[data-cy="card-https://someportal.org/datasets/ds0001"]').should('be.visible'); cy.get('[data-cy="card-https://someportal.org/datasets/ds0002"]').should('be.visible'); @@ -16,7 +16,7 @@ describe('ResultContainer', () => { cy.get('[data-cy="dataset-level-download-results-button"]') .should('be.visible') .should('be.disabled'); - cy.get('[data-cy="how-to-get-data-modal-button"]').should('be.visible'); + cy.get('[data-cy="how-to-get-data-dialog-button"]').should('be.visible'); }); it('Selecting a dataset should enable the download result buttons', () => { cy.mount(); @@ -45,10 +45,10 @@ describe('ResultContainer', () => { 'not.be.checked' ); }); - it('Clicking the how to get data modal button should open the modal', () => { + it('Clicking the how to get data dialog button should open the dialog', () => { cy.mount(); cy.get('[data-cy="get-data-dialog"]').should('not.exist'); - cy.get('[data-cy="how-to-get-data-modal-button"]').click(); + cy.get('[data-cy="how-to-get-data-dialog-button"]').click(); cy.get('[data-cy="get-data-dialog"]').should('be.visible'); }); it('Shows no result view when result is empty', () => { diff --git a/cypress/e2e/Auth.cy.ts b/cypress/e2e/Auth.cy.ts new file mode 100644 index 00000000..623d2010 --- /dev/null +++ b/cypress/e2e/Auth.cy.ts @@ -0,0 +1,17 @@ +describe('Authentication flow', () => { + it('Auth dialog is not visible by default and user is not logged in', () => { + cy.visit('/'); + cy.get('[data-cy="auth-dialog"]').should('not.exist'); + cy.get('.MuiAvatar-root').click(); + cy.get('[data-cy="login-button"]').should('exist'); + }); + it('Auth dialog can be opened and closed', () => { + cy.visit('/'); + cy.get('.MuiAvatar-root').click(); + cy.get('[data-cy="login-button"]').click(); + cy.get('[data-cy="auth-dialog"]').should('be.visible'); + cy.get('[data-cy="close-auth-dialog-button"]').should('be.visible'); + cy.get('[data-cy="close-auth-dialog-button"]').click(); + cy.get('[data-cy="auth-dialog"]').should('not.exist'); + }); +}); diff --git a/src/App.tsx b/src/App.tsx index 5e4e2bbb..2b99405f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -48,6 +48,7 @@ function App() { const [searchParams, setSearchParams] = useSearchParams(); const [isLoggedIn, setIsLoggedIn] = useState(false); + const [openAuthDialog, setOpenAuthDialog] = useState(false); const [name, setName] = useState(''); const [profilePic, setProfilePic] = useState(''); const [IDToken, setIDToken] = useState(''); @@ -345,6 +346,7 @@ function App() { function login(credential: string | undefined) { setIsLoggedIn(true); + setOpenAuthDialog(false); const jwt: GoogleJWT = credential ? jwtDecode(credential) : ({} as GoogleJWT); setIDToken(credential); setName(jwt.given_name); @@ -363,7 +365,11 @@ function App() { <> {enableAuth && ( - login(credential)} /> + login(credential)} + onClose={() => setOpenAuthDialog(false)} + /> )} - logout()} /> + logout()} + onLogin={() => setOpenAuthDialog(true)} + /> {showAlert() && ( <> diff --git a/src/components/AuthDialog.tsx b/src/components/AuthDialog.tsx index 6a346e49..66877e10 100644 --- a/src/components/AuthDialog.tsx +++ b/src/components/AuthDialog.tsx @@ -1,28 +1,39 @@ import Dialog from '@mui/material/Dialog'; import DialogContent from '@mui/material/DialogContent'; import DialogTitle from '@mui/material/DialogTitle'; +import DialogActions from '@mui/material/DialogActions'; +import Button from '@mui/material/Button'; import useMediaQuery from '@mui/material/useMediaQuery'; import { useTheme } from '@mui/material/styles'; import { GoogleLogin } from '@react-oauth/google'; function AuthDialog({ - isLoggedIn, + open, onAuth, + onClose, }: { - isLoggedIn: boolean; + open: boolean; onAuth: (credential: string | undefined) => void; + onClose: () => void; }) { const theme = useTheme(); const fullScreen = useMediaQuery(theme.breakpoints.down('md')); return ( - - You must log in to a trusted identity provider in order to query! + + + You must log in to a trusted identity provider in order to query all available nodes! + onAuth(response.credential)} /> + + + Close + + ); } diff --git a/src/components/Navbar.tsx b/src/components/Navbar.tsx index 4b457ea4..83246a55 100644 --- a/src/components/Navbar.tsx +++ b/src/components/Navbar.tsx @@ -8,20 +8,27 @@ import { Menu, MenuItem, ListItemIcon, + Tooltip, } from '@mui/material'; -import GitHubIcon from '@mui/icons-material/GitHub'; +import GitHub from '@mui/icons-material/GitHub'; +import Article from '@mui/icons-material/Article'; import Logout from '@mui/icons-material/Logout'; +import Login from '@mui/icons-material/Login'; import Avatar from '@mui/material/Avatar'; import { enableAuth } from '../utils/constants'; function Navbar({ + isLoggedIn, name, profilePic, onLogout, + onLogin, }: { + isLoggedIn: boolean; name: string; profilePic: string; onLogout: () => void; + onLogin: () => void; }) { const [latestReleaseTag, setLatestReleaseTag] = useState(''); const [anchorEl, setAnchorEl] = useState(null); @@ -66,11 +73,13 @@ function Navbar({ - - Documentation - + + + + + - + {enableAuth && ( <> @@ -96,15 +105,26 @@ function Navbar({ - - Logged in as {name} - - - - - - Logout - + {isLoggedIn ? ( + <> + + Logged in as {name} + + + + + + Logout + + > + ) : ( + + + + + Login + + )} > )} diff --git a/src/components/ResultContainer.tsx b/src/components/ResultContainer.tsx index 75d277c8..61b694e3 100644 --- a/src/components/ResultContainer.tsx +++ b/src/components/ResultContainer.tsx @@ -7,7 +7,7 @@ import GetDataDialog from './GetDataDialog'; function ResultContainer({ response }: { response: QueryResponse | null }) { const [download, setDownload] = useState([]); - const [openModal, setOpenModal] = useState(false); + const [openDialog, setOpenDialog] = useState(false); const selectAll: boolean = response ? response.responses.length === download.length && response.responses.every((r) => download.includes(r.dataset_uuid)) @@ -227,12 +227,12 @@ function ResultContainer({ response }: { response: QueryResponse | null }) { setOpenModal(true)} + data-cy="how-to-get-data-dialog-button" + onClick={() => setOpenDialog(true)} > How to get data - setOpenModal(false)} /> + setOpenDialog(false)} />