UI: Update behavior when deleting nested secret from list (#26845)

* Update error states on secret list template

* Remove usage of navToNearestAncestor mixin

* don't throw error on list when 404

* Update test with expected behavior

* cleanup

* Add changelog

changelog/26845.txt

@@ -0,0 +1,3 @@
+```release-note:change
+ui: deleting a nested secret will no longer redirect you to the nearest path segment
+```

ui/app/controllers/vault/cluster/secrets/backend/list.js

@@ -8,11 +8,10 @@ import { computed } from '@ember/object';
 import { service } from '@ember/service';
 import Controller from '@ember/controller';
 import BackendCrumbMixin from 'vault/mixins/backend-crumb';
-import WithNavToNearestAncestor from 'vault/mixins/with-nav-to-nearest-ancestor';
 import ListController from 'core/mixins/list-controller';
 import { keyIsFolder } from 'core/utils/key-utils';
 
-export default Controller.extend(ListController, BackendCrumbMixin, WithNavToNearestAncestor, {
+export default Controller.extend(ListController, BackendCrumbMixin, {
   flashMessages: service(),
   queryParams: ['page', 'pageFilter', 'tab'],
 
@@ -52,16 +51,13 @@ export default Controller.extend(ListController, BackendCrumbMixin, WithNavToNea
         });
     },
 
-    delete(item, type) {
+    delete(item) {
       const name = item.id;
       item
         .destroyRecord()
         .then(() => {
           this.flashMessages.success(`${name} was successfully deleted.`);
           this.send('reload');
-          if (type === 'secret') {
-            this.navToNearestAncestor.perform(name);
-          }
         })
         .catch((e) => {
           const error = e.errors ? e.errors.join('. ') : e.message;

ui/app/mixins/with-nav-to-nearest-ancestor.js

@@ -1,45 +0,0 @@
-/**
- * Copyright (c) HashiCorp, Inc.
- * SPDX-License-Identifier: BUSL-1.1
- */
-
-import Mixin from '@ember/object/mixin';
-import { task } from 'ember-concurrency';
-import { ancestorKeysForKey } from 'core/utils/key-utils';
-
-// This mixin is currently used in a controller and a component, but we
-// don't see cancellation of the task as the while loop runs in either
-
-// Controller in Ember are singletons so there's no cancellation there
-// during the loop. For components, it might be expected that the task would
-// be cancelled when we transitioned to a new route and a rerender occured, but this is not
-// the case since we are catching the error. Since Ember's route transitions are lazy
-// and we're catching any 404s, the loop continues until the transtion succeeds, or exhausts
-// the ancestors array and transitions to the root
-export default Mixin.create({
-  navToNearestAncestor: task(function* (key) {
-    const ancestors = ancestorKeysForKey(key);
-    let errored = false;
-    let nearest = ancestors.pop();
-    while (nearest) {
-      try {
-        const transition = this.transitionToRoute('vault.cluster.secrets.backend.list', nearest);
-        transition.data.isDeletion = true;
-        yield transition.promise;
-      } catch (e) {
-        // in the route error event handler, we're only throwing when it's a 404,
-        // other errors will be in the route and will not be caught, so the task will complete
-        errored = true;
-        nearest = ancestors.pop();
-      } finally {
-        if (!errored) {
-          nearest = null;
-          // eslint-disable-next-line
-          return;
-        }
-        errored = false;
-      }
-    }
-    yield this.transitionToRoute('vault.cluster.secrets.backend.list-root');
-  }),
-});

ui/app/routes/vault/cluster/secrets/backend/list.js

@@ -15,6 +15,20 @@ import { pathIsDirectory } from 'kv/utils/kv-breadcrumbs';
 
 const SUPPORTED_BACKENDS = supportedSecretBackends();
 
+function getValidPage(pageParam) {
+  if (typeof pageParam === 'number') {
+    return pageParam;
+  }
+  if (typeof pageParam === 'string') {
+    try {
+      return parseInt(pageParam, 10) || 1;
+    } catch (e) {
+      return 1;
+    }
+  }
+  return 1;
+}
+
 export default Route.extend({
   store: service(),
   templateName: 'vault/cluster/secrets/backend/list',
@@ -119,7 +133,7 @@ export default Route.extend({
           id: secret,
           backend,
           responsePath: 'data.keys',
-          page: params.page || 1,
+          page: getValidPage(params.page),
           pageFilter: params.pageFilter,
         })
         .then((model) => {
@@ -127,8 +141,7 @@ export default Route.extend({
           return model;
         })
         .catch((err) => {
-          // if we're at the root we don't want to throw
+          if (backendModel && err.httpStatus === 404) {
-          if (backendModel && err.httpStatus === 404 && secret === '') {
             return [];
           } else {
             // else we're throwing and dealing with this in the error action
@@ -189,12 +202,6 @@ export default Route.extend({
       /* eslint-disable-next-line ember/no-controller-access-in-routes */
       const hasModel = this.controllerFor(this.routeName).hasModel;
 
-      // this will occur if we've deleted something,
-      // and navigate to its parent and the parent doesn't exist -
-      // this if often the case with nested keys in kv-like engines
-      if (transition.data.isDeletion && is404) {
-        throw error;
-      }
       set(error, 'secret', secret);
       set(error, 'isRoot', true);
       set(error, 'backend', backend);

ui/app/templates/vault/cluster/secrets/backend/list.hbs

@@ -13,39 +13,37 @@
 {{#let (options-for-backend this.backendType this.tab) as |options|}}
   {{#if (or this.model.meta.total (not this.isConfigurableTab))}}
     <Toolbar>
-      {{#if this.model.meta.total}}
+      <ToolbarFilters>
-        <ToolbarFilters>
+        <NavigateInput
-          <NavigateInput
+          @enterpriseProduct="vault"
-            @enterpriseProduct="vault"
+          @filterFocusDidChange={{action "setFilterFocus"}}
-            @filterFocusDidChange={{action "setFilterFocus"}}
+          @filterDidChange={{action "setFilter"}}
-            @filterDidChange={{action "setFilter"}}
+          @filter={{this.filter}}
-            @filter={{this.filter}}
+          @filterMatchesKey={{this.filterMatchesKey}}
-            @filterMatchesKey={{this.filterMatchesKey}}
+          @firstPartialMatch={{this.firstPartialMatch}}
-            @firstPartialMatch={{this.firstPartialMatch}}
+          @baseKey={{get this.baseKey "id"}}
-            @baseKey={{get this.baseKey "id"}}
+          @shouldNavigateTree={{options.navigateTree}}
-            @shouldNavigateTree={{options.navigateTree}}
+          @placeholder={{options.searchPlaceholder}}
-            @placeholder={{options.searchPlaceholder}}
+          @mode={{if (eq this.tab "cert") "secrets-cert" "secrets"}}
-            @mode={{if (eq this.tab "cert") "secrets-cert" "secrets"}}
+        />
-          />
+        {{#if this.filterFocused}}
-          {{#if this.filterFocused}}
+          {{#if this.filterMatchesKey}}
-            {{#if this.filterMatchesKey}}
+            {{#unless this.filterIsFolder}}
-              {{#unless this.filterIsFolder}}
-                <p class="input-hint">
-                  <kbd>Enter</kbd>
-                  to view
-                  {{this.filter}}
-                </p>
-              {{/unless}}
-            {{/if}}
-            {{#if this.firstPartialMatch}}
               <p class="input-hint">
-                <kbd>Tab</kbd>
+                <kbd>Enter</kbd>
-                to autocomplete
+                to view
+                {{this.filter}}
               </p>
-            {{/if}}
+            {{/unless}}
           {{/if}}
-        </ToolbarFilters>
+          {{#if this.firstPartialMatch}}
-      {{/if}}
+            <p class="input-hint">
+              <kbd>Tab</kbd>
+              to autocomplete
+            </p>
+          {{/if}}
+        {{/if}}
+      </ToolbarFilters>
 
       <ToolbarActions>
         <ToolbarSecretLink
@@ -80,33 +78,28 @@
         />
       {{/let}}
     {{else}}
-      <div class="box is-sideless">
+      {{! Empty state here means that a filter was applied on top of the results list }}
-        {{#if this.filterFocused}}
+      <EmptyState
-          There are no
+        @title='There are no {{pluralize options.item}} matching "{{this.filter}}". {{if
-          {{pluralize options.item}}
+          this.filterFocused
-          matching
+          'Press ENTER to add one.'
-          <code>{{this.filter}}</code>, press
+        }}'
-          <kbd>ENTER</kbd>
+      />
-          to add one.
+
-        {{else}}
-          There are no
-          {{pluralize options.item}}
-          matching
-          <code>{{this.filter}}</code>.
-        {{/if}}
-      </div>
     {{/each}}
 
-    <Hds::Pagination::Numbered
+    {{#if this.model.length}}
-      @currentPage={{this.model.meta.currentPage}}
+      {{! Only show pagination when there are results on the page }}
-      @currentPageSize={{this.model.meta.pageSize}}
+      <Hds::Pagination::Numbered
-      @route={{concat "vault.cluster.secrets.backend.list" (unless this.baseKey.id "-root")}}
+        @currentPage={{this.model.meta.currentPage}}
-      @model={{this.backendModel.id}}
+        @currentPageSize={{this.model.meta.pageSize}}
-      @showSizeSelector={{false}}
+        @route={{concat "vault.cluster.secrets.backend.list" (unless this.baseKey.id "-root")}}
-      @totalItems={{this.model.meta.total}}
+        @model={{this.backendModel.id}}
-      @queryFunction={{this.paginationQueryParams}}
+        @showSizeSelector={{false}}
-    />
+        @totalItems={{this.model.meta.total}}
-
+        @queryFunction={{this.paginationQueryParams}}
+      />
+    {{/if}}
   {{else}}
     {{#if (eq this.baseKey.id "")}}
       {{#if (and options.firstStep (not this.tab))}}
@@ -124,10 +117,20 @@
         <EmptyState
           @title={{if
             (eq this.filter this.baseKey.id)
-            (concat "No " (pluralize options.item) " under &quot;" this.filter "&quot;")
+            (concat "No " (pluralize options.item) ' under "' this.filter '".')
-            (concat "No folders matching &quot;" this.filter "&quot;")
+            (concat 'No folders matching "' this.filter '".')
           }}
-        />
+        >
+          <LinkTo @route="vault.cluster.secrets.backend.list-root" @model={{this.backend}} data-test-list-root-link>
+            Back to root
+          </LinkTo>
+        </EmptyState>
+      {{else}}
+        <EmptyState @title={{(concat "No " (pluralize options.item) ' matching "' this.filter '".')}}>
+          <LinkTo @route="vault.cluster.secrets.backend.list-root" @model={{this.backend}} data-test-list-root-link>
+            Back to root
+          </LinkTo>
+        </EmptyState>
       {{/if}}
     {{/if}}
   {{/if}}

ui/tests/acceptance/secrets/backend/kv/secret-test.js

@@ -19,6 +19,7 @@ import { writeSecret, writeVersionedSecret } from 'vault/tests/helpers/kv/kv-run
 import { runCmd } from 'vault/tests/helpers/commands';
 import { PAGE } from 'vault/tests/helpers/kv/kv-selectors';
 import codemirror from 'vault/tests/helpers/codemirror';
+import { GENERAL } from 'vault/tests/helpers/general-selectors';
 
 const deleteEngine = async function (enginePath, assert) {
   await logout.visit();
@@ -157,31 +158,28 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
     // https://github.com/hashicorp/vault/issues/5960
     test('version 1: nested paths creation maintains ability to navigate the tree', async function (assert) {
       const enginePath = this.backend;
-      const secretPath = '1/2/3/4';
+      await runCmd([
-      await listPage.create();
+        `write ${enginePath}/1/2 foo=bar`,
-      await editPage.createSecret(secretPath, 'foo', 'bar');
+        `write ${enginePath}/1/2/3/4 foo=bar`,
+        `write ${enginePath}/1/2/3/4a foo=bar`,
+        'refresh',
+      ]);
+      await settled();
+      // navigate to farthest leaf
+      await visit(`/vault/secrets/${enginePath}/list`);
+      assert.dom('[data-test-component="navigate-input"]').hasNoValue();
+      assert.dom('[data-test-secret-link]').exists({ count: 1 });
+      await click('[data-test-secret-link="1/"]');
+      assert.dom('[data-test-component="navigate-input"]').hasValue('1/');
+      assert.dom('[data-test-secret-link]').exists({ count: 2 });
+      await click('[data-test-secret-link="1/2/"]');
+      assert.dom('[data-test-component="navigate-input"]').hasValue('1/2/');
+      assert.dom('[data-test-secret-link]').exists({ count: 1 });
+      await click('[data-test-secret-link="1/2/3/"]');
+      assert.dom('[data-test-component="navigate-input"]').hasValue('1/2/3/');
+      assert.dom('[data-test-secret-link]').exists({ count: 2 });
 
-      // setup an ancestor for when we delete
+      // delete the items
-      await listPage.visitRoot({ backend: enginePath });
-      await listPage.secrets.filterBy('text', '1/')[0].click();
-      await listPage.create();
-      await editPage.createSecret('1/2', 'foo', 'bar');
-
-      // lol we have to do this because ember-cli-page-object doesn't like *'s in visitable
-      await listPage.visitRoot({ backend: enginePath });
-      await listPage.secrets.filterBy('text', '1/')[0].click();
-      await listPage.secrets.filterBy('text', '2/')[0].click();
-      await listPage.secrets.filterBy('text', '3/')[0].click();
-      await listPage.create();
-
-      await editPage.createSecret(secretPath + 'a', 'foo', 'bar');
-      await listPage.visitRoot({ backend: enginePath });
-      await listPage.secrets.filterBy('text', '1/')[0].click();
-      await listPage.secrets.filterBy('text', '2/')[0].click();
-      const secretLink = listPage.secrets.filterBy('text', '3/')[0];
-      assert.ok(secretLink, 'link to the 3/ branch displays properly');
-
-      await listPage.secrets.filterBy('text', '3/')[0].click();
       await listPage.secrets.objectAt(0).menuToggle();
       await settled();
       await listPage.delete();
@@ -190,17 +188,20 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
       assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list');
       assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list/1/2/3/`, 'remains on the page');
 
+      assert.dom('[data-test-secret-link]').exists({ count: 1 });
       await listPage.secrets.objectAt(0).menuToggle();
       await listPage.delete();
       await listPage.confirmDelete();
       await settled();
-      assert.strictEqual(currentRouteName(), 'vault.cluster.secrets.backend.list');
+      assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list/1/2/3/`, 'remains on the page');
-      assert.strictEqual(
+      assert.dom(GENERAL.emptyStateTitle).hasText('No secrets under "1/2/3/".');
-        currentURL(),
+      await fillIn('[data-test-component="navigate-input"]', '1/2/');
-        `/vault/secrets/${enginePath}/list/1/`,
+      assert.dom(GENERAL.emptyStateTitle).hasText('No secrets under "1/2/".');
-        'navigates to the ancestor created earlier'
+      await click('[data-test-list-root-link]');
-      );
+      assert.strictEqual(currentURL(), `/vault/secrets/${enginePath}/list`);
+      assert.dom('[data-test-secret-link]').exists({ count: 1 });
     });
+
     test('first level secrets redirect properly upon deletion', async function (assert) {
       const secretPath = 'test';
       await listPage.create();