Merge pull request #154 from hashicorp/refactor-target-pathing

Refactor target pathing

addons/api/addon/adapters/application.js

@@ -41,6 +41,7 @@ function prenormalizeArrayResponse(response) {
 export default class ApplicationAdapter extends RESTAdapter.extend(
   AdapterBuildURLMixin
 ) {
+
   // =attributes
 
   /**
@@ -57,6 +58,12 @@ export default class ApplicationAdapter extends RESTAdapter.extend(
    */
   namespace = get(config, 'api.namespace');
 
+  /**
+   * Whether or not to prefix the URL with the resource scope.
+   * @type {boolean}
+   */
+  hasScopePrefix = true;
+
   // =methods
 
   /**
@@ -77,10 +84,9 @@ export default class ApplicationAdapter extends RESTAdapter.extend(
    */
   urlPrefix(path, parentURL, modelName, id, snapshot) {
     const prefix = super.urlPrefix(...arguments);
-    const isScope = modelName === 'scope';
     let scopePath = '';
     let scopeID = '';
-    if (snapshot) {
+    if (snapshot && this.hasScopePrefix) {
       // Not all snapshots have `attr` (such as array snapshots),
       // so we do this sort of ugly check.
       if (snapshot.attr) {
@@ -94,7 +100,7 @@ export default class ApplicationAdapter extends RESTAdapter.extend(
     // Only non-scope resources need a scope path, since scope resources
     // aren't technically "scoped" the same way.
     // Ensure a slash is added between prefix + scope path if needed.
-    if (!isScope && scopeID && prefix.charAt(prefix.length - 1) !== '/') {
+    if (scopeID && prefix.charAt(prefix.length - 1) !== '/') {
       scopePath = `/scopes/${scopeID}`;
     }
     return `${prefix}${scopePath}`;

addons/api/addon/adapters/scope.js

@@ -3,6 +3,16 @@ import { serializeIntoHash } from '@ember-data/adapter/-private';
 
 export default class ScopeAdapter extends ApplicationAdapter {
 
+  // =attributes
+
+  /**
+   * Scopes do not include a scope prefix.
+   * @override
+   */
+  hasScopePrefix = false;
+
+  // =methods
+
   /**
    * Identical to default method except that we pass a query parameter
    * indicating the parent scope.

addons/api/addon/adapters/target.js

@@ -0,0 +1,13 @@
+import ApplicationAdapter from './application';
+
+export default class TargetAdapter extends ApplicationAdapter {
+
+  // =attributes
+  
+  /**
+   * Targets use the new, unprefixed pathing style.
+   * @override
+   */
+  hasScopePrefix = false;
+
+}

addons/api/addon/serializers/application.js

@@ -14,6 +14,12 @@ export default class ApplicationSerializer extends RESTSerializer {
     scope: { serialize: false }
   };
 
+  /**
+   * Whether or not to serialize the scope ID into scope_id.
+   * @type {boolean}
+   */
+  serializeScopeID = false;
+
   // =methods
 
   /**
@@ -67,6 +73,21 @@ export default class ApplicationSerializer extends RESTSerializer {
     Object.assign(hash, serialized);
   }
 
+  /**
+   * Optionally serializes the scope ID into `scope_id`.
+   * @override
+   * @param {Snapshot} snapshot
+   * @return {object}
+   */
+  serialize(snapshot) {
+    const serialized = super.serialize(...arguments);
+    if (this.serializeScopeID) {
+      const scope_id = snapshot?.attr('scope')?.attr('scope_id');
+      if (scope_id) serialized.scope_id = scope_id;
+    }
+    return serialized;
+  }
+
   /**
    * In our API, array payloads are always rooted under the same key `items`.
    * Ember Data normally expects them to be rooted under their model name,

addons/api/addon/serializers/target.js

@@ -2,6 +2,14 @@ import ApplicationSerializer from './application';
 
 export default class TargetSerializer extends ApplicationSerializer {
 
+  // =attributes
+
+  /**
+   * Serialized the scope ID into the `scope_id` field.
+   * @type {boolean}
+   */
+  serializeScopeID = true;
+
   // =methods
 
   /**

addons/api/app/adapters/target.js

@@ -0,0 +1 @@
+export { default } from 'api/adapters/target';

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

@@ -35,14 +35,16 @@ module('Unit | Adapter | application', function (hooks) {
     assert.equal(prefix, `v1/scopes/${scopeID}`);
   });
 
-  test('it does not prefixes URLs with `scopeID` for scope resources', function (assert) {
-    assert.expect(1);
+  test('it does not prefix URLs with `scopeID` when hasScopePrefix is `false`', function (assert) {
+    assert.expect(2);
     const scopeID = 'global';
     const adapter = this.owner.lookup('adapter:application');
-    const prefix = adapter.urlPrefix(null, null, 'scope', null, {
+    adapter.hasScopePrefix = false;
+    const prefix = adapter.urlPrefix(null, null, null, null, {
       adapterOptions: { scopeID }
     });
-    assert.equal(prefix, `v1`);
+    assert.ok(config.api.namespace);
+    assert.equal(prefix, 'v1');
   });
 
   test('it generates correct default URL suffixes', function (assert) {

addons/api/tests/unit/adapters/scope-test.js

@@ -6,6 +6,16 @@ module('Unit | Adapter | scope', function(hooks) {
   setupTest(hooks);
   setupMirage(hooks);
 
+  test('it does not prefix URLs with `scopeID` for scope resources', function (assert) {
+    assert.expect(1);
+    const scopeID = 'global';
+    const adapter = this.owner.lookup('adapter:scope');
+    const prefix = adapter.urlPrefix(null, null, 'scope', null, {
+      adapterOptions: { scopeID }
+    });
+    assert.equal(prefix, `v1`);
+  });
+  
   test('it generates correct createRecord URLs with a scope_id query parameter', function (assert) {
     assert.expect(1);
     this.server.post('/v1/scopes', (schema, { queryParams }) =>{

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

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

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

@@ -59,7 +59,7 @@ module('Unit | Model | target', function (hooks) {
 
   test('it has an `addHostSets` method that targets a specific POST API endpoint and serialization', async function (assert) {
     assert.expect(1);
-    this.server.post('/v1/scopes/o_1/targets/123abc:add-host-sets', (schema, request) => {
+    this.server.post('/v1/targets/123abc:add-host-sets', (schema, request) => {
       const body = JSON.parse(request.requestBody);
       assert.deepEqual(body, {
         host_set_ids: ['123_abc', 'foobar'],
@@ -92,7 +92,7 @@ module('Unit | Model | target', function (hooks) {
 
   test('it has a `removeHostSets` method that targets a specific POST API endpoint and serialization', async function (assert) {
     assert.expect(1);
-    this.server.post('/v1/scopes/o_1/targets/123abc:remove-host-sets', (schema, request) => {
+    this.server.post('/v1/targets/123abc:remove-host-sets', (schema, request) => {
       const body = JSON.parse(request.requestBody);
       assert.deepEqual(body, {
         host_set_ids: ['1', '3'],
@@ -125,7 +125,7 @@ module('Unit | Model | target', function (hooks) {
 
   test('it has a `removeHostSet` method that deletes a single host set using `removeHostSets` method', async function (assert) {
     assert.expect(1);
-    this.server.post('/v1/scopes/o_1/targets/123abc:remove-host-sets', (schema, request) => {
+    this.server.post('/v1/targets/123abc:remove-host-sets', (schema, request) => {
       const body = JSON.parse(request.requestBody);
       assert.deepEqual(body, {
         host_set_ids: ['3'],

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

@@ -29,6 +29,48 @@ module('Unit | Serializer | application', function (hooks) {
     });
   });
 
+  test('it does not serialize scope_id when serializeScopeID is false', function (assert) {
+    assert.expect(2);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('user');
+    const record = store.createRecord('user', {
+      name: 'User',
+      description: 'Description',
+      scope: {
+        scope_id: 'global',
+        type: 'global'
+      }
+    });
+    const serializedRecord = record.serialize();
+    assert.equal(serializer.serializeScopeID, false);
+    assert.deepEqual(serializedRecord, {
+      name: 'User',
+      description: 'Description'
+    });
+  });
+
+  test('it does serialize scope_id when serializeScopeID is true', function (assert) {
+    assert.expect(2);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('user');
+    serializer.serializeScopeID = true;
+    const record = store.createRecord('user', {
+      name: 'User',
+      description: 'Description',
+      scope: {
+        scope_id: 'global',
+        type: 'global'
+      }
+    });
+    const serializedRecord = record.serialize();
+    assert.equal(serializer.serializeScopeID, true);
+    assert.deepEqual(serializedRecord, {
+      name: 'User',
+      description: 'Description',
+      scope_id: 'global'
+    });
+  });
+
   test('it serializes non-nullish version fields', function (assert) {
     assert.expect(2);
     const store = this.owner.lookup('service:store');

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

@@ -15,6 +15,10 @@ module('Unit | Serializer | target', function(hooks) {
         { host_set_id: '1', host_catalog_id: '2' },
         { host_set_id: '3', host_catalog_id: '4' }
       ],
+      scope: {
+        scope_id: 'org_1',
+        type: 'org'
+      },
       version: 1
     });
     const snapshot = record._createSnapshot();
@@ -24,7 +28,8 @@ module('Unit | Serializer | target', function(hooks) {
       name: 'User',
       description: 'Description',
       version: 1,
-      protocol: null
+      protocol: null,
+      scope_id: 'org_1'
     });
 
   });

ui/core/app/routes/scopes/scope/projects/project/targets.js

@@ -16,8 +16,8 @@ export default class ScopesScopeProjectsProjectTargetsRoute extends Route {
    * @return {Promise{[TargetModel]}}
    */
   async model() {
-    const scopeID = this.modelFor('scopes.scope.projects.project').id;
-    return this.store.findAll('target', { adapterOptions: { scopeID } });
+    const { id: scope_id } = this.modelFor('scopes.scope.projects.project');
+    return this.store.query('target', { scope_id });
   }
 
   // =actions

ui/core/mirage/config.js

@@ -182,12 +182,17 @@ export default function() {
 
   // target
 
-  this.get('/scopes/:scope_id/targets');
-  this.post('/scopes/:scope_id/targets');
-  this.get('/scopes/:scope_id/targets/:id');
-  this.patch('/scopes/:scope_id/targets/:id');
-  this.del('/scopes/:scope_id/targets/:id');
-  this.post('/scopes/:scope_id/targets/:idMethod', function ({ targets }, { params: { idMethod } }) {
+  this.get('/targets', function ({ targets }, { queryParams: { scope_id } }) {
+    return targets.where(target => target.scopeId === scope_id);
+  });
+  this.post('/targets', function ({ targets }) {
+    const attrs = this.normalizedRequestAttrs();
+    return targets.create(attrs);
+  });
+  this.get('/targets/:id');
+  this.patch('/targets/:id');
+  this.del('/targets/:id');
+  this.post('/targets/:idMethod', function ({ targets }, { params: { idMethod } }) {
     const attrs = this.normalizedRequestAttrs();
     const id = idMethod.split(':')[0];
     const method = idMethod.split(':')[1];

ui/core/mirage/models/target.js

@@ -1,7 +1,8 @@
-import { Model, hasMany } from 'ember-cli-mirage';
+import { Model, belongsTo, hasMany } from 'ember-cli-mirage';
 
 export default Model.extend({
 
+  scope: belongsTo(),
   hostSets: hasMany()
 
 });

ui/core/mirage/scenarios/default.js

@@ -12,25 +12,20 @@ export default function(server) {
     type: 'org',
     scope: { id: globalScope.id, type: globalScope.type }
   }, 'withChildren')[0];
-  const scope = { id: orgScope.id, type: orgScope.type };
+  const scopePojo = { id: orgScope.id, type: orgScope.type };
 
   // Auth
-
-  // Auth methods exist both at the global scope and the org scope.
-  // For simplicity we don't scope our mock auth methods and just return the
-  // same items for request at all scopes.
-  server.createList('auth-method', 3, { scope });
+  server.createList('auth-method', 3, { scope: scopePojo });
 
   // Groups and Users
-  server.createList('group', 5, { scope }, 'withMembers');
+  server.createList('group', 5, { scope: scopePojo }, 'withMembers');
   // Role
-  server.createList('role', 5, { scope }, 'withPrincipals');
+  server.createList('role', 5, { scope: scopePojo }, 'withPrincipals');
 
   // Other resources
-
-  server.db.scopes.where({type: 'project'}).forEach(({ id, type }) => {
-    const scope = { id, type };
-    server.createList('host-catalog', 2, { scope }, 'withChildren');
+  server.schema.scopes.where({type: 'project'}).models.forEach(scope => {
+    const scopePojo = { id: scope.id, type: scope.type };
+    server.createList('host-catalog', 2, { scope: scopePojo }, 'withChildren');
     server.createList('target', 2, { scope }, 'withRandomHostSets');
   });
 

ui/core/mirage/serializers/target.js

@@ -10,6 +10,12 @@ export default ApplicationSerializer.extend({
     )}_ids`;
   },
 
+  keyForRelationshipId(relationshipName) {
+    return `${this._container.inflector.singularize(
+      underscore(relationshipName)
+    )}_id`;
+  },
+
   _hashForModel(model) {
     const json = ApplicationSerializer.prototype._hashForModel.apply(this, arguments);
     const { hostSets } = this.schema;
@@ -18,6 +24,10 @@ export default ApplicationSerializer.extend({
       const host_catalog_id = hostSet?.hostCatalog?.id;
       return { host_set_id, host_catalog_id };
     });
+    json.scope = {
+      id: model.scope.id,
+      type: model.scope.type
+    };
     return json;
   }