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

Original url destinations like experiment detail page will not redirect to home after login #1143

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

danoswaltCL
Copy link
Collaborator

#1137

Most routes will redirect to /home if you click on a shared url such as an experiment detail page like /home/detail/abc123... because the original destination url gets lost after navigating to /login.

This should allow one to open up a shared link, login if needed, and return to the intended destination page.

@bcb37 bcb37 self-requested a review November 21, 2023 17:18
@bcb37 bcb37 self-assigned this Nov 21, 2023
user: null,
googleCredential: null,
redirectUrl: '/home',
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to set any attributes (besides redirectUrl) here, to test that they get set to the default values when clearState is called? The other two attributes in 'settings', for instance, aren't set here. If we do set them, it might be a more definitive test if they're initially set to different values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i updated the specs to make this a little more explicitly tested

@@ -25,6 +32,13 @@ describe('clearState', () => {
toCheckAuth: null,
toFilterMetric: null,
},
auth: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the 'it' string read 'should reset settings and auth state...'?

@danoswaltCL danoswaltCL merged commit be88797 into dev Nov 21, 2023
8 checks passed
@danoswaltCL danoswaltCL deleted the feature/1137-shareable-urls-no-redirect-to-home branch November 21, 2023 21:43
@ppratikcr7 ppratikcr7 linked an issue Nov 22, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug (Enhancement?): Shared URLs should not always redirect to /home
2 participants