DAH-2876 feat: allow redirect after sign in #2435
Conversation
cade-exygy
changed the title
cade-exygy
requested review from
a team,
tallulahkay and
jimlin-sfgov
and removed request for
a team
|
@cade-exygy could you try to up the code cov here? |
| @@ -456,7 +457,8 @@ const AccountSettingsPage = () => { | |||
| const { profile, loading, initialStateLoaded } = React.useContext(UserContext) | |||
|
|
|||
| if (!profile && !loading && initialStateLoaded) { | |||
| // TODO: Redirect to React sign in page and show a message that user needs to sign in | |||
| window.sessionStorage.setItem("redirect", "settings") | |||
There was a problem hiding this comment.
thought: We're using sessionStorage across various files, and it's not immediately obvious what arguments for setItem mean. We should look into consolidating all routing-related sessionStorage usage into a single file, probably in routeUtil.ts.
| @@ -62,7 +63,8 @@ const MyAccount = (_props: MyAccountProps) => { | |||
| const { profile, loading, initialStateLoaded } = React.useContext(UserContext) | |||
|
|
|||
| if (!profile && !loading && initialStateLoaded) { | |||
| // TODO: Redirect to React sign in page and show a message that user needs to sign in | |||
| window.sessionStorage.setItem("redirect", "account") | |||
| setSiteAlertMessage(t("signIn.loginRequired"), "secondary") | |||
There was a problem hiding this comment.
thought: We're well along this path now, but in the future we should look into using constants instead of strings for things like "redirect", "account", and "secondary".
| @@ -198,6 +199,8 @@ const MyApplications = () => { | |||
| }, [authLoading, initialStateLoaded, profile]) | |||
|
|
|||
| if (!profile && !authLoading && initialStateLoaded) { | |||
| window.sessionStorage.setItem("redirect", "applications") | |||
| setSiteAlertMessage(t("signIn.loginRequired"), "secondary") | |||
There was a problem hiding this comment.
issue: I think something here needs to be updated to address the following issue: the "You must be signed in..." banner shows up when I click the sign out button
There was a problem hiding this comment.
(I think) This has been fixed in main and the fix just hasn't been pulled into this branch yet
Description
If a user tries to go to the my-account, account-settings, or my-applications and is not signed in: redirect to sign in page, show banner. Once the login happens, the user is redirected to the page they were trying to access prior.
Jira ticket
https://sfgovdt.jira.com/browse/DAH-2876
Checklist before requesting review
Version Control
angularif it contains updates to Angular codetype: TICKET-NUMBER Descriptionformat, e.g.feat: DAH-123 New FeatureCode quality
Review instructions
Request review
needs reviewlabelHousing Enggroup to automatically assign reviewers, and/or assign specific engineersReview Instructions
Test edge cases like signing out, triggering other banners, etc