Chore/cleanup filter attribute (#1500)

* 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

@@ -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],

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;
 }

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;
 }

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()`.

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);
   }
 }

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);
+  }
 }

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: {},
+      },
+    });
+  });
 });

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: {},
+      },
+    });
+  });
 });

ui/admin/app/components/form/host-set/aws/index.hbs

@@ -221,7 +221,7 @@
               <form.input
                 @type='text'
                 @name='filters'
-                @value={{@model.filter}}
+                @value={{@model.filter_string}}
               />
             </row.cell>
             <row.cell @shrink={{true}}>
@@ -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'}}
               </Rose::Button>

ui/admin/app/components/form/host-set/azure/index.hbs

@@ -168,18 +168,18 @@
   <form.textarea
     @name='filter'
     @type='filter'
-    @value={{@model.filter}}
+    @value={{@model.filter_string}}
     @label={{t 'resources.host-set.form.filter.label'}}
     @helperText={{t 'resources.host-set.form.filter.azure.help'}}
     @linkText={{t 'actions.learn-more'}}
-    @error={{@model.errors.description}}
+    @error={{@model.errors.filter_string}}
     disabled={{@model.isSaving}}
     readonly={{false}}
     as |field|
   >
-    {{#if @model.errors.filter}}
+    {{#if @model.errors.filter_string}}
       <field.errors as |errors|>
-        {{#each @model.errors.filter as |error|}}
+        {{#each @model.errors.filter_string as |error|}}
           <errors.message>{{error.message}}</errors.message>
         {{/each}}
       </field.errors>

ui/admin/app/components/form/managed-group/index.hbs

@@ -54,15 +54,15 @@
   <form.input
     @name='filter'
     @type='text'
-    @value={{@model.filter}}
+    @value={{@model.filter_string}}
     @label={{t 'form.filter.label'}}
-    @error={{@model.errors.attributes_filter}}
+    @error={{@model.errors.filter_string}}
     @helperText={{t 'form.filter.help'}}
     as |field|
   >
-    {{#if @model.errors.attributes_filter}}
+    {{#if @model.errors.filter_string}}
       <field.errors as |errors|>
-        {{#each @model.errors.attributes_filter as |error|}}
+        {{#each @model.errors.filter_string as |error|}}
           <errors.message>{{error.message}}</errors.message>
         {{/each}}
       </field.errors>