From f8429b15e9b27d773dddb014e2eac2871d2faf14 Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Fri, 2 Dec 2022 13:44:11 -0500 Subject: [PATCH] Chore/cleanup filter attribute (#1500) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 Rename attributes called filter * refactor: 💡 Remove adapter handling of old attribute errors * refactor: 💡 Check for undefined in serializer instead of falsy --- addons/api/addon/adapters/application.js | 41 +++++----------- addons/api/addon/generated/models/host-set.js | 4 +- .../addon/generated/models/managed-group.js | 4 +- addons/api/addon/models/managed-group.js | 2 - addons/api/addon/serializers/host-set.js | 23 +++++++-- addons/api/addon/serializers/managed-group.js | 24 ++++++++++ .../tests/unit/serializers/host-set-test.js | 45 +++++++++++++++++- .../unit/serializers/managed-group-test.js | 47 +++++++++++++++++-- .../components/form/host-set/aws/index.hbs | 7 ++- .../components/form/host-set/azure/index.hbs | 8 ++-- .../components/form/managed-group/index.hbs | 8 ++-- 11 files changed, 160 insertions(+), 53 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 1c4149511..808de0522 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -263,36 +263,17 @@ export default class ApplicationAdapter extends RESTAdapter.extend( source: { pointer: '/data' }, }; // Normalize field-specific errors, if any. - const errors = fieldErrors.flatMap((error) => { - const newValue = [ - { - detail: error.description, - // Ember Data cannot handle errors on nested attributes, so these are - // flattened (e.g. `attributes.foobar` becomes `attributes_foobar`). - // To capture these errors, add read-only fields on the model - // corresponding to the underscored field name. - source: { - pointer: `/data/attributes/${error.name.replace('.', '_')}`, - }, - }, - ]; - if (error.name.includes('attributes.')) { - newValue.push({ - detail: error.description, - // Going forward, nested attributes will be hoisted. Thus "attributes." - // must be stripped from the error name key. Until all models are - // transitioned away from model fragments, however, both this and the - // prior error key mappings are necessary. - source: { - pointer: `/data/attributes/${error.name.replace( - 'attributes.', - '' - )}`, - }, - }); - } - return newValue; - }); + const errors = fieldErrors.map((error) => ({ + detail: error.description, + // Going forward, nested attributes will be hoisted. Thus "attributes." + // must be stripped from the error name key. Until all models are + // transitioned away from model fragments, however, both this and the + // prior error key mappings are necessary. + source: { + pointer: `/data/attributes/${error.name.replace('attributes.', '')}`, + }, + })); + // Return a list of JSON API errors rooted under the `errors` key. return { errors: [baseError, ...errors], diff --git a/addons/api/addon/generated/models/host-set.js b/addons/api/addon/generated/models/host-set.js index 03c74cb27..a6ed83614 100644 --- a/addons/api/addon/generated/models/host-set.js +++ b/addons/api/addon/generated/models/host-set.js @@ -85,10 +85,10 @@ export default class GeneratedHostSetModel extends BaseModel { }) filters; - // Azure specific + // Azure specific, comes in from API as "filter". This is to avoid collisions with filter @attr('string', { description: '', isNestedAttribute: true, }) - filter; + filter_string; } diff --git a/addons/api/addon/generated/models/managed-group.js b/addons/api/addon/generated/models/managed-group.js index 9b9d2a83d..cd3a29c69 100644 --- a/addons/api/addon/generated/models/managed-group.js +++ b/addons/api/addon/generated/models/managed-group.js @@ -50,7 +50,7 @@ export default class GeneratedManagedGroupModel extends BaseModel { @attr('string', { isNestedAttribute: true, description: - 'The boolean expression filter to use to determine membership.', + 'The boolean expression filter to use to determine membership. Comes in from API as "filter".', }) - filter; + filter_string; } diff --git a/addons/api/addon/models/managed-group.js b/addons/api/addon/models/managed-group.js index da53c04a5..1dadd2cad 100644 --- a/addons/api/addon/models/managed-group.js +++ b/addons/api/addon/models/managed-group.js @@ -5,8 +5,6 @@ import { A } from '@ember/array'; export default class ManagedGroupModel extends GeneratedManagedGroupModel { // =attributes - @attr('string', { readOnly: true }) attributes_filter; - /** * Members is read-only under normal circumstances. But members can * be persisted via calls to `addMembers()` or `removeMembers()`. diff --git a/addons/api/addon/serializers/host-set.js b/addons/api/addon/serializers/host-set.js index 19994f7da..4934be99f 100644 --- a/addons/api/addon/serializers/host-set.js +++ b/addons/api/addon/serializers/host-set.js @@ -3,7 +3,7 @@ import { copy } from 'ember-copy'; const fieldByType = { aws: ['preferred_endpoints', 'filters', 'sync_interval_seconds'], - azure: ['preferred_endpoints', 'filter', 'sync_interval_seconds'], + azure: ['preferred_endpoints', 'filter_string', 'sync_interval_seconds'], }; export default class HostSetSerializer extends ApplicationSerializer { // =properties @@ -69,6 +69,15 @@ export default class HostSetSerializer extends ApplicationSerializer { delete json.attributes[key]; } } + + if (key === 'filter_string') { + const { filter_string } = json.attributes; + if (filter_string) { + json.attributes.filter = json.attributes.filter_string; + } + delete json.attributes.filter_string; + } + return value; } @@ -80,10 +89,18 @@ export default class HostSetSerializer extends ApplicationSerializer { */ normalize(typeClass, hash, ...rest) { const normalizedHash = copy(hash, true); + if (typeof normalizedHash?.attributes?.filters === 'string') { normalizedHash.attributes.filters = [normalizedHash.attributes.filters]; } - const normalized = super.normalize(typeClass, normalizedHash, ...rest); - return normalized; + + // Change name from filter to filter_string + if (normalizedHash?.attributes?.filter) { + normalizedHash.attributes.filter_string = + normalizedHash.attributes.filter; + delete normalizedHash.attributes.filter; + } + + return super.normalize(typeClass, normalizedHash, ...rest); } } diff --git a/addons/api/addon/serializers/managed-group.js b/addons/api/addon/serializers/managed-group.js index f42fc78d2..0b15683d8 100644 --- a/addons/api/addon/serializers/managed-group.js +++ b/addons/api/addon/serializers/managed-group.js @@ -1,4 +1,5 @@ import ApplicationSerializer from './application'; +import { copy } from 'ember-copy'; export default class ManagedGroupSerializer extends ApplicationSerializer { // =properties @@ -7,4 +8,27 @@ export default class ManagedGroupSerializer extends ApplicationSerializer { * @type {boolean} */ serializeScopeID = false; + + serialize() { + const serialized = super.serialize(...arguments); + + if (serialized.attributes.filter_string !== undefined) { + serialized.attributes.filter = serialized.attributes.filter_string; + } + delete serialized.attributes.filter_string; + + return serialized; + } + + normalize(typeClass, hash, ...rest) { + const normalizedHash = copy(hash, true); + + if (normalizedHash?.attributes?.filter) { + normalizedHash.attributes.filter_string = + normalizedHash.attributes.filter; + delete normalizedHash.attributes.filter; + } + + return super.normalize(typeClass, normalizedHash, ...rest); + } } diff --git a/addons/api/tests/unit/serializers/host-set-test.js b/addons/api/tests/unit/serializers/host-set-test.js index 7e8889192..38d85f590 100644 --- a/addons/api/tests/unit/serializers/host-set-test.js +++ b/addons/api/tests/unit/serializers/host-set-test.js @@ -80,7 +80,7 @@ module('Unit | Serializer | host set', function (hooks) { version: 1, preferred_endpoints: [{ value: 'option 1' }, { value: 'option 2' }], sync_interval_seconds: 1, - filter: 'filter 1 && filter 2', + filter_string: 'filter 1 && filter 2', }); assert.deepEqual(record.serialize(), { name: 'Host Set 1', @@ -167,4 +167,47 @@ module('Unit | Serializer | host set', function (hooks) { }, }); }); + + test('it normalizes filter correctly', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('host-set'); + const hostSetModelClass = store.createRecord('host-set').constructor; + const payload = { + id: '1', + name: 'Host Set 1', + type: 'plugin', + host_ids: ['1', '2', '3'], + preferred_endpoints: ['endpoint 1', 'endpoint 2'], + attributes: { + filter: 'filter', + }, + }; + const normalized = serializer.normalizeSingleResponse( + store, + hostSetModelClass, + payload + ); + + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + type: 'host-set', + attributes: { + type: 'plugin', + authorized_actions: [], + name: 'Host Set 1', + host_ids: ['1', '2', '3'], + preferred_endpoints: [ + { value: 'endpoint 1' }, + { value: 'endpoint 2' }, + ], + filters: [], + filter_string: 'filter', + }, + relationships: {}, + }, + }); + }); }); diff --git a/addons/api/tests/unit/serializers/managed-group-test.js b/addons/api/tests/unit/serializers/managed-group-test.js index a75ac1dd2..638310c41 100644 --- a/addons/api/tests/unit/serializers/managed-group-test.js +++ b/addons/api/tests/unit/serializers/managed-group-test.js @@ -41,7 +41,7 @@ module('Unit | Serializer | Managed group', function (hooks) { version: 1, auth_method_id: '1234', type: 'oidc', - filter: 'key=value', + filter_string: 'key=value', }); const snapshot = record._createSnapshot(); snapshot.adapterOptions = {}; @@ -73,7 +73,7 @@ module('Unit | Serializer | Managed group', function (hooks) { version: undefined, auth_method_id: '1234', type: 'oidc', - filter: 'key=value', + filter_string: 'key=value', }, }, }); @@ -134,7 +134,7 @@ module('Unit | Serializer | Managed group', function (hooks) { version: 1, auth_method_id: '1234', type: 'oidc', - filter: 'key=value', + filter_string: 'key=value', }, }, }); @@ -152,4 +152,45 @@ module('Unit | Serializer | Managed group', function (hooks) { }, }); }); + + test('it normalizes correctly', function (assert) { + assert.expect(1); + const store = this.owner.lookup('service:store'); + const serializer = store.serializerFor('managed-group'); + const hostSetModelClass = store.createRecord('managed-group').constructor; + const payload = { + id: '1', + description: 'Description for the test', + version: 1, + auth_method_id: '1234', + type: 'oidc', + attributes: { + filter: 'key=value', + }, + }; + const normalized = serializer.normalizeSingleResponse( + store, + hostSetModelClass, + payload + ); + + assert.deepEqual(normalized, { + included: [], + data: { + id: '1', + + attributes: { + type: 'oidc', + version: 1, + authorized_actions: [], + member_ids: [], + description: 'Description for the test', + auth_method_id: '1234', + filter_string: 'key=value', + }, + type: 'managed-group', + relationships: {}, + }, + }); + }); }); diff --git a/ui/admin/app/components/form/host-set/aws/index.hbs b/ui/admin/app/components/form/host-set/aws/index.hbs index 12db6b61a..cc131cd79 100644 --- a/ui/admin/app/components/form/host-set/aws/index.hbs +++ b/ui/admin/app/components/form/host-set/aws/index.hbs @@ -221,7 +221,7 @@ @@ -233,7 +233,10 @@ false (if form.isEditable false true) }} - {{on 'click' (fn @addStringItem 'filters' @model.filter)}} + {{on + 'click' + (fn @addStringItem 'filters' @model.filter_string) + }} > {{t 'actions.add'}} diff --git a/ui/admin/app/components/form/host-set/azure/index.hbs b/ui/admin/app/components/form/host-set/azure/index.hbs index 43cec87f2..65071fee4 100644 --- a/ui/admin/app/components/form/host-set/azure/index.hbs +++ b/ui/admin/app/components/form/host-set/azure/index.hbs @@ -168,18 +168,18 @@ - {{#if @model.errors.filter}} + {{#if @model.errors.filter_string}} - {{#each @model.errors.filter as |error|}} + {{#each @model.errors.filter_string as |error|}} {{error.message}} {{/each}} diff --git a/ui/admin/app/components/form/managed-group/index.hbs b/ui/admin/app/components/form/managed-group/index.hbs index 05d78caab..428c5aa35 100644 --- a/ui/admin/app/components/form/managed-group/index.hbs +++ b/ui/admin/app/components/form/managed-group/index.hbs @@ -54,15 +54,15 @@ - {{#if @model.errors.attributes_filter}} + {{#if @model.errors.filter_string}} - {{#each @model.errors.attributes_filter as |error|}} + {{#each @model.errors.filter_string as |error|}} {{error.message}} {{/each}}