-
Notifications
You must be signed in to change notification settings - Fork 1
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
[MPDX-8386] Redirect after login #1168
Conversation
Preview branch generated at https://8386-login-redirect.d3dytjb8adxkk5.amplifyapp.com |
Bundle sizes [mpdx-react]Compared against 1c203d1 No significant changes found |
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.
This looks great! Should we automattically start to log users in when they are redirected to the login screen?
signOut({ redirect: true, callbackUrl: 'signOut' }).then(() => { | ||
clearDataDogUser(); | ||
client.clearStore(); | ||
graphQLErrors?.forEach((graphQLError) => { |
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.
Why are we removing this window.location.pathname !== '/login'
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.
The Apollo client is no longer present on the login page, so it is impossible to receive GraphQL errors on the login page.
Sorry, but I'm not sure what you mean. |
@dr-bizz Do you mean adjusting this useEffect(() => {
if (immediateSignIn) {
signIn(signInAuthProviderId);
}
}, []); |
Sorry, I can add more context. When we redirect users from the page they were on to the login page, we shouldn't add another click to the user, we should start to log the users in as soon as they land on the login. They will need to add their login details. Yes, It's using the |
The Apollo client is no longer present on the login page, so it is impossible to receive GraphQL errors on the login page.
25147a1
to
6626050
Compare
Description
Redirect the user back to the page they were trying to go to after they successfully log in.
Looking at the redirect trace in the DevTools network tab (select preserve log, filter by Doc to see page loads, and look at the
Location
response header), it appears that after sign in, NextAuth, redirects back to the page that initiated the login, in our case,/login?redirect=xxx
. OurgetServerSideProps
function on the login page notices that we have a session and a redirect URL and redirects the user one last time to their original page.Testing
?redirect
is set to the tasks page.MPDX-8386
Checklist: