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

ui: Enable recovery from an unreachable datacenter (500 error) #7404

Merged
merged 2 commits into from
Mar 9, 2020

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Mar 6, 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 URL 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.

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.

Acceptance tests included here to replicate the original bug report.

cc @lkysow

@johncowen johncowen added type/bug Feature does not function as expected theme/ui Anything related to the UI backport/1.7 labels Mar 6, 2020
@johncowen johncowen added this to the 1.7.x milestone Mar 6, 2020
@johncowen johncowen requested a review from a team March 6, 2020 12:41
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.
@johncowen
Copy link
Contributor Author

Still looking into the failing tests here, will re-ping once we are happy

@johncowen
Copy link
Contributor Author

@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
@johncowen johncowen changed the title ui: Enable recovery from a unreachable datacenter (500 error) ui: Enable recovery from an unreachable datacenter (500 error) Mar 6, 2020
Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

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

LGTM

@johncowen johncowen merged commit 5f62566 into master Mar 9, 2020
@johncowen johncowen deleted the ui/bugfix/500-dc-recovery branch March 9, 2020 09:10
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
@johncowen johncowen modified the milestones: 1.7.x, 1.7.2 Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants