-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Navlink url tracker: Reset navlink on failed redirect #61460
Conversation
@elasticmachine merge upstream |
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.
tested on Chrome locally, works as expected. Code LGTM.
Is this fixes original issue described in #59999 ? |
@Dosant I couldn't reproduce or find another issue that could cause the original description, I assumed it was a variant of that problem. Are you fine with closing this issue and open again when the problem pops up in another context? cc @streamich did you run into this recently (since this issues got reported)? |
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.
LGTM, tested using example flow in the description.
Don't know if this case is related and if this is also a regression in 7.7:
- Go to visualize, save vis
- Go to management / saved objects - delete that vis
- Link in nabber still point to that deleted saved object. clicking it will just redirect me to listing page with a "missing" toast.
src/plugins/kibana_utils/public/history/redirect_when_missing.tsx
Outdated
Show resolved
Hide resolved
Go to visualize, save vis That's a good point, just tested with 7.4 and it behaves the same there. I'm not sure whether that qualifies as a bug though, it's at least consistent. |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/discover/_field_visualize·ts.discover app discover field visualize button should preserve app filters in visualizeStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
@kertal See the flaky test above - do you think this is related to the recent changes in that area? |
Fixes #59999
I looked through the code and one scenario that leads to a redirect-loop is that when a missing saved object or is causing a redirect to management, it will still be tracked as the last active url.
Example:
This PR solves this loop by resetting the navlink url to the default when a redirect to outside of the current app is happening - in the scenario from above the second click to Visualize will take you to the listing page instead.
I found 3 cases where this could happen: