-
Notifications
You must be signed in to change notification settings - Fork 65
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
Upgrade react-router to v6 #3261
Changes from 21 commits
3c977e2
1f24239
0cda207
5007567
86e9268
6bb30ac
7997852
b57c3c4
3daf66c
4fe9c77
713319f
b60ef2d
45db92a
3a5e30c
414ab98
93a2988
d0ad467
ce65cdc
66a434d
a2e0772
33c7d5f
20256f8
6a3a4ab
fda9951
294d217
594b6b6
7d43416
008ed0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import React, { useEffect } from "react"; | ||
import { gql, useQuery } from "@apollo/client"; | ||
import { useDispatch, connect } from "react-redux"; | ||
import { Redirect, Route, Switch } from "react-router-dom"; | ||
import { Navigate, Route, Routes, useLocation } from "react-router-dom"; | ||
import { ApplicationInsights } from "@microsoft/applicationinsights-web"; | ||
|
||
import ProtectedRoute from "./commonComponents/ProtectedRoute"; | ||
|
@@ -50,13 +50,14 @@ export const WHOAMI_QUERY = gql` | |
const App = () => { | ||
const appInsights = getAppInsights(); | ||
const dispatch = useDispatch(); | ||
const location = useLocation(); | ||
|
||
// Check if the user is logged in, if not redirect to Okta | ||
if (process.env.REACT_APP_OKTA_ENABLED === "true") { | ||
const accessToken = localStorage.getItem("access_token"); | ||
if (!accessToken) { | ||
// If Okta login has been attempted and returned to SR with an error, don't redirect back to Okta | ||
const params = new URLSearchParams(window.location.hash.slice(1)); | ||
const params = new URLSearchParams(location.hash.slice(1)); | ||
if (params.get("error")) { | ||
throw new Error( | ||
params.get("error_description") || "Unknown Okta error" | ||
|
@@ -118,100 +119,120 @@ const App = () => { | |
let homepagePath: string; | ||
|
||
if (isSupportAdmin) { | ||
homepagePath = "/admin"; | ||
homepagePath = "admin"; | ||
} else if (isOrgAdmin) { | ||
homepagePath = "/dashboard"; | ||
homepagePath = "dashboard"; | ||
} else { | ||
homepagePath = "/queue"; | ||
homepagePath = "queue"; | ||
} | ||
|
||
return ( | ||
<> | ||
<VersionEnforcer /> | ||
<MaintenanceBanner /> | ||
{process.env.REACT_APP_DISABLE_MAINTENANCE_BANNER === "true" ? null : ( | ||
<MaintenanceBanner /> | ||
)} | ||
{process.env.REACT_APP_IS_TRAINING_SITE === "true" && ( | ||
<TrainingNotification /> | ||
)} | ||
<WithFacility> | ||
<Page> | ||
<Header /> | ||
<Switch> | ||
<Route | ||
path="/queue" | ||
render={() => { | ||
return <TestQueueContainer />; | ||
}} | ||
/> | ||
<Routes> | ||
<Route | ||
path="/" | ||
exact | ||
render={({ location }) => ( | ||
<Redirect | ||
to={{ | ||
...location, | ||
pathname: homepagePath, | ||
}} | ||
element={ | ||
<Navigate | ||
to={{ pathname: homepagePath, search: location.search }} | ||
/> | ||
)} | ||
} | ||
/> | ||
<ProtectedRoute | ||
path="/results/:page" | ||
render={({ match }: any) => { | ||
return <TestResultsList pageNumber={match.params.page} />; | ||
}} | ||
requiredPermissions={appPermissions.results.canView} | ||
userPermissions={data.whoami.permissions} | ||
<Route path="queue" element={<TestQueueContainer />} /> | ||
<Route | ||
path="results/:pageNumber" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.results.canView} | ||
userPermissions={data.whoami.permissions} | ||
element={<TestResultsList />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path="/results/" | ||
render={() => <CleanTestResultsList />} | ||
requiredPermissions={appPermissions.results.canView} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="results" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.results.canView} | ||
userPermissions={data.whoami.permissions} | ||
element={<CleanTestResultsList />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path={`/patients/:page?`} | ||
render={({ match }: any) => { | ||
return <ManagePatientsContainer page={match.params.page} />; | ||
}} | ||
requiredPermissions={appPermissions.people.canView} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="patients/:pageNumber" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.people.canView} | ||
userPermissions={data.whoami.permissions} | ||
element={<ManagePatientsContainer />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path={`/patient/:patientId`} | ||
render={({ match }: any) => ( | ||
<EditPatientContainer patientId={match.params.patientId} /> | ||
)} | ||
requiredPermissions={appPermissions.people.canEdit} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="patients" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.people.canView} | ||
userPermissions={data.whoami.permissions} | ||
element={<ManagePatientsContainer />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path={`/add-patient/`} | ||
render={() => <AddPatient />} | ||
requiredPermissions={appPermissions.people.canEdit} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="patient/:patientId" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.people.canEdit} | ||
userPermissions={data.whoami.permissions} | ||
element={<EditPatientContainer />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path="/settings" | ||
component={Settings} | ||
requiredPermissions={appPermissions.settings.canView} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="add-patient" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.people.canEdit} | ||
userPermissions={data.whoami.permissions} | ||
element={<AddPatient />} | ||
/> | ||
} | ||
/> | ||
<ProtectedRoute | ||
path="/dashboard" | ||
component={Analytics} | ||
requiredPermissions={appPermissions.settings.canView} | ||
userPermissions={data.whoami.permissions} | ||
<Route | ||
path="settings/*" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.settings.canView} | ||
userPermissions={data.whoami.permissions} | ||
element={<Settings />} | ||
/> | ||
} | ||
/> | ||
<Route | ||
path={"/admin"} | ||
render={({ match }) => ( | ||
<SupportAdminRoutes | ||
match={match} | ||
isAdmin={data.whoami.isAdmin} | ||
path="dashboard" | ||
element={ | ||
<ProtectedRoute | ||
requiredPermissions={appPermissions.settings.canView} | ||
userPermissions={data.whoami.permissions} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: we may want to extract this data.whoami.permissions out to a variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above - moving to a shorter named const seemed to cause issues, so I reverted |
||
element={<Analytics />} | ||
/> | ||
)} | ||
} | ||
/> | ||
<Route | ||
path="admin/*" | ||
element={<SupportAdminRoutes isAdmin={data.whoami.isAdmin} />} | ||
/> | ||
</Switch> | ||
</Routes> | ||
</Page> | ||
</WithFacility> | ||
</> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,13 @@ | ||
import React from "react"; | ||
import { Route } from "react-router-dom"; | ||
import { Route, Routes } from "react-router-dom"; | ||
|
||
const HealthChecks: React.FC<{}> = ({ match }: any) => ( | ||
<> | ||
<Route path={match.url + "/ping"} render={() => <div>pong</div>} /> | ||
const HealthChecks = () => ( | ||
<Routes> | ||
<Route path="ping" element={<div>pong</div>} /> | ||
<Route | ||
path={match.url + "/commit"} | ||
render={() => <div>{process.env.REACT_APP_CURRENT_COMMIT}</div>} | ||
path="commit" | ||
element={<div>{process.env.REACT_APP_CURRENT_COMMIT}</div>} | ||
/> | ||
</> | ||
</Routes> | ||
); | ||
|
||
export default HealthChecks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's worth defining this protected route as a variable, since it's repeated under the regular
patients
route too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah not a bad idea! The reason I had to duplicate this route is because v5 supported optional URL params like
/patients/:pageNumber?
which would match both/patients
and/patients/1
, but v6 no longer supports that. Hence the need for two routes. But I will definitely DRY it upThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Soooo I tried to do this, and for some reason it broke the patients page. It kept throwing "The operation is insecure", and I couldn't figure out why 🤔 so I reverted for now.