-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ui: Enable recovery from an unreachable datacenter (500 error) #7404
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
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1eb1b4f
to
5b0893a
Compare
For URL maintenance reasons we store the last visited DC in localStorage incase you come back to a page (for example settings) that doesn't have a dc in the URL. A problem arises here if the last DC you tried to visit is unreachable. The first fix here clears out the last visited DC from localStorage if the API has errored out. Secondly, our `href-mut` helper which mutates the current current and replaces 'parts' in the URL rather than the whole thing functioned by detecting the current route/URL you are on an 'mutating' that. A problem arose here as even though you might be on the `/ui/dc-1/services` URL the actual route is the 'error' route which does not have a URL that can be changed properly. The second fix here changes the arguments that href-mut accepts to `{{href-mut fallbackRouteName, hashOfThingsToReplace}}`. If the route is the error route it will use the fallbackRouteName. Although it seems strange we decided to put the fallbackRoute as the first argument so it looks similar to the normal `{{href-to}}` helper. Acceptance tests included here to replicate the original bug report.
5b0893a
to
91b2952
Compare
Still looking into the failing tests here, will re-ping once we are happy |
@kaxcode looks like this is ready for another look! |
When `router.currentRouteName === 'error'` then `router.currentRoute.name === Name Of Route Before It Errored` it seems
24b5ab8
to
a359906
Compare
kaxcode
approved these changes
Mar 6, 2020
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
alvin-huang
pushed a commit
that referenced
this pull request
Mar 9, 2020
For URL maintenance reasons we store the last visited DC in localStorage incase you come back to a page (for example settings) that doesn't have a dc in the URL. A problem arises here if the last DC you tried to visit is unreachable. The first fix here clears out the last visited DC from localStorage if the API has errored out. Secondly, our `href-mut` helper which mutates the current current and replaces 'parts' in the URL rather than the whole thing functioned by detecting the current route/URL you are on an 'mutating' that. A problem arose here as even though you might be on the `/ui/dc-1/services` URL the actual route is the 'error' route which does not have a URL that can be changed properly. The second fix here uses route.currentRoute.name over route.currentRouteName. The latter is equal to error when an error occurs whereas the former gives you the name of the route before the error happened, which is actually what we want/the intent here. ie. when `router.currentRouteName === 'error'` then `router.currentRoute.name === Name Of Route Before It Errored` it seems
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For URL maintenance reasons we store the last visited DC in
localStorage incase you come back to a page (for example settings) that
doesn't have a dc in the URL.
A problem arises here if the last DC you tried to visit is unreachable.
The first fix here clears out the last visited DC from localStorage if
the API has errored out.
Secondly, ourhref-mut
helper which mutates the current URL andreplaces 'parts' in the URL rather than the whole thing functioned by
detecting the current route/URL you are on an 'mutating' that. A problem
arose here as even though you might be on the
/ui/dc-1/services
URL theactual route is the 'error' route which does not have a URL that can be
changed properly.
The second fix here changes the arguments that href-mut accepts to
{{href-mut fallbackRouteName, hashOfThingsToReplace}}
. If the route is theerror route it will use the fallbackRouteName. Although it seems strange
we decided to put the fallbackRoute as the first argument so it looks
similar to the normal
{{href-to}}
helper.The second fix here uses
route.currentRoute.name
overroute.currentRouteName
.The latter is equal to
error
when an error occurs whereas the former gives you the name of the route before the error happened, which is actually what we want/the intent here.Acceptance tests included here to replicate the original bug report.
cc @lkysow