UI: Fix empty item on kv list (#22838)

ui/app/components/secret-delete-menu.js

@@ -9,8 +9,6 @@ import { inject as service } from '@ember/service';
 import Component from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
-import { alias } from '@ember/object/computed';
-import { maybeQueryRecord } from 'vault/macros/maybe-query-record';
 
 const getErrorMessage = (errors) => {
   const errorMessage =
@@ -24,92 +22,21 @@ export default class SecretDeleteMenu extends Component {
 
   @tracked showDeleteModal = false;
 
-  @maybeQueryRecord(
-    'capabilities',
-    (context) => {
-      if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return;
-      const [backend, id] = JSON.parse(context.args.modelForData.id);
-      return {
-        id: `${backend}/undelete/${id}`,
-      };
-    },
-    'model.id'
-  )
-  undeleteVersionPath;
-  @alias('undeleteVersionPath.canUpdate') canUndeleteVersion;
-
-  @maybeQueryRecord(
-    'capabilities',
-    (context) => {
-      if (!context.args || !context.args.modelForData || !context.args.modelForData.id) return;
-      const [backend, id] = JSON.parse(context.args.modelForData.id);
-      return {
-        id: `${backend}/destroy/${id}`,
-      };
-    },
-    'model.id'
-  )
-  destroyVersionPath;
-  @alias('destroyVersionPath.canUpdate') canDestroyVersion;
-
-  @maybeQueryRecord(
-    'capabilities',
-    (context) => {
-      if (!context.args.model || !context.args.model.engine || !context.args.model.id) return;
-      const backend = context.args.model.engine.id;
-      const id = context.args.model.id;
-      return {
-        id: `${backend}/metadata/${id}`,
-      };
-    },
-    'model',
-    'model.id',
-    'mode'
-  )
-  v2UpdatePath;
-  @alias('v2UpdatePath.canDelete') canDestroyAllVersions;
-
-  @maybeQueryRecord(
-    'capabilities',
-    (context) => {
-      if (!context.args.model || context.args.mode === 'create') {
-        return;
-      }
-      const backend = context.args.isV2 ? context.args.model.engine.id : context.args.model.backend;
-      const id = context.args.model.id;
-      const path = context.args.isV2 ? `${backend}/data/${id}` : `${backend}/${id}`;
-      return {
-        id: path,
-      };
-    },
-    'isV2',
-    'model',
-    'model.id',
-    'mode'
-  )
-  secretDataPath;
-  @alias('secretDataPath.canDelete') canDeleteSecretData;
-
-  @maybeQueryRecord(
-    'capabilities',
-    (context) => {
-      if (!context.args.model || context.args.mode === 'create') {
-        return;
-      }
-      const backend = context.args.isV2 ? context.args.model.engine.id : context.args.model.backend;
-      const id = context.args.model.id;
-      const path = context.args.isV2 ? `${backend}/delete/${id}` : `${backend}/${id}`;
-      return {
-        id: path,
-      };
-    },
-    'isV2',
-    'model',
-    'model.id',
-    'mode'
-  )
-  secretSoftDataPath;
-  @alias('secretSoftDataPath.canUpdate') canSoftDeleteSecretData;
+  get canUndeleteVersion() {
+    return this.args.modelForData.canUndeleteVersion;
+  }
+  get canDestroyVersion() {
+    return this.args.modelForData.canDestroyVersion;
+  }
+  get canDestroyAllVersions() {
+    return this.args.modelForData.canDestroyAllVersions;
+  }
+  get canDeleteSecretData() {
+    return this.args.modelForData.canDeleteSecretData;
+  }
+  get canSoftDeleteSecretData() {
+    return this.args.modelForData.canSoftDeleteSecretData;
+  }
 
   get isLatestVersion() {
     // must have metadata access.

ui/app/models/secret-v2.js

@@ -58,6 +58,9 @@ export default SecretV2Model.extend(KeyMixin, {
   }),
   secretDataPath: lazyCapabilities(apiPath`${'engineId'}/data/${'id'}`, 'engineId', 'id'),
   secretMetadataPath: lazyCapabilities(apiPath`${'engineId'}/metadata/${'id'}`, 'engineId', 'id'),
+  secretUndeletePath: lazyCapabilities(apiPath`${'engineId'}/undelete/${'id'}`, 'engineId', 'id'),
+  secretDeletePath: lazyCapabilities(apiPath`${'engineId'}/delete/${'id'}`, 'engineId', 'id'),
+  secretDestroyPath: lazyCapabilities(apiPath`${'engineId'}/destroy/${'id'}`, 'engineId', 'id'),
 
   canListMetadata: alias('secretMetadataPath.canList'),
   canReadMetadata: alias('secretMetadataPath.canRead'),
@@ -66,4 +69,9 @@ export default SecretV2Model.extend(KeyMixin, {
   canReadSecretData: alias('secretDataPath.canRead'),
   canEditSecretData: alias('secretDataPath.canUpdate'),
   canDeleteSecretData: alias('secretDataPath.canDelete'),
+
+  canUndelete: alias('secretUndeletePath.canUpdate'),
+  canDestroyVersion: alias('secretDestroyPath.canUpdate'),
+  canDestroyAllVersions: alias('secretMetadataPath.canDelete'),
+  canSoftDeleteSecretData: alias('secretDeletePath.canUpdate'),
 });

ui/app/services/store.js

@@ -4,7 +4,7 @@
  */
 
 import Store from '@ember-data/store';
-import { schedule } from '@ember/runloop';
+import { run, schedule } from '@ember/runloop';
 import { resolve, Promise } from 'rsvp';
 import { dasherize } from '@ember/string';
 import { assert } from '@ember/debug';
@@ -138,12 +138,22 @@ export default class StoreService extends Store {
     return resp;
   }
 
+  forceUnload(modelName) {
+    // Hack to get unloadAll to work correctly until we update to ember-data@4.12
+    // so that all the records are properly unloaded and we don't get ghost records
+    this.peekAll(modelName).length;
+    // force destroy queue to flush https://github.com/emberjs/data/issues/5447
+    run(() => this.unloadAll(modelName));
+  }
+
   // pushes records into the store and returns the result
   fetchPage(modelName, query) {
     const response = this.constructResponse(modelName, query);
-    this.unloadAll(modelName);
+    this.forceUnload(modelName);
+    // Hack to ensure the pushed records below all get in the store. remove with update to ember-data@4.12
+    this.peekAll(modelName).length;
     return new Promise((resolve) => {
-      // after the above unloadRecords are finished, push into store
+      // push subset of records into the store
       schedule('destroy', () => {
         this.push(
           this.serializerFor(modelName).normalizeResponse(
@@ -154,6 +164,8 @@ export default class StoreService extends Store {
             'query'
           )
         );
+        // Hack to make sure all records get in model correctly. remove with update to ember-data@4.12
+        this.peekAll(modelName).length;
         const model = this.peekAll(modelName).toArray();
         model.set('meta', response.meta);
         resolve(model);
@@ -188,6 +200,7 @@ export default class StoreService extends Store {
   clearAllDatasets() {
     this.clearDataset();
   }
+
   /**
    * this is designed to be a temporary workaround to an issue in the test environment after upgrading to Ember 4.12
    * when performing an unloadAll or unloadRecord for auth-method or secret-engine models within the app code an error breaks the tests

ui/app/templates/components/secret-delete-menu.hbs

@@ -136,7 +136,7 @@
       </button>
     </footer>
   </Modal>
-{{else if this.canDeleteSecretData}}
+{{else if @model.canDelete}}
   <ConfirmAction
     @buttonClasses="toolbar-link"
     @confirmTitle="Delete secret?"

ui/lib/kv/addon/routes/list-directory.js

@@ -46,9 +46,15 @@ export default class KvSecretsListRoute extends Route {
       });
   }
 
+  getPathToSecret(pathParam) {
+    if (!pathParam) return '';
+    // links and routing assumes pathToParam includes trailing slash
+    return pathIsDirectory(pathParam) ? normalizePath(pathParam) : normalizePath(`${pathParam}/`);
+  }
+
   model(params) {
     const { pageFilter, path_to_secret } = params;
-    const pathToSecret = path_to_secret ? normalizePath(path_to_secret) : '';
+    const pathToSecret = this.getPathToSecret(path_to_secret);
     const backend = this.secretMountPath.currentPath;
     const filterValue = pathToSecret ? (pageFilter ? pathToSecret + pageFilter : pathToSecret) : pageFilter;
     return hash({

ui/tests/acceptance/secrets/backend/kv/kv-v2-workflow-edge-cases-test.js

@@ -34,6 +34,8 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
     await authPage.login();
     await runCmd(mountEngineCmd('kv-v2', this.backend), false);
     await writeSecret(this.backend, this.fullSecretPath, 'foo', 'bar');
+    await writeSecret(this.backend, 'edge/one', 'foo', 'bar');
+    await writeSecret(this.backend, 'edge/two', 'foo', 'bar');
     return;
   });
 
@@ -249,6 +251,19 @@ module('Acceptance | kv-v2 workflow | edge cases', function (hooks) {
         );
     });
   });
+
+  test('no ghost item after editing metadata', async function (assert) {
+    await visit(`/vault/secrets/${this.backend}/kv/edge/directory`);
+    assert.dom(PAGE.list.item()).exists({ count: 2 }, 'two secrets are listed');
+    await click(PAGE.list.item('two'));
+    await click(PAGE.secretTab('Metadata'));
+    await click(PAGE.metadata.editBtn);
+    await fillIn(FORM.keyInput(), 'foo');
+    await fillIn(FORM.valueInput(), 'bar');
+    await click(FORM.saveBtn);
+    await click(PAGE.breadcrumbAtIdx(2));
+    assert.dom(PAGE.list.item()).exists({ count: 2 }, 'two secrets are listed');
+  });
 });
 
 // NAMESPACE TESTS

ui/tests/unit/unload-test.js

@@ -0,0 +1,109 @@
+import { run } from '@ember/runloop';
+import Model, { attr } from '@ember-data/model';
+import { setupTest } from 'ember-qunit';
+import { module, test } from 'qunit';
+import Adapter from '@ember/test/adapter';
+
+/**
+ * This test is testing ember internals for what we need available on lazyPaginatedQuery
+ */
+module('Unit | Model | unloadAll works as expected', function (hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(function () {
+    const Company = Model.extend({
+      name: attr('string'),
+    });
+
+    const CompanyAdapter = Adapter.extend({
+      updateRecord: () => {
+        return undefined;
+      },
+      createRecord: () => {
+        return {
+          id: '4',
+          data: {
+            name: 'Foobar',
+          },
+        };
+      },
+    });
+
+    this.owner.register('model:company', Company);
+    this.owner.register('adapter:company', CompanyAdapter);
+    this.store = this.owner.lookup('service:store');
+  });
+
+  test('edit then unload correctly removes all records', async function (assert) {
+    this.store.push({
+      data: [
+        {
+          id: '1',
+          type: 'company',
+          attributes: {
+            name: 'ACME',
+          },
+        },
+        {
+          id: '2',
+          type: 'company',
+          attributes: {
+            name: 'EMCA',
+          },
+        },
+      ],
+    });
+    assert.strictEqual(this.store.peekAll('company').length, 2, '2 companies loaded');
+    const editRecord = this.store.peekRecord('company', '1');
+    editRecord.name = 'Rebrand';
+    await editRecord.save();
+    assert.false(editRecord.hasDirtyAttributes, 'edit record does not have dirty attrs after save');
+    this.store.peekAll('company').length;
+    run(() => {
+      this.store.unloadAll('company');
+    });
+
+    assert.strictEqual(this.store.peekAll('company').length, 0, 'peekAll 0 - companies unloaded');
+    assert.strictEqual(
+      this.store.peekAll('company').toArray().length,
+      0,
+      'peekAll array 0 - companies unloaded'
+    );
+  });
+  test('create then unload correctly removes all records', async function (assert) {
+    this.store.push({
+      data: [
+        {
+          id: '1',
+          type: 'company',
+          attributes: {
+            name: 'ACME',
+          },
+        },
+        {
+          id: '2',
+          type: 'company',
+          attributes: {
+            name: 'EMCA',
+          },
+        },
+      ],
+    });
+    assert.strictEqual(this.store.peekAll('company').length, 2, '2 companies loaded');
+    const newRecord = this.store.createRecord('company', { name: 'Foobar' });
+
+    await newRecord.save();
+    assert.false(newRecord.hasDirtyAttributes, 'new record does not have dirty attrs after save');
+    this.store.peekAll('company').length;
+    run(() => {
+      this.store.unloadAll('company');
+    });
+
+    assert.strictEqual(this.store.peekAll('company').length, 0, 'peekAll 0 - companies unloaded');
+    assert.strictEqual(
+      this.store.peekAll('company').toArray().length,
+      0,
+      'peekAll array 0 - companies unloaded'
+    );
+  });
+});