From bce27df1f4801ec32a0af82c38aa2225173c4bda Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 3 Sep 2020 14:35:05 -0400 Subject: [PATCH 1/3] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20update=20to=20un?= =?UTF-8?q?prefixed=20target=20pathing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/adapters/application.js | 9 +++++++- addons/api/addon/adapters/target.js | 13 ++++++++++++ addons/api/addon/serializers/application.js | 21 +++++++++++++++++++ addons/api/addon/serializers/target.js | 8 +++++++ addons/api/app/adapters/target.js | 1 + addons/api/tests/unit/adapters/target-test.js | 12 +++++++++++ addons/api/tests/unit/models/target-test.js | 6 +++--- .../scopes/scope/projects/project/targets.js | 4 ++-- ui/core/mirage/config.js | 19 +++++++++++------ 9 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 addons/api/addon/adapters/target.js create mode 100644 addons/api/app/adapters/target.js create mode 100644 addons/api/tests/unit/adapters/target-test.js diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 187c94ad1..93f0c3925 100644 --- a/addons/api/addon/adapters/application.js +++ b/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 /** @@ -80,7 +87,7 @@ export default class ApplicationAdapter extends RESTAdapter.extend( 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) { diff --git a/addons/api/addon/adapters/target.js b/addons/api/addon/adapters/target.js new file mode 100644 index 000000000..9d853cc7d --- /dev/null +++ b/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; + +} diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index bfe422161..cea8c06a0 100644 --- a/addons/api/addon/serializers/application.js +++ b/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, diff --git a/addons/api/addon/serializers/target.js b/addons/api/addon/serializers/target.js index eae3ad5b1..eb7fa1155 100644 --- a/addons/api/addon/serializers/target.js +++ b/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 /** diff --git a/addons/api/app/adapters/target.js b/addons/api/app/adapters/target.js new file mode 100644 index 000000000..4cb45ecc3 --- /dev/null +++ b/addons/api/app/adapters/target.js @@ -0,0 +1 @@ +export { default } from 'api/adapters/target'; diff --git a/addons/api/tests/unit/adapters/target-test.js b/addons/api/tests/unit/adapters/target-test.js new file mode 100644 index 000000000..bed673821 --- /dev/null +++ b/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); + }); +}); diff --git a/addons/api/tests/unit/models/target-test.js b/addons/api/tests/unit/models/target-test.js index 11f0d83a4..2652fcce7 100644 --- a/addons/api/tests/unit/models/target-test.js +++ b/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'], diff --git a/ui/core/app/routes/scopes/scope/projects/project/targets.js b/ui/core/app/routes/scopes/scope/projects/project/targets.js index acb1e07cc..e91f0480e 100644 --- a/ui/core/app/routes/scopes/scope/projects/project/targets.js +++ b/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 diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 8bc387276..a74e8fe30 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -182,12 +182,19 @@ 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.scope?.id === scope_id); + }); + this.post('/targets', function ({ targets, scopes }) { + const attrs = this.normalizedRequestAttrs(); + const scope = scopes.findBy(scope => scope.id === attrs.scopeId); + attrs.scope = { id: scope.id, type: scope.type }; + 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]; From 2eeb73b62280b539ef76201ed3eb5daa2c9642ef Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 3 Sep 2020 14:48:59 -0400 Subject: [PATCH 2/3] =?UTF-8?q?test:=20=F0=9F=92=8D=20add=20tests=20for=20?= =?UTF-8?q?adapter=20and=20serializer=20scope=20options?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/adapters/application.js | 5 +-- addons/api/addon/adapters/scope.js | 10 +++++ .../tests/unit/adapters/application-test.js | 10 +++-- addons/api/tests/unit/adapters/scope-test.js | 10 +++++ .../unit/serializers/application-test.js | 42 +++++++++++++++++++ .../api/tests/unit/serializers/target-test.js | 7 +++- 6 files changed, 76 insertions(+), 8 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 93f0c3925..f0feb12d7 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -41,7 +41,7 @@ function prenormalizeArrayResponse(response) { export default class ApplicationAdapter extends RESTAdapter.extend( AdapterBuildURLMixin ) { - + // =attributes /** @@ -84,7 +84,6 @@ 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 && this.hasScopePrefix) { @@ -101,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}`; diff --git a/addons/api/addon/adapters/scope.js b/addons/api/addon/adapters/scope.js index 6d4549844..f078e89df 100644 --- a/addons/api/addon/adapters/scope.js +++ b/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. diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js index dc984bba2..64ae9fd27 100644 --- a/addons/api/tests/unit/adapters/application-test.js +++ b/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) { diff --git a/addons/api/tests/unit/adapters/scope-test.js b/addons/api/tests/unit/adapters/scope-test.js index 7dcd482c7..b38f491ce 100644 --- a/addons/api/tests/unit/adapters/scope-test.js +++ b/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 }) =>{ diff --git a/addons/api/tests/unit/serializers/application-test.js b/addons/api/tests/unit/serializers/application-test.js index c067939b9..e838a8da0 100644 --- a/addons/api/tests/unit/serializers/application-test.js +++ b/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'); diff --git a/addons/api/tests/unit/serializers/target-test.js b/addons/api/tests/unit/serializers/target-test.js index e37eb1de2..cc4413567 100644 --- a/addons/api/tests/unit/serializers/target-test.js +++ b/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' }); }); From 4c4e7c7e2f5709a8abe32edc9d12de244aeb9038 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Fri, 4 Sep 2020 08:41:28 -0400 Subject: [PATCH 3/3] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20formally=20model?= =?UTF-8?q?=20scope=20on=20target=20mocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui/core/mirage/config.js | 6 ++---- ui/core/mirage/models/target.js | 3 ++- ui/core/mirage/scenarios/default.js | 19 +++++++------------ ui/core/mirage/serializers/target.js | 10 ++++++++++ 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index a74e8fe30..bbb7d46d9 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -183,12 +183,10 @@ export default function() { // target this.get('/targets', function ({ targets }, { queryParams: { scope_id } }) { - return targets.where(target => target.scope?.id === scope_id); + return targets.where(target => target.scopeId === scope_id); }); - this.post('/targets', function ({ targets, scopes }) { + this.post('/targets', function ({ targets }) { const attrs = this.normalizedRequestAttrs(); - const scope = scopes.findBy(scope => scope.id === attrs.scopeId); - attrs.scope = { id: scope.id, type: scope.type }; return targets.create(attrs); }); this.get('/targets/:id'); diff --git a/ui/core/mirage/models/target.js b/ui/core/mirage/models/target.js index d33295663..8c2757e71 100644 --- a/ui/core/mirage/models/target.js +++ b/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() }); diff --git a/ui/core/mirage/scenarios/default.js b/ui/core/mirage/scenarios/default.js index 7d4c343f8..d08c2fb75 100644 --- a/ui/core/mirage/scenarios/default.js +++ b/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'); }); diff --git a/ui/core/mirage/serializers/target.js b/ui/core/mirage/serializers/target.js index c9c6935a0..8ced88cfc 100644 --- a/ui/core/mirage/serializers/target.js +++ b/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; }