refactor: 💡 convert brokered cred IDs array to simple attribute (#1259)

* refactor: 💡 convert brokered cred IDs array to simple attribute

* fix: 🐛 missing test assertion

* fix: 🐛 simplify computed

* refactor: 💡 convert host sources array to simple attribute (#1260)

* refactor: 💡 convert host source IDs array to simple attribute

* fix: 🐛 host source tests

* Refactor role fragment arrays (#1262)

* refactor: 💡 convert grant strings to regular attribute

* refactor: 💡 convert principals array to simple attribute

* Update ui/admin/app/routes/scopes/scope/roles/role.js

Co-authored-by: Zhihe Li <zhihe.li@hashicorp.com>

Co-authored-by: Zhihe Li <zhihe.li@hashicorp.com>

Co-authored-by: Zhihe Li <zhihe.li@hashicorp.com>

* fix: 🐛 remove unused import

Co-authored-by: Zhihe Li <zhihe.li@hashicorp.com>

addons/api/addon/models/fragment-host-source.js

@@ -1,16 +0,0 @@
-import Fragment from 'ember-data-model-fragments/fragment';
-import { attr } from '@ember-data/model';
-
-export default class FragmentHostSourceModel extends Fragment {
-  // =attributes
-
-  @attr('string', {
-    description: 'ID of the related host source',
-  })
-  host_source_id;
-
-  @attr('string', {
-    description: 'ID of the catalog to which the host set belongs',
-  })
-  host_catalog_id;
-}

addons/api/addon/models/fragment-principal.js

@@ -1,21 +0,0 @@
-import Fragment from 'ember-data-model-fragments/fragment';
-import { attr } from '@ember-data/model';
-
-export default class FragmentPrincipal extends Fragment {
-  // =attributes
-
-  @attr('string', {
-    description: 'ID of the related scope',
-  })
-  scope_id;
-
-  @attr('string', {
-    description: 'ID of the related principal (user or group)',
-  })
-  principal_id;
-
-  @attr('string', {
-    description: 'Type of the related principal (user or group)',
-  })
-  type;
-}

addons/api/addon/models/role.js

@@ -1,5 +1,5 @@
 import GeneratedRoleModel from '../generated/models/role';
-import { fragmentArray, array } from 'ember-data-model-fragments/attributes';
+import { attr } from '@ember-data/model';
 
 export default class RoleModel extends GeneratedRoleModel {
   // =attributes
@@ -11,7 +11,7 @@ export default class RoleModel extends GeneratedRoleModel {
    * see obvious.  Instead, the application layer is expected to load referenced
    * users and groups as needed.
    */
-  @fragmentArray('fragment-principal', {
+  @attr('principal-array', {
     readOnly: true,
     emptyArrayIfMissing: true,
   })
@@ -21,7 +21,7 @@ export default class RoleModel extends GeneratedRoleModel {
    * Grant strings are read-only.  But grants can be persisted via a dedicated
    * call to `saveGrantStrings(grants)`.
    */
-  @array('string', { readOnly: true }) grant_strings;
+  @attr({ readOnly: true, emptyArrayIfMissing: true }) grant_strings;
 
   /**
    * Convenience for looking up the grant scope, if loaded.

addons/api/addon/models/target.js

@@ -1,21 +1,19 @@
 import GeneratedTargetModel from '../generated/models/target';
 import { attr } from '@ember-data/model';
-import { fragmentArray } from 'ember-data-model-fragments/attributes';
-import { computed } from '@ember/object';
 
 export default class TargetModel extends GeneratedTargetModel {
   // =attributes
 
   @attr('number', {
     isNestedAttribute: true,
-    description: 'The  default port a target should use if present.',
+    description: 'The default port a target should use if present.',
   })
   default_port;
 
   /**
-   * @type {[FragmentHostSourceModel]}
+   * @type Array
    */
-  @fragmentArray('fragment-host-source', {
+  @attr('host-source-id-array', {
     readOnly: true,
     emptyArrayIfMissing: true,
   })
@@ -25,7 +23,7 @@ export default class TargetModel extends GeneratedTargetModel {
    * Credential source ids are read only and can be
    * persisted via a dedicated call to `addBrokeredCredentialSources()`.
    */
-  @fragmentArray('fragment-string', {
+  @attr('string-array', {
     readOnly: true,
     emptyArrayIfMissing: true,
   })
@@ -37,7 +35,6 @@ export default class TargetModel extends GeneratedTargetModel {
    * instances).  Unresolvable instances are excluded from the array.
    * @type {[{model: HostSetModel, hostCatalog: HostCatalogModel}]}
    */
-  @computed('host_sources.[]', 'store')
   get hostSets() {
     return this.host_sources
       .map(({ host_source_id, host_catalog_id }) => ({
@@ -53,7 +50,6 @@ export default class TargetModel extends GeneratedTargetModel {
    * instances).  Unresolvable instances are excluded from the array.
    * @type {[CredentialLibraryModel, CredentialModel]}
    */
-  @computed('brokered_credential_source_ids.[]', 'store')
   get brokeredCredentialSources() {
     return this.brokered_credential_source_ids
       .map((source) => {

addons/api/addon/serializers/fragment-host-source.js

@@ -1,16 +0,0 @@
-import JSONSerializer from '@ember-data/serializer/json';
-
-export default class FragmentHostSourceSerializer extends JSONSerializer {
-  // =methods
-
-  /**
-   * Renames `id` to `host_source_id` because fragments may not have IDs.
-   * Otherwise normalizes as normal.
-   * @param {ModelClass} typeClass
-   * @param {object} resourceHash
-   * @return {object}
-   */
-  normalize(typeClass, { id: host_source_id, ...obj }) {
-    return super.normalize(typeClass, { host_source_id, ...obj });
-  }
-}

addons/api/addon/serializers/fragment-principal.js

@@ -1,16 +0,0 @@
-import JSONSerializer from '@ember-data/serializer/json';
-
-export default class FragmentPrincipalSerializer extends JSONSerializer {
-  // =methods
-
-  /**
-   * Renames `id` to `principal_id` because fragments may not have IDs.
-   * Otherwise normalizes as normal.
-   * @param {ModelClass} typeClass
-   * @param {object} resourceHash
-   * @return {object}
-   */
-  normalize(typeClass, { id: principal_id, ...obj }) {
-    return super.normalize(typeClass, { principal_id, ...obj });
-  }
-}

addons/api/addon/transforms/host-source-id-array.js

@@ -0,0 +1,16 @@
+import Transform from '@ember-data/serializer/transform';
+
+export default class HostSourceIdArrayTransform extends Transform {
+  deserialize(serialized) {
+    const stringValues = serialized || [];
+    return stringValues.map(({ id: host_source_id, ...obj }) => ({
+      host_source_id,
+      ...obj,
+    }));
+  }
+
+  serialize(deserialized) {
+    const strings = deserialized || [];
+    return strings.map(({ host_source_id: id, ...obj }) => ({ id, ...obj }));
+  }
+}

addons/api/addon/transforms/principal-array.js

@@ -0,0 +1,21 @@
+import Transform from '@ember-data/serializer/transform';
+
+export default class PrincipalArrayTransform extends Transform {
+  deserialize(serialized) {
+    const stringValues = serialized || [];
+    return stringValues.map(({ id: principal_id, scope_id, type }) => ({
+      principal_id,
+      scope_id,
+      type,
+    }));
+  }
+
+  serialize(deserialized) {
+    const strings = deserialized || [];
+    return strings.map(({ principal_id: id, scope_id, type }) => ({
+      id,
+      scope_id,
+      type,
+    }));
+  }
+}

addons/api/addon/transforms/string-array.js

@@ -0,0 +1,13 @@
+import Transform from '@ember-data/serializer/transform';
+
+export default class StringArrayTransform extends Transform {
+  deserialize(serialized) {
+    const stringValues = serialized || [];
+    return stringValues.map((value) => ({ value }));
+  }
+
+  serialize(deserialized) {
+    const strings = deserialized || [];
+    return strings.map(({ value }) => value);
+  }
+}

addons/api/app/models/fragment-host-source.js

@@ -1 +0,0 @@
-export { default } from 'api/models/fragment-host-source';

addons/api/app/models/fragment-principal.js

@@ -1 +0,0 @@
-export { default } from 'api/models/fragment-principal';

addons/api/app/serializers/fragment-host-source.js

@@ -1 +0,0 @@
-export { default } from 'api/serializers/fragment-host-source';

addons/api/app/serializers/fragment-principal.js

@@ -1 +0,0 @@
-export { default } from 'api/serializers/fragment-principal';

addons/api/app/transforms/host-source-id-array.js

@@ -0,0 +1 @@
+export { default } from 'api/transforms/host-source-id-array';

addons/api/app/transforms/principal-array.js

@@ -0,0 +1 @@
+export { default } from 'api/transforms/principal-array';

addons/api/app/transforms/string-array.js

@@ -0,0 +1 @@
+export { default } from 'api/transforms/string-array';

addons/api/tests/unit/models/fragment-host-source-test.js

@@ -1,12 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-
-module('Unit | Model | fragment host source', function (hooks) {
-  setupTest(hooks);
-
-  test('it exists', function (assert) {
-    let store = this.owner.lookup('service:store');
-    let model = store.createRecord('fragment-host-source', {});
-    assert.ok(model);
-  });
-});

addons/api/tests/unit/models/fragment-principal-test.js

@@ -1,12 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-
-module('Unit | Model | fragment principal', function (hooks) {
-  setupTest(hooks);
-
-  test('it exists', function (assert) {
-    let store = this.owner.lookup('service:store');
-    let model = store.createRecord('fragment-principal', {});
-    assert.ok(model);
-  });
-});

addons/api/tests/unit/models/target-test.js

@@ -46,10 +46,6 @@ module('Unit | Model | target', function (hooks) {
         attributes: {},
       },
     });
-    // Since `hostSets` is computed on `host_sources`, not the store itself,
-    // it's necessary to do this assignment to kick-off the computed update.
-    // eslint-disable-next-line no-self-assign
-    target.host_sources = target.host_sources;
     assert.strictEqual(
       target.host_sources.length,
       2,
@@ -252,7 +248,7 @@ module('Unit | Model | target', function (hooks) {
     assert.strictEqual(
       target.brokeredCredentialSources.length,
       0,
-      'Target has no resolved credentialSources because they are not loaded yet'
+      'Target has no resolved credentialSources, because they are not yet loaded'
     );
     store.push({
       data: {
@@ -268,12 +264,6 @@ module('Unit | Model | target', function (hooks) {
         attributes: {},
       },
     });
-    // Since `credentialSources` is computed on `brokered_credential_source_ids`,
-    // not the store itself, it's necessary to do this assignment to kick-off the
-    // computed update.
-    /* eslint-disable no-self-assign */
-    target.brokered_credential_source_ids =
-      target.brokered_credential_source_ids;
     /* eslint-enable no-self-assign */
     assert.strictEqual(
       target.brokered_credential_source_ids.length,

addons/api/tests/unit/serializers/fragment-host-source-test.js

@@ -1,28 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-import FragmentHostSource from 'api/models/fragment-host-source';
-
-module('Unit | Serializer | fragment host source', function (hooks) {
-  setupTest(hooks);
-
-  test('it normalizes `id` into `host_source_id`', function (assert) {
-    assert.expect(1);
-    const store = this.owner.lookup('service:store');
-    const serializer = store.serializerFor('fragment-host-source');
-    const normalized = serializer.normalize(FragmentHostSource, {
-      id: '2',
-      host_catalog_id: '1',
-    });
-    assert.deepEqual(normalized, {
-      data: {
-        attributes: {
-          host_source_id: '2',
-          host_catalog_id: '1',
-        },
-        id: null,
-        relationships: {},
-        type: 'fragment-host-source',
-      },
-    });
-  });
-});

addons/api/tests/unit/serializers/fragment-principal-test.js

@@ -1,30 +0,0 @@
-import { module, test } from 'qunit';
-import { setupTest } from 'ember-qunit';
-import FragmentPrincipal from 'api/models/fragment-principal';
-
-module('Unit | Serializer | fragment principal', function (hooks) {
-  setupTest(hooks);
-
-  test('it normalizes `id` into `principal_id`', function (assert) {
-    assert.expect(1);
-    const store = this.owner.lookup('service:store');
-    const serializer = store.serializerFor('fragment-principal');
-    const normalized = serializer.normalize(FragmentPrincipal, {
-      scope_id: '1',
-      principal_id: '2',
-      type: 'user',
-    });
-    assert.deepEqual(normalized, {
-      data: {
-        attributes: {
-          scope_id: '1',
-          principal_id: '2',
-          type: 'user',
-        },
-        id: null,
-        relationships: {},
-        type: 'fragment-principal',
-      },
-    });
-  });
-});

addons/api/tests/unit/serializers/role-test.js

@@ -98,6 +98,7 @@ module('Unit | Serializer | role', function (hooks) {
           name: 'Role 1',
           scope: { id: 'o_123', scope_id: 'o_123' },
           principals: [],
+          grant_strings: [],
         },
         relationships: {},
       },

addons/api/tests/unit/transforms/host-source-id-array-test.js

@@ -0,0 +1,26 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+
+module('Unit | Transform | host source id array', function (hooks) {
+  setupTest(hooks);
+
+  test('it deserializes an array of strings to an array of `{value}`', function (assert) {
+    assert.expect(1);
+    const transform = this.owner.lookup('transform:host-source-id-array');
+    const deserialized = transform.deserialize([
+      { id: '123', host_catalog_id: '1' },
+    ]);
+    assert.deepEqual(deserialized, [
+      { host_source_id: '123', host_catalog_id: '1' },
+    ]);
+  });
+
+  test('it serializes an array of `{value}` to an array of strings', function (assert) {
+    assert.expect(1);
+    const transform = this.owner.lookup('transform:host-source-id-array');
+    const serialized = transform.serialize([
+      { host_source_id: '123', host_catalog_id: '1' },
+    ]);
+    assert.deepEqual(serialized, [{ id: '123', host_catalog_id: '1' }]);
+  });
+});

addons/api/tests/unit/transforms/principal-array-test.js

@@ -0,0 +1,12 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+
+module('Unit | Transform | principal array', function (hooks) {
+  setupTest(hooks);
+
+  // Replace this with your real tests.
+  test('it exists', function (assert) {
+    let transform = this.owner.lookup('transform:principal-array');
+    assert.ok(transform);
+  });
+});

addons/api/tests/unit/transforms/string-array-test.js

@@ -0,0 +1,28 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+
+module('Unit | Transform | string array', function (hooks) {
+  setupTest(hooks);
+
+  test('it deserializes an array of strings to an array of `{value}`', function (assert) {
+    assert.expect(1);
+    const transform = this.owner.lookup('transform:string-array');
+    const deserialized = transform.deserialize(['a', 'bee', 'Hello World']);
+    assert.deepEqual(deserialized, [
+      { value: 'a' },
+      { value: 'bee' },
+      { value: 'Hello World' },
+    ]);
+  });
+
+  test('it serializes an array of `{value}` to an array of strings', function (assert) {
+    assert.expect(1);
+    const transform = this.owner.lookup('transform:string-array');
+    const serialized = transform.serialize([
+      { value: 'a' },
+      { value: 'bee' },
+      { value: 'Hello World' },
+    ]);
+    assert.deepEqual(serialized, ['a', 'bee', 'Hello World']);
+  });
+});

ui/admin/app/routes/scopes/scope/roles/role.js

@@ -1,4 +1,5 @@
 import Route from '@ember/routing/route';
+import { action } from '@ember/object';
 
 export default class ScopesScopeRolesRoleRoute extends Route {
   // =methods
@@ -16,4 +17,15 @@ export default class ScopesScopeRolesRoleRoute extends Route {
       adapterOptions: { scopeID },
     });
   }
+
+  // =actions
+  /**
+   * Rollback changes on a role.
+   * @param {RoleModel} role
+   */
+  @action
+  cancel(role) {
+    role.rollbackAttributes();
+    this.refresh();
+  }
 }

ui/admin/app/routes/scopes/scope/roles/role/grants.js

@@ -21,7 +21,7 @@ export default class ScopesScopeRolesRoleGrantsRoute extends Route {
    */
   @action
   addGrant(role, grantString) {
-    role.grant_strings.unshiftObject(grantString);
+    role.grant_strings = [grantString].concat(role.grant_strings);
   }
 
   /**
@@ -32,11 +32,13 @@ export default class ScopesScopeRolesRoleGrantsRoute extends Route {
    */
   @action
   removeGrant(role, grantString) {
-    role.grant_strings.removeObject(grantString);
+    role.grant_strings = role.grant_strings.filter(
+      (str) => str !== grantString
+    );
   }
 
   /**
-   * Save an role in current scope.
+   * Save a role.
    * @param {RoleModel} role
    * @param {[string]} grantStrings
    */

ui/admin/app/routes/scopes/scope/roles/role/principals.js

@@ -24,10 +24,10 @@ export default class ScopesScopeRolesRolePrincipalsRoute extends Route {
     // Gather user and group IDs as separate arrays, since these
     // will be queried in separate API queries.
     const userIDs = role.principals
-      .filterBy('type', 'user')
+      .filter(({ type }) => type === 'user')
       .map(({ principal_id }) => principal_id);
     const groupIDs = role.principals
-      .filterBy('type', 'group')
+      .filter(({ type }) => type === 'group')
       .map(({ principal_id }) => principal_id);
 
     // Query for users.