From e80708eeb48bc630d30bc1274f15dcc7f41b7d93 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 9 Sep 2020 16:24:48 -0400 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20implement=20new=20pa?= =?UTF-8?q?thing=20for=20groups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/api/addon/adapters/group.js | 13 +++++++++++++ addons/api/addon/serializers/group.js | 8 ++++++++ addons/api/app/adapters/group.js | 1 + addons/api/tests/unit/adapters/application-test.js | 8 +++----- addons/api/tests/unit/adapters/group-test.js | 12 ++++++++++++ addons/api/tests/unit/models/group-test.js | 6 +++--- ui/core/app/routes/scopes/scope/groups.js | 4 ++-- ui/core/app/routes/scopes/scope/groups/group.js | 6 +----- .../scopes/scope/roles/role/add-principals.js | 4 ++-- .../routes/scopes/scope/roles/role/principals.js | 2 +- ui/core/mirage/config.js | 14 ++++++++------ ui/core/mirage/factories/group.js | 3 ++- ui/core/mirage/models/group.js | 3 ++- ui/core/mirage/scenarios/default.js | 2 +- ui/core/mirage/serializers/group.js | 10 ++++++++++ ui/core/tests/acceptance/bread-crumbs-test.js | 2 +- ui/core/tests/acceptance/groups-test.js | 13 +++++-------- 17 files changed, 75 insertions(+), 36 deletions(-) create mode 100644 addons/api/addon/adapters/group.js create mode 100644 addons/api/app/adapters/group.js create mode 100644 addons/api/tests/unit/adapters/group-test.js diff --git a/addons/api/addon/adapters/group.js b/addons/api/addon/adapters/group.js new file mode 100644 index 000000000..2a621c443 --- /dev/null +++ b/addons/api/addon/adapters/group.js @@ -0,0 +1,13 @@ +import ApplicationAdapter from './application'; + +export default class GroupAdapter extends ApplicationAdapter { + + // =attributes + + /** + * Groups use the new, unprefixed pathing style. + * @override + */ + hasScopePrefix = false; + +} diff --git a/addons/api/addon/serializers/group.js b/addons/api/addon/serializers/group.js index bbccc068f..2c7327a84 100644 --- a/addons/api/addon/serializers/group.js +++ b/addons/api/addon/serializers/group.js @@ -2,6 +2,14 @@ import ApplicationSerializer from './application'; export default class GroupSerializer extends ApplicationSerializer { + // =attributes + + /** + * Serialized the scope ID into the `scope_id` field. + * @type {boolean} + */ + serializeScopeID = true; + // =methods /** diff --git a/addons/api/app/adapters/group.js b/addons/api/app/adapters/group.js new file mode 100644 index 000000000..bb77ff26c --- /dev/null +++ b/addons/api/app/adapters/group.js @@ -0,0 +1 @@ +export { default } from 'api/adapters/group'; diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js index 64ae9fd27..a230c5848 100644 --- a/addons/api/tests/unit/adapters/application-test.js +++ b/addons/api/tests/unit/adapters/application-test.js @@ -93,13 +93,11 @@ module('Unit | Adapter | application', function (hooks) { test('it can request records through the store from a specified scope', async function (assert) { assert.expect(1); const store = this.owner.lookup('service:store'); - this.server.get('/v1/scopes/p_456/groups', () => { - assert.ok(true, 'Scoped resource URL was requested.'); + this.server.get('/v1/groups', (_, { queryParams: { scope_id }}) => { + assert.equal(scope_id, 'p_456', 'Scoped resource URL was requested.'); return {items: []}; }); - await store.findAll('group', { - adapterOptions: { scopeID: 'p_456' } - }); + await store.query('group', { scope_id: 'p_456' }); }); test('it rewrites PUT to PATCH, but leaves others unchanged', function (assert) { diff --git a/addons/api/tests/unit/adapters/group-test.js b/addons/api/tests/unit/adapters/group-test.js new file mode 100644 index 000000000..d39297843 --- /dev/null +++ b/addons/api/tests/unit/adapters/group-test.js @@ -0,0 +1,12 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Adapter | group', function(hooks) { + setupTest(hooks); + + // Replace this with your real tests. + test('it exists', function(assert) { + let adapter = this.owner.lookup('adapter:group'); + assert.ok(adapter); + }); +}); diff --git a/addons/api/tests/unit/models/group-test.js b/addons/api/tests/unit/models/group-test.js index 0c52ba369..e5b2423c8 100644 --- a/addons/api/tests/unit/models/group-test.js +++ b/addons/api/tests/unit/models/group-test.js @@ -43,7 +43,7 @@ module('Unit | Model | group', function(hooks) { test('it has an `addMembers` method that groups a specific POST API endpoint and serialization', async function (assert) { assert.expect(1); - this.server.post('/v1/scopes/o_1/groups/123abc:add-members', (schema, request) => { + this.server.post('/v1/groups/123abc:add-members', (schema, request) => { const body = JSON.parse(request.requestBody); assert.deepEqual(body, { member_ids: ['123_abc', 'foobar'], @@ -73,7 +73,7 @@ module('Unit | Model | group', function(hooks) { test('it has a `removeMembers` method that groups a specific POST API endpoint and serialization', async function (assert) { assert.expect(1); - this.server.post('/v1/scopes/o_1/groups/123abc:remove-members', (schema, request) => { + this.server.post('/v1/groups/123abc:remove-members', (schema, request) => { const body = JSON.parse(request.requestBody); assert.deepEqual(body, { member_ids: ['1', '3'], @@ -103,7 +103,7 @@ module('Unit | Model | group', function(hooks) { test('it has a `removeMember` method that deletes a single member set using `removeMembers` method', async function (assert) { assert.expect(1); - this.server.post('/v1/scopes/o_1/groups/123abc:remove-members', (schema, request) => { + this.server.post('/v1/groups/123abc:remove-members', (schema, request) => { const body = JSON.parse(request.requestBody); assert.deepEqual(body, { member_ids: ['3'], diff --git a/ui/core/app/routes/scopes/scope/groups.js b/ui/core/app/routes/scopes/scope/groups.js index c2c75c8fb..d51718c20 100644 --- a/ui/core/app/routes/scopes/scope/groups.js +++ b/ui/core/app/routes/scopes/scope/groups.js @@ -23,8 +23,8 @@ export default class ScopesScopeGroupsRoute extends Route { * @return {Promise[GroupModel]} */ async model() { - const { id: scopeID } = this.modelFor('scopes.scope'); - return this.store.findAll('group', { adapterOptions: { scopeID } }); + const { id: scope_id } = this.modelFor('scopes.scope'); + return this.store.query('group', { scope_id }); } // =actions diff --git a/ui/core/app/routes/scopes/scope/groups/group.js b/ui/core/app/routes/scopes/scope/groups/group.js index b39abc4b8..e7ed63424 100644 --- a/ui/core/app/routes/scopes/scope/groups/group.js +++ b/ui/core/app/routes/scopes/scope/groups/group.js @@ -11,11 +11,7 @@ export default class ScopesScopeGroupsGroupRoute extends Route { * @return {groupModel} */ model({ group_id }) { - const { id: scopeID } = this.modelFor('scopes.scope'); - return this.store.findRecord('group', group_id, { - reload: true, - adapterOptions: { scopeID }, - }); + return this.store.findRecord('group', group_id, { reload: true }); } /** diff --git a/ui/core/app/routes/scopes/scope/roles/role/add-principals.js b/ui/core/app/routes/scopes/scope/roles/role/add-principals.js index 860f3ba00..30f40366c 100644 --- a/ui/core/app/routes/scopes/scope/roles/role/add-principals.js +++ b/ui/core/app/routes/scopes/scope/roles/role/add-principals.js @@ -18,7 +18,7 @@ export default class ScopesScopeRolesRoleAddPrincipalsRoute extends Route { */ beforeModel() { this.store.unloadAll('user'); - this.store.unloadAll('group'); + //this.store.unloadAll('group'); } /** @@ -32,7 +32,7 @@ export default class ScopesScopeRolesRoleAddPrincipalsRoute extends Route { return hash({ role, users: this.store.findAll('user', options), - groups: this.store.findAll('group', options) + groups: this.store.query('group', { scope_id: scopeID }) }); } diff --git a/ui/core/app/routes/scopes/scope/roles/role/principals.js b/ui/core/app/routes/scopes/scope/roles/role/principals.js index 91fa7eb2a..9f7545830 100644 --- a/ui/core/app/routes/scopes/scope/roles/role/principals.js +++ b/ui/core/app/routes/scopes/scope/roles/role/principals.js @@ -32,7 +32,7 @@ export default class ScopesScopeRolesRolePrincipalsRoute extends Route { const groups = role.principals .filterBy('type', 'group') .map(({ principal_id }) => - this.store.findRecord('group', principal_id, { adapterOptions }) + this.store.findRecord('group', principal_id) .then(model => ({ type: 'group', model diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 7d33d0b89..0446758fb 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -107,12 +107,14 @@ export default function() { }); // IAM: Groups - this.get('/scopes/:scope_id/groups'); - this.post('/scopes/:scope_id/groups'); - this.get('/scopes/:scope_id/groups/:id'); - this.patch('/scopes/:scope_id/groups/:id'); - this.del('/scopes/:scope_id/groups/:id'); - this.post('/scopes/:scope_id/groups/:idMethod', function ({ groups }, { params: { idMethod } }) { + this.get('/groups', ({ groups }, { queryParams: { scope_id: scopeId } }) => { + return groups.where({ scopeId }); + }); + this.post('/groups'); + this.get('/groups/:id'); + this.patch('/groups/:id'); + this.del('/groups/:id'); + this.post('/groups/:idMethod', function ({ groups }, { params: { idMethod } }) { const attrs = this.normalizedRequestAttrs(); const id = idMethod.split(':')[0]; const method = idMethod.split(':')[1]; diff --git a/ui/core/mirage/factories/group.js b/ui/core/mirage/factories/group.js index 9531de4f8..e52d8bb82 100644 --- a/ui/core/mirage/factories/group.js +++ b/ui/core/mirage/factories/group.js @@ -11,7 +11,8 @@ export default factory.extend({ withMembers: trait({ afterCreate(group, server) { const scope = group.scope; - const members = server.createList('user', 2, { scope }); + const scopePojo = { id: scope.id, type: scope.type }; + const members = server.createList('user', 2, { scope: scopePojo }); group.update({ members }); } }) diff --git a/ui/core/mirage/models/group.js b/ui/core/mirage/models/group.js index 7f556c505..3d73f3d73 100644 --- a/ui/core/mirage/models/group.js +++ b/ui/core/mirage/models/group.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(), members: hasMany('user') }); diff --git a/ui/core/mirage/scenarios/default.js b/ui/core/mirage/scenarios/default.js index 84d6708d8..b42e7ca5f 100644 --- a/ui/core/mirage/scenarios/default.js +++ b/ui/core/mirage/scenarios/default.js @@ -19,7 +19,7 @@ export default function(server) { server.createList('auth-method', 3, { scope: orgScope }); // Groups and Users - server.createList('group', 5, { scope: scopePojo }, 'withMembers'); + server.createList('group', 5, { scope: orgScope }, 'withMembers'); // Role server.createList('role', 5, { scope: scopePojo }, 'withPrincipals'); diff --git a/ui/core/mirage/serializers/group.js b/ui/core/mirage/serializers/group.js index 2854224f8..ca0f9b138 100644 --- a/ui/core/mirage/serializers/group.js +++ b/ui/core/mirage/serializers/group.js @@ -10,9 +10,19 @@ 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); json.member_ids = model.memberIds; + json.scope = { + id: model.scope.id, + type: model.scope.type + }; return json; } diff --git a/ui/core/tests/acceptance/bread-crumbs-test.js b/ui/core/tests/acceptance/bread-crumbs-test.js index 879eb311b..aafeecb5a 100644 --- a/ui/core/tests/acceptance/bread-crumbs-test.js +++ b/ui/core/tests/acceptance/bread-crumbs-test.js @@ -44,7 +44,7 @@ module('Acceptance | breadcrumbs', function (hooks) { project = getProjectScope(); user = this.server.create('user'); role = this.server.create('role'); - group = this.server.create('group'); + group = this.server.create('group', { scope: orgScope }); hostCatalog = this.server.create('host-catalog', { scope: { id: project.id, diff --git a/ui/core/tests/acceptance/groups-test.js b/ui/core/tests/acceptance/groups-test.js index a6ad6ccd9..017d2e669 100644 --- a/ui/core/tests/acceptance/groups-test.js +++ b/ui/core/tests/acceptance/groups-test.js @@ -30,10 +30,7 @@ module('Acceptance | groups', function (hooks) { ); const group = this.server.create('group', { - scope: { - id: orgScope.id, - type: orgScope.type, - }, + scope: orgScope }); groupsURL = `/scopes/${orgScope.id}/groups`; @@ -78,7 +75,7 @@ module('Acceptance | groups', function (hooks) { test('saving a new group with invalid fields displays error messages', async function (assert) { assert.expect(2); - this.server.post('/scopes/:scope_id/groups', () => { + this.server.post('/groups', () => { return new Response( 400, {}, @@ -139,7 +136,7 @@ module('Acceptance | groups', function (hooks) { test('saving an existing group with invalid fields displays error messages', async function (assert) { assert.expect(2); - this.server.patch('/scopes/:scope_id/groups/:id', () => { + this.server.patch('/groups/:id', () => { return new Response( 400, {}, @@ -175,7 +172,7 @@ module('Acceptance | groups', function (hooks) { test('errors are displayed when save project fails', async function (assert) { assert.expect(1); - this.server.patch('/scopes/:scope_id/groups/:id', () => { + this.server.patch('/groups/:id', () => { return new Response( 490, {}, @@ -198,7 +195,7 @@ module('Acceptance | groups', function (hooks) { test('errors are displayed when delete project fails', async function (assert) { assert.expect(1); - this.server.del('/scopes/:scope_id/groups/:id', () => { + this.server.del('/groups/:id', () => { return new Response( 490, {},