Ui/small replication action fixes by Monkeychip · Pull Request #9387 · hashicorp/vault

Monkeychip · 2020-07-02T18:07:46Z

This PR fixes the following issues:

Fix breadcrumb link. If a DR cluster was not enabled, the breadcrumb link on the landing page did not work. I changed it so now if replication is not enabled the link takes you back to the index page vault.cluster.replication.index. I also changed the breadcrumb label to "Replication" whereas before it was Disaster Recovery. This label change only happens if the DR is not enabled.

Another breadcrumb change. Before when you were on the manage page and you clicked the breadcrumb it was trying to redirect you to the manage page. I confirmed with Design that in the case of a DR Secondary, there should be no breadcrumb because you only have two routes in this state: manage and details, and toggle back and forth between those with a breadcrumb didn't make much sense. Here's an image of a DR secondary, with no breadcrumb.

We had two situations where the transition needed some help (no errors just look/feel fixes). The first fix was to add a black nav during the initialization of a secondary DR cluster. To reproduce, add a secondary DR cluster. Before it showed only a loader. Now you can see a black nav bar above the loader. (in the gif, I wait for the network request so that the state of the loader is on the screen the longest).

The second transition issue occurred after you enabled a DR secondary and then demoted that same DR. This was fixed by adding a new property to the cluster to capture the drMode. I then saved the initial drMode when the route's model was generated and set it on a property called drModeInit. Because the mode changes before any transition, to capture the change (computed properties did not work here), I compared the saved drModeInit property to the drMode property in the template. This situation only occurs when the drMode goes from primary to secondary (e.g. during demotion). Once the transition is complete the model is reset and the loader will go away.

*note, not very often the drMode will be 'disabled' and not 'primary' during the transition. In this case, the loader will not show. However, because this is rare, and I can't really seem to understand under which circumstance this happens. I feel better about not trying to capture this particular situation and be ok with the occasional flash of template screen and no loader. My feelings are that this is a feel/look fix, and if the fix is wrong the user could end up seeing a loader and not getting out of it, and that's a much worse situation than fixing a flash of the template page.

Here is a gif with a debugger so you can see the two properties.

Here is a gif without the debugger so you can see how it looks.

The last fix was to prevent the super slow modal issues we were seeing after demoting a cluster. This issue was happening because the shamir-modal was not found as it was not in the addon component library. The fix was to move the shamir-modal and shamir-modal-flow into the addon components so it could be accessed from the replication engine.

…k. Now if DR not enabled, the breadcrumb says replication and links back to rep index.

…d for menu items and because NavHeader component and the icon live in the app and not addons I cannot access them without moving them. I figured the black bar was enough, and it wasn't worth moving just for that

…what page they are currently on (details or manage). Before the breadcrumb link didn't do anything if they were on the manage page

…odal not being in the addon engine and erroring out.

…perty on cluster and compare the mode of the dr which changes from primary to secondary during demotion. If dr mode changes, showing loading status

…ing, but only on demote

…r in the dom

Monkeychip · 2020-07-02T18:47:10Z

    <ReplicationPage @model={{model}} as |Page|>
      <Page.header
        @showTabs={{true}}
+        @pageType="manage"


used to determine on manage page or not, and if you are change the link the breadcrumb uses.

chelshaw

A couple pending questions, but generally the changes look good! Awesome work figuring out the navbar disappearing act 👏

chelshaw · 2020-07-02T18:49:34Z

+            {{/link-to}}
+          {{else}}
+            {{#if (eq pageType "manage")}}
+              {{#link-to "vault.cluster.replication-dr-promote.details"}}


A little curious why this link wouldn't go back to general Replication dashboard (vault.cluster.replication.index) instead. Or at the very least, maybe naming the breadcrumbs Disaster Recovery Details and Disaster Recovery Manage to be more indicative of what it's doing.

Good thoughts. Amending my previous response. This is because it doesn't exist. We're a DR secondary and the index replication is no longer around. See gif below. There are only two routes when you're a DR secondary (manage or details).

I could see us removing the breadcrumbs entirely, but honestly, I think it's nice to have them. I'll check with design later today.

Confirmed with design that we'll remove breadcrumbs if it's a DR Secondary.

Monkeychip · 2020-07-06T17:07:53Z

 <PageHeader as |p|>
  <p.top>
-    {{#if (not isSummaryDashboard) }}
+    {{#if (not (or isSummaryDashboard isSecondary)) }}


If is drSecondary then don't show breadcrumb, there is a conditional higher up that is making sure this is DR.

Monkeychip · 2020-07-06T17:11:37Z

          </span>
-          {{#link-to "vault.cluster.replication-dr-promote"}}
-            Disaster Recovery
+          {{#link-to "vault.cluster.replication.index"}}


The only time a breadcrumb shows on the route ui/vault/replication-dr-promote/details or the ui/vault/replication-dr-promote/ is when there is no DR Secondary and you land on the empty state component. In this case. you can use the breadcrumb to navigate back to the replication index page which lets you setup replication. See image here:

Monkeychip · 2020-07-06T17:16:01Z

-      <PageHeader as |p|>
-        <p.top>
-          <nav class="key-value-header breadcrumb">
+    {{#if (and (eq model.drMode "secondary") (eq model.drModeInit "primary"))}}


The rest of this is indentation changes due to wrapping everything in conditional

andaley · 2020-07-07T19:03:57Z

 <section class="section">
  <div class="container is-widescreen">
    {{#if model.replicationIsInitializing }}
+      <nav class="navbar"></nav>


nice fix! ✨

andaley

this is fantastic. thank you for including such thorough descriptions and screenshots. code looks solid all around, too. nice work!

* fix issue where if dr is not enabled, the breadcrumb link did not work. Now if DR not enabled, the breadcrumb says replication and links back to rep index. * show black nav when cluster is not initialized and is loading, no need for menu items and because NavHeader component and the icon live in the app and not addons I cannot access them without moving them. I figured the black bar was enough, and it wasn't worth moving just for that * conditional change the breadcrumb link in Disaster Recovery based on what page they are currently on (details or manage). Before the breadcrumb link didn't do anything if they were on the manage page * fix slow modal loading after demoting a dr secondary. due to shamir modal not being in the addon engine and erroring out. * to prevent confusing transition state during dr demotion, set new property on cluster and compare the mode of the dr which changes from primary to secondary during demotion. If dr mode changes, showing loading status * get more specific about conditional so loader does not some on disabling, but only on demote * remove concurrency from onSubmit * revert all concurency, I think this is solved by the removal of shamir in the dom * reverse order * cleanup * forgot that tricky layout, hopefull this will fix test * remove page container, it's not needed * remove breadcrumbs if DR secondary * remove pageType no now longer using * remove conditional that is no longer hit

Monkeychip added 9 commits June 29, 2020 10:08

fix issue where if dr is not enabled, the breadcrumb link did not wor…

46bbd8e

…k. Now if DR not enabled, the breadcrumb says replication and links back to rep index.

conditional change the breadcrumb link in Disaster Recovery based on …

7712215

…what page they are currently on (details or manage). Before the breadcrumb link didn't do anything if they were on the manage page

fix slow modal loading after demoting a dr secondary. due to shamir m…

b58b5c3

…odal not being in the addon engine and erroring out.

to prevent confusing transition state during dr demotion, set new pro…

ca90536

…perty on cluster and compare the mode of the dr which changes from primary to secondary during demotion. If dr mode changes, showing loading status

get more specific about conditional so loader does not some on disabl…

ae0d2af

…ing, but only on demote

remove concurrency from onSubmit

ba13043

revert all concurency, I think this is solved by the removal of shami…

5797ec8

…r in the dom

reverse order

7c0a691

Monkeychip added the ui label Jul 2, 2020

Monkeychip added this to the 1.5 milestone Jul 2, 2020

Monkeychip added 2 commits July 2, 2020 12:38

cleanup

e0f0546

Merge branch 'master' into ui/small-replication-action-fixes

12679a3

Monkeychip commented Jul 2, 2020

View reviewed changes

chelshaw reviewed Jul 2, 2020

View reviewed changes

Monkeychip added 5 commits July 2, 2020 13:14

forgot that tricky layout, hopefull this will fix test

d201b52

remove page container, it's not needed

3e6e998

remove breadcrumbs if DR secondary

bd9d44f

remove pageType no now longer using

5906c8b

remove conditional that is no longer hit

2df958e

Monkeychip commented Jul 6, 2020

View reviewed changes

Merge branch 'master' into ui/small-replication-action-fixes

76bf47f

Monkeychip requested a review from andaley July 6, 2020 21:04

andaley reviewed Jul 7, 2020

View reviewed changes

andaley approved these changes Jul 7, 2020

View reviewed changes

Monkeychip merged commit 4e73f1b into master Jul 7, 2020

Monkeychip deleted the ui/small-replication-action-fixes branch July 7, 2020 19:09

Monkeychip mentioned this pull request Jul 7, 2020

Backport 1.5.x: Ui/small replication action fixes (#9387) #9412

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ui/small replication action fixes#9387

Ui/small replication action fixes#9387
Monkeychip merged 17 commits into
masterfrom
ui/small-replication-action-fixes

Monkeychip commented Jul 2, 2020 •

edited

Loading

Uh oh!

Monkeychip Jul 2, 2020

Uh oh!

chelshaw left a comment

Uh oh!

Uh oh!

chelshaw Jul 2, 2020

Uh oh!

Monkeychip Jul 2, 2020 •

edited

Loading

Uh oh!

Monkeychip Jul 6, 2020

Uh oh!

Monkeychip Jul 6, 2020

Uh oh!

Monkeychip Jul 6, 2020

Uh oh!

Monkeychip Jul 6, 2020

Uh oh!

andaley Jul 7, 2020

Uh oh!

andaley left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Monkeychip commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Monkeychip Jul 2, 2020

Choose a reason for hiding this comment

Uh oh!

chelshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chelshaw Jul 2, 2020

Choose a reason for hiding this comment

Uh oh!

Monkeychip Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Monkeychip Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

Monkeychip Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

Monkeychip Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

Monkeychip Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

andaley Jul 7, 2020

Choose a reason for hiding this comment

Uh oh!

andaley left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Monkeychip commented Jul 2, 2020 •

edited

Loading

Monkeychip Jul 2, 2020 •

edited

Loading