Merge pull request #178 from hashicorp/fix-last-one-out

Fix last one out

addons/api/addon/models/group.js

@@ -11,7 +11,11 @@ export default class GroupModel extends GeneratedGroupModel {
    * Members is read-only under normal circumstances.  But members can
    * be persisted via calls to `addMembers()` or `removeMembers()`.
    */
-  @attr({readOnly: true, defaultValue: () => A()}) member_ids;
+  @attr({
+    readOnly: true,
+    defaultValue: () => A(),
+    emptyArrayIfMissing: true
+  }) member_ids;
 
   /**
    * An array of resolved user instances.

addons/api/addon/models/host-set.js

@@ -9,7 +9,10 @@ export default class HostSetModel extends GeneratedHostSetModel {
    * Grants is read-only _most_ under normal circumstances.  But grants can
    * be persisted via a dedicated call to `saveGrants()`.
    */
-  @fragmentArray('fragment-string', {readOnly: true}) host_ids;
+  @fragmentArray('fragment-string', {
+    readOnly: true,
+    emptyArrayIfMissing: true
+  }) host_ids;
 
   // =methods
 

addons/api/addon/models/role.js

@@ -12,7 +12,10 @@ 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', {readOnly: true}) principals;
+  @fragmentArray('fragment-principal', {
+    readOnly: true,
+    emptyArrayIfMissing: true
+  }) principals;
 
   /**
    * Grants is read-only _most_ under normal circumstances.  But grants can

addons/api/addon/models/target.js

@@ -9,7 +9,10 @@ export default class TargetModel extends GeneratedTargetModel {
   /**
    * @type {[FragmentHostSetModel]}
    */
-  @fragmentArray('fragment-host-set', {readOnly: true}) host_sets;
+  @fragmentArray('fragment-host-set', {
+    readOnly: true,
+    emptyArrayIfMissing: true
+  }) host_sets;
 
   /**
    * An array of resolved host set and host catalog instances.  Model instances

addons/api/addon/serializers/application.js

@@ -117,6 +117,7 @@ export default class ApplicationSerializer extends RESTSerializer {
    * This method makes this transformation to accommodate Ember Data.
    *
    * @method normalizeSingleResponse
+   * @override
    * @param {Store} store
    * @param {Model} primaryModelClass
    * @param {Object} payload
@@ -125,6 +126,10 @@ export default class ApplicationSerializer extends RESTSerializer {
    * @return {Object} JSON-API Document
    */
   normalizeSingleResponse(store, primaryModelClass, payload, id, requestType) {
+    // Copy payload (mostly to prevent mocking issues)
+    payload = copy(payload, true);
+    // Check for and normalize missing arrays
+    payload = this.normalizeMissingArrays(store, primaryModelClass, payload);
     // Setup a new payload data structure.
     const transformedPayload = {};
     // Find the Ember-data-expected root key name.
@@ -132,7 +137,7 @@ export default class ApplicationSerializer extends RESTSerializer {
       primaryModelClass.modelName
     );
     // Copy the unrooted payload under the expected root key name.
-    transformedPayload[payloadKey] = copy(payload, true);
+    transformedPayload[payloadKey] = payload;
     // Return the result of normalizing the transformed payload.
     return super.normalizeSingleResponse(
       store,
@@ -143,6 +148,29 @@ export default class ApplicationSerializer extends RESTSerializer {
     );
   }
 
+  /**
+   * Resets missing array fields to an empty array if they are annotated by
+   * `normalizeToEmptyArray: true` in the associated model attribute
+   * declaration.  Our API excludes arrays when they are empty from
+   * singular responses.
+   *
+   * @param {Store} store
+   * @param {Model} primaryModelClass
+   * @param {object} payload
+   * @return {object}
+   */
+  normalizeMissingArrays(store, primaryModelClass, payload) {
+    const attrDefs = store._attributesDefinitionFor(primaryModelClass.modelName);
+    if (attrDefs) {
+      Object.keys(attrDefs).forEach(key => {
+        if (!payload[key] && attrDefs[key]?.options?.emptyArrayIfMissing) {
+          payload[key] = [];
+        }
+      });
+    }
+    return payload;
+  }
+
   /**
    * One consequence of using model fragments to represent the embedded scope
    * is that they cannot have `id` fields.  We still need to know scope IDs,

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

@@ -138,4 +138,34 @@ module('Unit | Serializer | application', function (hooks) {
       },
     });
   });
+
+  test('it normalizes missing arrays in single responses if annotated in model attributes', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('target');
+    const target = store.createRecord('target').constructor;
+    const payload = {
+      id: '1',
+      name: 'Target 1',
+      scope: { id: 'o_123' }
+    };
+    const normalized = serializer.normalizeSingleResponse(
+      store,
+      target,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'target',
+        attributes: {
+          name: 'Target 1',
+          scope: { scope_id: 'o_123' },
+          host_sets: []
+        },
+        relationships: {},
+      },
+    });
+  });
 });

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

@@ -45,4 +45,34 @@ module('Unit | Serializer | group', function(hooks) {
     });
   });
 
+  test('it normalizes missing member_ids to empty array', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('group');
+    const group = store.createRecord('group').constructor;
+    const payload = {
+      id: '1',
+      name: 'Group 1',
+      scope: { id: 'o_123' }
+    };
+    const normalized = serializer.normalizeSingleResponse(
+      store,
+      group,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'group',
+        attributes: {
+          name: 'Group 1',
+          scope: { scope_id: 'o_123' },
+          member_ids: []
+        },
+        relationships: {},
+      },
+    });
+  });
+
 });

addons/api/tests/unit/serializers/host-set-test.js

@@ -74,4 +74,35 @@ module('Unit | Serializer | host set', function(hooks) {
       },
     });
   });
+
+  test('it normalizes missing host_ids to empty array', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('host-set');
+    const hostSet = store.createRecord('host-set').constructor;
+    const payload = {
+      id: '1',
+      name: 'Host Set 1',
+      scope: { id: 'o_123' }
+    };
+    const normalized = serializer.normalizeSingleResponse(
+      store,
+      hostSet,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'host-set',
+        attributes: {
+          name: 'Host Set 1',
+          scope: { scope_id: 'o_123' },
+          host_ids: []
+        },
+        relationships: {},
+      },
+    });
+  });
+
 });

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

@@ -68,6 +68,37 @@ module('Unit | Serializer | role', function(hooks) {
         attributes: {
           name: 'Role 1',
           grants: [{value: '*'}, {value: '*'}],
+          principals: []
+        },
+        relationships: {},
+      },
+    });
+  });
+
+  test('it normalizes missing principals to empty array', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('role');
+    const role = store.createRecord('role').constructor;
+    const payload = {
+      id: '1',
+      name: 'Role 1',
+      scope: { id: 'o_123' }
+    };
+    const normalized = serializer.normalizeSingleResponse(
+      store,
+      role,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'role',
+        attributes: {
+          name: 'Role 1',
+          scope: { scope_id: 'o_123' },
+          principals: []
         },
         relationships: {},
       },

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

@@ -94,4 +94,34 @@ module('Unit | Serializer | target', function(hooks) {
     });
   });
 
+  test('it normalizes missing host_sets to empty array', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('target');
+    const target = store.createRecord('target').constructor;
+    const payload = {
+      id: '1',
+      name: 'Target 1',
+      scope: { id: 'o_123' }
+    };
+    const normalized = serializer.normalizeSingleResponse(
+      store,
+      target,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'target',
+        attributes: {
+          name: 'Target 1',
+          scope: { scope_id: 'o_123' },
+          host_sets: []
+        },
+        relationships: {},
+      },
+    });
+  });
+
 });

ui/core/mirage/config.js

@@ -109,17 +109,18 @@ export default function() {
     const id = idMethod.split(':')[0];
     const method = idMethod.split(':')[1];
     const role = roles.find(id);
-    const updatedAttrs = {
-      version: attrs.version,
-      userIds: role.userIds,
-      groupIds: role.groupIds
-    };
+    let updatedAttrs = {};
 
     // Principals is a combined list of users and groups, but in Mirage we've
     // internally modelled them as separate lists.  Therefore we must check
     // the type by looking up the user or group first, then determine which
     // list to add the principal to.
     if (method === 'add-principals') {
+      updatedAttrs = {
+        version: attrs.version,
+        userIds: role.userIds,
+        groupIds: role.groupIds
+      };
       attrs.principalIds.forEach(id => {
         const isUser = users.find(id);
         const isGroup = groups.find(id);
@@ -134,6 +135,11 @@ export default function() {
     // If deleting principals, filter them out of both users and groups,
     // and in this case don't care about the type
     if (method === 'remove-principals') {
+      updatedAttrs = {
+        version: attrs.version,
+        userIds: role.userIds,
+        groupIds: role.groupIds
+      };
       updatedAttrs.userIds = updatedAttrs.userIds.filter(id => {
         return !attrs.principalIds.includes(id);
       });
@@ -141,6 +147,14 @@ export default function() {
         return !attrs.principalIds.includes(id);
       });
     }
+
+    if (method === 'set-grants') {
+      updatedAttrs = {
+        version: attrs.version,
+        grant_strings: attrs.grant_strings
+      };
+    }
+
     return role.update(updatedAttrs);
   });
 

ui/core/mirage/factories/role.js

@@ -4,7 +4,7 @@ import { trait } from 'ember-cli-mirage';
 export default factory.extend({
 
   // eslint-disable-next-line ember/avoid-leaking-state-in-ember-objects
-  grant_strings: [
+  grant_strings: () => [
     'id=*;action=*',
     'id=*;type=host-catalog;actions=create,read'
   ],

ui/core/mirage/serializers/group.js

@@ -12,7 +12,7 @@ export default ApplicationSerializer.extend({
 
   _hashForModel(model) {
     const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments);
-    json.member_ids = model.memberIds;
+    if (model.memberIds.length) json.member_ids = model.memberIds;
     return json;
   }
 

ui/core/mirage/serializers/host-set.js

@@ -12,8 +12,10 @@ export default ApplicationSerializer.extend({
 
   _hashForModel(model) {
     const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments);
-    json.host_ids = model.hostIds;
     json.host_catalog_id = model.hostCatalogId;
+    if (model.hostIds?.length) {
+      json.host_ids = model.hostIds;
+    }
     return json;
   }
 

ui/core/mirage/serializers/role.js

@@ -16,6 +16,7 @@ export default ApplicationSerializer.extend({
       type: 'group'
     }));
     json.principals = [ ...serializedUsers, ...serializedGroups ];
+    if (!json.principals.length) delete json.principals;
     return json;
   }
 });

ui/core/mirage/serializers/target.js

@@ -13,11 +13,13 @@ export default ApplicationSerializer.extend({
   _hashForModel(model) {
     const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments);
     const { hostSets } = this.schema;
-    json.host_sets = model.hostSetIds.map(host_set_id => {
-      const hostSet = hostSets.find(host_set_id);
-      const host_catalog_id = hostSet?.hostCatalog?.id;
-      return { id: host_set_id, host_catalog_id };
-    });
+    if (model.hostSetIds?.length) {
+      json.host_sets = model.hostSetIds.map(host_set_id => {
+        const hostSet = hostSets.find(host_set_id);
+        const host_catalog_id = hostSet?.hostCatalog?.id;
+        return { id: host_set_id, host_catalog_id };
+      });
+    }
     return json;
   }