From 857c26272d474cef108dae1f87fcfe9251fea9cb Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Wed, 22 Jul 2020 17:16:20 -0400 Subject: [PATCH 1/8] chore: add initial scopes routes with scope optional scope loading --- ui/core/app/router.js | 7 +- ui/core/app/routes/scopes.js | 4 + ui/core/app/routes/scopes/scope.js | 26 ++++++ ui/core/app/templates/scopes.hbs | 1 + ui/core/app/templates/scopes/scope.hbs | 1 + ui/core/mirage/config.js | 86 +++++++++---------- ui/core/mirage/factories/scope.js | 25 ++++++ ui/core/mirage/generated/factories/scope.js | 15 ++++ ui/core/mirage/scenarios/default.js | 3 +- ui/core/mirage/serializers/scope.js | 5 ++ ui/core/package.json | 1 + ui/core/tests/unit/routes/scopes-test.js | 11 +++ .../tests/unit/routes/scopes/scope-test.js | 11 +++ 13 files changed, 150 insertions(+), 46 deletions(-) create mode 100644 ui/core/app/routes/scopes.js create mode 100644 ui/core/app/routes/scopes/scope.js create mode 100644 ui/core/app/templates/scopes.hbs create mode 100644 ui/core/app/templates/scopes/scope.hbs create mode 100644 ui/core/mirage/factories/scope.js create mode 100644 ui/core/mirage/generated/factories/scope.js create mode 100644 ui/core/mirage/serializers/scope.js create mode 100644 ui/core/tests/unit/routes/scopes-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope-test.js diff --git a/ui/core/app/router.js b/ui/core/app/router.js index 61566937f..6dbe00b3b 100644 --- a/ui/core/app/router.js +++ b/ui/core/app/router.js @@ -7,6 +7,11 @@ export default class Router extends EmberRouter { } Router.map(function () { + + this.route('scopes', function() { + this.route('scope', { path: ':scope_id' }); + }); + this.route('orgs', function () { this.route('org', { path: ':org_id' }, function () { this.route('projects', function () { @@ -23,7 +28,7 @@ Router.map(function () { this.route('user', { path: ':user_id' }, function() {}); this.route('new'); }); - + this.route('roles', function() { this.route('role', { path: ':role_id' }, function() {}); this.route('new'); diff --git a/ui/core/app/routes/scopes.js b/ui/core/app/routes/scopes.js new file mode 100644 index 000000000..5fff74b2e --- /dev/null +++ b/ui/core/app/routes/scopes.js @@ -0,0 +1,4 @@ +import Route from '@ember/routing/route'; + +export default class ScopesRoute extends Route { +} diff --git a/ui/core/app/routes/scopes/scope.js b/ui/core/app/routes/scopes/scope.js new file mode 100644 index 000000000..68201166e --- /dev/null +++ b/ui/core/app/routes/scopes/scope.js @@ -0,0 +1,26 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeRoute extends Route { + + /** + * Attempts to load the specified scope from the API. This is allowed + * to fail, since in some cases the user may not have permission to read a + * scope directly, but may have permission to read its resources. + * If the scope fails to load, we still proceed using a temporary scope object + * consisting of only the specified ID. + * @param {object} params + * @param {string} params.scope_id + * @return {Promise{ScopeModel}} + */ + async model({ scope_id: id }) { + try { + return await this.store.findRecord('scope', id); + } catch { + const maybeExisting = this.store.peekRecord('scope', id); + // TODO it's unclear if this is reliable... + // we may need to revert to a POJO. + return maybeExisting || this.store.createRecord('scope', { id }); + } + } + +} diff --git a/ui/core/app/templates/scopes.hbs b/ui/core/app/templates/scopes.hbs new file mode 100644 index 000000000..e2147cab0 --- /dev/null +++ b/ui/core/app/templates/scopes.hbs @@ -0,0 +1 @@ +{{outlet}} \ No newline at end of file diff --git a/ui/core/app/templates/scopes/scope.hbs b/ui/core/app/templates/scopes/scope.hbs new file mode 100644 index 000000000..e2147cab0 --- /dev/null +++ b/ui/core/app/templates/scopes/scope.hbs @@ -0,0 +1 @@ +{{outlet}} \ No newline at end of file diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 781a607d5..421f87a8f 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -1,5 +1,5 @@ import config from '../config/environment'; -// import { Response } from 'miragejs'; +import { Response } from 'miragejs'; export default function() { @@ -16,66 +16,66 @@ export default function() { this.passthrough(); // Scope resources - // org - this.get('/orgs'); - this.post('/orgs'); - this.get('/orgs/:id'); - this.patch('/orgs/:id'); - this.del('/orgs/:id'); - - // project - - this.get('/orgs/:org_id/projects'); - this.post('/orgs/:org_id/projects'); - this.get('/orgs/:org_id/projects/:id'); - this.patch('/orgs/:org_id/projects/:id'); - this.del('/orgs/:org_id/projects/:id'); + this.post('/scopes'); + // To simulate a possible real-world case, org scopes are not returned, + // but project scopes are. + // TODO this should be expanded to support other scenarios. + this.get('/scopes/:id', ({ scopes }, { params: { id: scopeID } }) => { + const scope = scopes.find(scopeID); + if (scope.type !== 'project') { + return new Response(404, {}, {}); + } else { + return scope; + } + }); + this.patch('/scopes/:id'); + this.del('/scopes/:id'); // Auth & IAM resources - this.get('/orgs/:org_id/auth-methods'); - this.post('/orgs/:org_id/auth-methods'); - this.get('/orgs/:org_id/auth-methods/:id'); - this.patch('/orgs/:org_id/auth-methods/:id'); - this.del('/orgs/:org_id/auth-methods/:id'); + this.get('/scopes/:scope_id/auth-methods'); + this.post('/scopes/:scope_id/auth-methods'); + this.get('/scopes/:scope_id/auth-methods/:id'); + this.patch('/scopes/:scope_id/auth-methods/:id'); + this.del('/scopes/:scope_id/auth-methods/:id'); // IAM : Users - this.get('/orgs/:org_id/users'); - this.post('/orgs/:org_id/users'); - this.get('/orgs/:org_id/users/:id'); - this.patch('/orgs/:org_id/users/:id'); - this.del('/orgs/:org_id/users/:id'); - + this.get('/scopes/:scope_id/users'); + this.post('/scopes/:scope_id/users'); + this.get('/scopes/:scope_id/users/:id'); + this.patch('/scopes/:scope_id/users/:id'); + this.del('/scopes/:scope_id/users/:id'); + // IAM: Roles - this.get('/orgs/:org_id/roles'); - this.post('/orgs/:org_id/roles'); - this.get('/orgs/:org_id/roles/:id'); - this.patch('/orgs/:org_id/roles/:id'); - this.del('/orgs/:org_id/roles/:id'); + this.get('/scopes/:scope_id/roles'); + this.post('/scopes/:scope_id/roles'); + this.get('/scopes/:scope_id/roles/:id'); + this.patch('/scopes/:scope_id/roles/:id'); + this.del('/scopes/:scope_id/roles/:id'); // group - this.get('/orgs/:org_id/groups'); - this.post('/orgs/:org_id/groups'); - this.get('/orgs/:org_id/groups/:id'); - this.patch('/orgs/:org_id/groups/:id'); - this.del('/orgs/:org_id/groups/:id'); + 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'); // Other resources // host-catalog - this.get('/orgs/:org_id/projects/:project_id/host-catalogs'); - this.post('/orgs/:org_id/projects/:project_id/host-catalogs'); - this.get('/orgs/:org_id/projects/:project_id/host-catalogs/:id'); - this.patch('/orgs/:org_id/projects/:project_id/host-catalogs/:id'); - this.del('/orgs/:org_id/projects/:project_id/host-catalogs/:id'); + this.get('/scopes/:scope_id/host-catalogs'); + this.post('/scopes/:scope_id/host-catalogs'); + this.get('/scopes/:scope_id/host-catalogs/:id'); + this.patch('/scopes/:scope_id/host-catalogs/:id'); + this.del('/scopes/:scope_id/host-catalogs/:id'); // Uncomment the following line and the Response import above // Then change the response code to simulate error responses. - // this.get('/orgs/:org_id/projects', () => new Response(505)); + // this.get('/scopes/:scope_id/projects', () => new Response(505)); // Update error payload to simulate specific error responses. - // this.get('/orgs/:org_id/projects', () => new Response(505, {}, { + // this.get('/scopes/:scope_id/projects', () => new Response(505, {}, { // errors: [{ // status: 505, // message: 'HTTP version not supported.', diff --git a/ui/core/mirage/factories/scope.js b/ui/core/mirage/factories/scope.js new file mode 100644 index 000000000..703e80ad4 --- /dev/null +++ b/ui/core/mirage/factories/scope.js @@ -0,0 +1,25 @@ +import factory from '../generated/factories/scope'; +import { trait } from 'ember-cli-mirage'; + +export default factory.extend({ + + type: 'global', + + withChildren: trait({ + afterCreate(record, server) { + if (record.type === 'global') { + server.createList('scope', 1, { + type: 'org', + scope: { id: record.id, type: record.type } + }, 'withChildren'); + } + if (record.type === 'org') { + server.createList('scope', 5, { + type: 'project', + scope: { id: record.id, type: record.type } + }); + } + } + }) + +}); diff --git a/ui/core/mirage/generated/factories/scope.js b/ui/core/mirage/generated/factories/scope.js new file mode 100644 index 000000000..cc574d3cd --- /dev/null +++ b/ui/core/mirage/generated/factories/scope.js @@ -0,0 +1,15 @@ +import { Factory } from 'ember-cli-mirage'; +import { random, date } from 'faker'; + +/** + * GeneratedScopeModelFactory + */ +export default Factory.extend({ + + name: () => random.words(), + description: () => random.words(), + created_time: () => date.recent(), + updated_time: () => date.recent(), + disabled: () => random.boolean(), + +}); diff --git a/ui/core/mirage/scenarios/default.js b/ui/core/mirage/scenarios/default.js index d4e4dc2b7..f143f6ec6 100644 --- a/ui/core/mirage/scenarios/default.js +++ b/ui/core/mirage/scenarios/default.js @@ -2,8 +2,7 @@ export default function(server) { // Scope resources - server.createList('org', 3); - server.createList('project', 6); + server.create('scope', null, 'withChildren'); // Auth diff --git a/ui/core/mirage/serializers/scope.js b/ui/core/mirage/serializers/scope.js new file mode 100644 index 000000000..bfb33f398 --- /dev/null +++ b/ui/core/mirage/serializers/scope.js @@ -0,0 +1,5 @@ +import ApplicationSerializer from './application'; + +export default ApplicationSerializer.extend({ + modelName: 'scope' +}); diff --git a/ui/core/package.json b/ui/core/package.json index 02bcc7ea3..f6ae93043 100644 --- a/ui/core/package.json +++ b/ui/core/package.json @@ -54,6 +54,7 @@ "ember-cli-template-lint": "^1.0.0-beta.3", "ember-cli-uglify": "^3.0.0", "ember-data": "~3.16.0", + "ember-data-model-fragments": "5.0.0-beta.0", "ember-exam": "^5.0.1", "ember-export-application-global": "^2.0.1", "ember-fetch": "^7.0.0", diff --git a/ui/core/tests/unit/routes/scopes-test.js b/ui/core/tests/unit/routes/scopes-test.js new file mode 100644 index 000000000..06a5af092 --- /dev/null +++ b/ui/core/tests/unit/routes/scopes-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes'); + assert.ok(route); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope-test.js b/ui/core/tests/unit/routes/scopes/scope-test.js new file mode 100644 index 000000000..62c97d9f0 --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope'); + assert.ok(route); + }); +}); From c743cc17168a0d86fc01370c431c81e6d5de2c15 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 09:37:06 -0400 Subject: [PATCH 2/8] chore: add projects routes --- .../app/controllers/scopes/scope/projects.js | 19 +++++ .../controllers/scopes/scope/projects/new.js | 19 +++++ .../scopes/scope/projects/project.js | 14 ++++ ui/core/app/router.js | 7 +- ui/core/app/routes/scopes/scope.js | 12 +-- ui/core/app/routes/scopes/scope/projects.js | 84 +++++++++++++++++++ .../app/routes/scopes/scope/projects/index.js | 4 + .../app/routes/scopes/scope/projects/new.js | 19 +++++ .../routes/scopes/scope/projects/project.js | 17 ++++ .../scopes/scope/projects/project/index.js | 4 + ui/core/app/templates/application.hbs | 3 + ui/core/app/templates/scopes/scope.hbs | 38 ++++++++- .../app/templates/scopes/scope/projects.hbs | 3 + .../templates/scopes/scope/projects/index.hbs | 56 +++++++++++++ .../templates/scopes/scope/projects/new.hbs | 21 +++++ .../scopes/scope/projects/project.hbs | 3 + .../scopes/scope/projects/project/index.hbs | 35 ++++++++ ui/core/mirage/config.js | 7 ++ .../controllers/scopes/scope/projects-test.js | 12 +++ .../scopes/scope/projects/new-test.js | 12 +++ .../scopes/scope/projects/project-test.js | 12 +++ .../unit/routes/scopes/scope/projects-test.js | 11 +++ .../scopes/scope/projects/index-test.js | 11 +++ .../routes/scopes/scope/projects/new-test.js | 11 +++ .../scopes/scope/projects/project-test.js | 11 +++ .../scope/projects/project/index-test.js | 11 +++ ui/core/translations/en-us.yaml | 3 +- 27 files changed, 451 insertions(+), 8 deletions(-) create mode 100644 ui/core/app/controllers/scopes/scope/projects.js create mode 100644 ui/core/app/controllers/scopes/scope/projects/new.js create mode 100644 ui/core/app/controllers/scopes/scope/projects/project.js create mode 100644 ui/core/app/routes/scopes/scope/projects.js create mode 100644 ui/core/app/routes/scopes/scope/projects/index.js create mode 100644 ui/core/app/routes/scopes/scope/projects/new.js create mode 100644 ui/core/app/routes/scopes/scope/projects/project.js create mode 100644 ui/core/app/routes/scopes/scope/projects/project/index.js create mode 100644 ui/core/app/templates/scopes/scope/projects.hbs create mode 100644 ui/core/app/templates/scopes/scope/projects/index.hbs create mode 100644 ui/core/app/templates/scopes/scope/projects/new.hbs create mode 100644 ui/core/app/templates/scopes/scope/projects/project.hbs create mode 100644 ui/core/app/templates/scopes/scope/projects/project/index.hbs create mode 100644 ui/core/tests/unit/controllers/scopes/scope/projects-test.js create mode 100644 ui/core/tests/unit/controllers/scopes/scope/projects/new-test.js create mode 100644 ui/core/tests/unit/controllers/scopes/scope/projects/project-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope/projects-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope/projects/index-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope/projects/new-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope/projects/project-test.js create mode 100644 ui/core/tests/unit/routes/scopes/scope/projects/project/index-test.js diff --git a/ui/core/app/controllers/scopes/scope/projects.js b/ui/core/app/controllers/scopes/scope/projects.js new file mode 100644 index 000000000..cdd9bda8a --- /dev/null +++ b/ui/core/app/controllers/scopes/scope/projects.js @@ -0,0 +1,19 @@ +import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; + +export default class ScopesScopeProjectsController extends Controller { + + // =services + + @service intl; + + // =attributes + + /** + * @type {string} + */ + get breadCrumb() { + return this.intl.t('resources.projects'); + } + +} diff --git a/ui/core/app/controllers/scopes/scope/projects/new.js b/ui/core/app/controllers/scopes/scope/projects/new.js new file mode 100644 index 000000000..50b30d187 --- /dev/null +++ b/ui/core/app/controllers/scopes/scope/projects/new.js @@ -0,0 +1,19 @@ +import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; + +export default class ScopesScopeProjectsNewController extends Controller { + + // =services + + @service intl; + + // =attributes + + /** + * @type {string} + */ + get breadCrumb() { + return this.intl.t('titles.new'); + } + +} diff --git a/ui/core/app/controllers/scopes/scope/projects/project.js b/ui/core/app/controllers/scopes/scope/projects/project.js new file mode 100644 index 000000000..8fc8bbb1c --- /dev/null +++ b/ui/core/app/controllers/scopes/scope/projects/project.js @@ -0,0 +1,14 @@ +import Controller from '@ember/controller'; + +export default class ScopesScopeProjectsProjectController extends Controller { + + // =attributes + + /** + * @type {string} + */ + get breadCrumb() { + return this.model.displayName; + } + +} diff --git a/ui/core/app/router.js b/ui/core/app/router.js index 6dbe00b3b..e07a9ee94 100644 --- a/ui/core/app/router.js +++ b/ui/core/app/router.js @@ -9,7 +9,12 @@ export default class Router extends EmberRouter { Router.map(function () { this.route('scopes', function() { - this.route('scope', { path: ':scope_id' }); + this.route('scope', { path: ':scope_id' }, function() { + this.route('projects', function() { + this.route('project', { path: ':project_id' }, function() {}); + this.route('new'); + }); + }); }); this.route('orgs', function () { diff --git a/ui/core/app/routes/scopes/scope.js b/ui/core/app/routes/scopes/scope.js index 68201166e..8cde487bd 100644 --- a/ui/core/app/routes/scopes/scope.js +++ b/ui/core/app/routes/scopes/scope.js @@ -2,10 +2,12 @@ import Route from '@ember/routing/route'; export default class ScopesScopeRoute extends Route { + // =methods + /** - * Attempts to load the specified scope from the API. This is allowed + * Attempt to load the specified scope from the API. This is allowed * to fail, since in some cases the user may not have permission to read a - * scope directly, but may have permission to read its resources. + * scope directly, but may have permission to read resources under it. * If the scope fails to load, we still proceed using a temporary scope object * consisting of only the specified ID. * @param {object} params @@ -16,10 +18,10 @@ export default class ScopesScopeRoute extends Route { try { return await this.store.findRecord('scope', id); } catch { - const maybeExisting = this.store.peekRecord('scope', id); - // TODO it's unclear if this is reliable... + const maybeExistingScope = this.store.peekRecord('scope', id); + // TODO it's unclear if creating a temporary unsaved scope is reliable... // we may need to revert to a POJO. - return maybeExisting || this.store.createRecord('scope', { id }); + return maybeExistingScope || this.store.createRecord('scope', { id }); } } diff --git a/ui/core/app/routes/scopes/scope/projects.js b/ui/core/app/routes/scopes/scope/projects.js new file mode 100644 index 000000000..f2a5fa77b --- /dev/null +++ b/ui/core/app/routes/scopes/scope/projects.js @@ -0,0 +1,84 @@ +import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; +import { action } from '@ember/object'; + +export default class ScopesScopeProjectsRoute extends Route { + + // =services + + @service notify; + + // =methods + + /** + * Loads all scopes under the current scope. + * @return {Promise{[ScopeModel]}} + */ + async model() { + const { id: scope_id } = this.modelFor('scopes.scope'); + return this.store.query('scope', { scope_id }); + } + + /** + * If arriving here within a verifiable project scope, redirect to its + * parent, which is necessarily an org, in order to list projects. + * + * TODO: this conditional redirect based on the scope type is probably + * necessary for many routes directly under scope, so we may want + * to move this into a mixin. + * @param {ScopeModel} + */ + redirect() { + const { isProject, scopeID } = this.modelFor('scopes.scope'); + if (isProject) this.transitionTo('scopes.scope.projects', scopeID); + } + + // =actions + + /** + * Rollback changes on project. + * @param {ProjectModel} project + */ + @action + cancel(project) { + const { isNew } = project; + project.rollbackAttributes(); + if (isNew) this.transitionTo('scopes.scope.projects'); + } + + /** + * Handle save project. + * @param {ProjectModel} project + * @param {Event} e + */ + @action + async save(project) { + try { + await project.save(); + // TODO: replace with translated strings + this.notify.success('Save succeeded.'); + this.transitionTo('scopes.scope.projects.project', project); + } catch (error) { + // TODO: replace with translated strings + this.notify.error(error.message, { closeAfter: null }); + } + } + + /** + * Deletes the project and redirects to index. + * @param {ProjectModel} project + */ + @action + async delete(project) { + try { + await project.destroyRecord(); + this.transitionTo('orgs.org.projects'); + // TODO: replace with translated strings + this.notify.success('Deleted project succesfully.'); + } catch (error) { + // TODO: replace with translated strings + this.notify.error(error.message, { closeAfter: null }); + } + } + +} diff --git a/ui/core/app/routes/scopes/scope/projects/index.js b/ui/core/app/routes/scopes/scope/projects/index.js new file mode 100644 index 000000000..294371052 --- /dev/null +++ b/ui/core/app/routes/scopes/scope/projects/index.js @@ -0,0 +1,4 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeProjectsIndexRoute extends Route { +} diff --git a/ui/core/app/routes/scopes/scope/projects/new.js b/ui/core/app/routes/scopes/scope/projects/new.js new file mode 100644 index 000000000..1781f5269 --- /dev/null +++ b/ui/core/app/routes/scopes/scope/projects/new.js @@ -0,0 +1,19 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeProjectsNewRoute extends Route { + + // =methods + + /** + * Creates a new unsaved project scope belonging to the current org scope. + * @return {ScopeModel} + */ + model() { + const { id } = this.modelFor('scopes.scope'); + return this.store.createRecord('scope', { + type: 'project', + scopeID: id + }); + } + +} diff --git a/ui/core/app/routes/scopes/scope/projects/project.js b/ui/core/app/routes/scopes/scope/projects/project.js new file mode 100644 index 000000000..c2aee8ed3 --- /dev/null +++ b/ui/core/app/routes/scopes/scope/projects/project.js @@ -0,0 +1,17 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeProjectsProjectRoute extends Route { + + // =methods + + /** + * Loads a scope by ID (ostensibly a project scope). + * @param {object} params + * @param {string} params.project_id + * @return {ScopeModel} + */ + model({ project_id }) { + return this.store.findRecord('scope', project_id); + } + +} diff --git a/ui/core/app/routes/scopes/scope/projects/project/index.js b/ui/core/app/routes/scopes/scope/projects/project/index.js new file mode 100644 index 000000000..f5511f2e7 --- /dev/null +++ b/ui/core/app/routes/scopes/scope/projects/project/index.js @@ -0,0 +1,4 @@ +import Route from '@ember/routing/route'; + +export default class ScopesScopeProjectsProjectIndexRoute extends Route { +} diff --git a/ui/core/app/templates/application.hbs b/ui/core/app/templates/application.hbs index 576e7875a..fbbecf1e7 100644 --- a/ui/core/app/templates/application.hbs +++ b/ui/core/app/templates/application.hbs @@ -1,3 +1,6 @@ + +{{title (app-name)}} + diff --git a/ui/core/app/templates/scopes/scope.hbs b/ui/core/app/templates/scopes/scope.hbs index e2147cab0..2d1a73607 100644 --- a/ui/core/app/templates/scopes/scope.hbs +++ b/ui/core/app/templates/scopes/scope.hbs @@ -1 +1,37 @@ -{{outlet}} \ No newline at end of file + + + + + + + {{t "resources.projects"}} + + + + {{outlet "sidebar"}} + + {{!-- + + + {{t "resources.users"}} + + + + {{t "resources.groups"}} + + + + {{t "resources.roles"}} + + + --}} + + + + +
+ {{outlet}} +
+ + + diff --git a/ui/core/app/templates/scopes/scope/projects.hbs b/ui/core/app/templates/scopes/scope/projects.hbs new file mode 100644 index 000000000..1f709c7f7 --- /dev/null +++ b/ui/core/app/templates/scopes/scope/projects.hbs @@ -0,0 +1,3 @@ +{{title (t "resources.projects")}} + +{{outlet}} diff --git a/ui/core/app/templates/scopes/scope/projects/index.hbs b/ui/core/app/templates/scopes/scope/projects/index.hbs new file mode 100644 index 000000000..c07e265ec --- /dev/null +++ b/ui/core/app/templates/scopes/scope/projects/index.hbs @@ -0,0 +1,56 @@ + + + + + + + +

{{t "resources.projects"}}

+ + + + {{#if @model}} + + {{t 'titles.new'}} + + {{/if}} + + + + + {{#if @model}} + + + {{#each @model as |project|}} + + {{/each}} + + + {{else}} + + + + + {{t 'descriptions.projects-welcome'}} + + + + {{t 'titles.new'}} + + + + + {{/if}} + + + + diff --git a/ui/core/app/templates/scopes/scope/projects/new.hbs b/ui/core/app/templates/scopes/scope/projects/new.hbs new file mode 100644 index 000000000..a8758e3c0 --- /dev/null +++ b/ui/core/app/templates/scopes/scope/projects/new.hbs @@ -0,0 +1,21 @@ +{{title (t 'titles.new')}} + + + + + + + + +

{{t "titles.new"}}

+ + + + + + + diff --git a/ui/core/app/templates/scopes/scope/projects/project.hbs b/ui/core/app/templates/scopes/scope/projects/project.hbs new file mode 100644 index 000000000..d66e83390 --- /dev/null +++ b/ui/core/app/templates/scopes/scope/projects/project.hbs @@ -0,0 +1,3 @@ +{{title @model.displayName}} + +{{outlet}} diff --git a/ui/core/app/templates/scopes/scope/projects/project/index.hbs b/ui/core/app/templates/scopes/scope/projects/project/index.hbs new file mode 100644 index 000000000..5dccae834 --- /dev/null +++ b/ui/core/app/templates/scopes/scope/projects/project/index.hbs @@ -0,0 +1,35 @@ + + + + + + + +

{{t "resources.project"}}

+ + + + + + {{t 'titles.details'}} + + + + + + + + + {{t 'actions.delete'}} + + + + diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 421f87a8f..d27476462 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -17,6 +17,13 @@ export default function() { // Scope resources + this.get('/scopes', ({ scopes }, { queryParams: { scope_id } }) => { + // Default parent scope is global + if (!scope_id) scope_id = 'global'; + return scopes.where(scope => { + return scope.scope ? scope.scope.id === scope_id : false + }); + }); this.post('/scopes'); // To simulate a possible real-world case, org scopes are not returned, // but project scopes are. diff --git a/ui/core/tests/unit/controllers/scopes/scope/projects-test.js b/ui/core/tests/unit/controllers/scopes/scope/projects-test.js new file mode 100644 index 000000000..21125f233 --- /dev/null +++ b/ui/core/tests/unit/controllers/scopes/scope/projects-test.js @@ -0,0 +1,12 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Controller | scopes/scope/projects', function(hooks) { + setupTest(hooks); + + // Replace this with your real tests. + test('it exists', function(assert) { + let controller = this.owner.lookup('controller:scopes/scope/projects'); + assert.ok(controller); + }); +}); diff --git a/ui/core/tests/unit/controllers/scopes/scope/projects/new-test.js b/ui/core/tests/unit/controllers/scopes/scope/projects/new-test.js new file mode 100644 index 000000000..eeb857274 --- /dev/null +++ b/ui/core/tests/unit/controllers/scopes/scope/projects/new-test.js @@ -0,0 +1,12 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Controller | scopes/scope/projects/new', function(hooks) { + setupTest(hooks); + + // Replace this with your real tests. + test('it exists', function(assert) { + let controller = this.owner.lookup('controller:scopes/scope/projects/new'); + assert.ok(controller); + }); +}); diff --git a/ui/core/tests/unit/controllers/scopes/scope/projects/project-test.js b/ui/core/tests/unit/controllers/scopes/scope/projects/project-test.js new file mode 100644 index 000000000..26a255430 --- /dev/null +++ b/ui/core/tests/unit/controllers/scopes/scope/projects/project-test.js @@ -0,0 +1,12 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Controller | scopes/scope/projects/project', function(hooks) { + setupTest(hooks); + + // Replace this with your real tests. + test('it exists', function(assert) { + let controller = this.owner.lookup('controller:scopes/scope/projects/project'); + assert.ok(controller); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope/projects-test.js b/ui/core/tests/unit/routes/scopes/scope/projects-test.js new file mode 100644 index 000000000..4a8dde55f --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope/projects-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope/projects', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope/projects'); + assert.ok(route); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope/projects/index-test.js b/ui/core/tests/unit/routes/scopes/scope/projects/index-test.js new file mode 100644 index 000000000..71c813a35 --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope/projects/index-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope/projects/index', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope/projects/index'); + assert.ok(route); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope/projects/new-test.js b/ui/core/tests/unit/routes/scopes/scope/projects/new-test.js new file mode 100644 index 000000000..6d22e1fa7 --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope/projects/new-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope/projects/new', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope/projects/new'); + assert.ok(route); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope/projects/project-test.js b/ui/core/tests/unit/routes/scopes/scope/projects/project-test.js new file mode 100644 index 000000000..54555a07c --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope/projects/project-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope/projects/project', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope/projects/project'); + assert.ok(route); + }); +}); diff --git a/ui/core/tests/unit/routes/scopes/scope/projects/project/index-test.js b/ui/core/tests/unit/routes/scopes/scope/projects/project/index-test.js new file mode 100644 index 000000000..c81b2f5a7 --- /dev/null +++ b/ui/core/tests/unit/routes/scopes/scope/projects/project/index-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Route | scopes/scope/projects/project/index', function(hooks) { + setupTest(hooks); + + test('it exists', function(assert) { + let route = this.owner.lookup('route:scopes/scope/projects/project/index'); + assert.ok(route); + }); +}); diff --git a/ui/core/translations/en-us.yaml b/ui/core/translations/en-us.yaml index bda14642a..e770de25c 100644 --- a/ui/core/translations/en-us.yaml +++ b/ui/core/translations/en-us.yaml @@ -6,7 +6,7 @@ actions: help: Need help? resources: user: User - users: Users + users: Users role: Role roles: Roles group: Group @@ -18,6 +18,7 @@ resources: titles: iam: IAM new: New + details: Details general: General overview: Overview settings: Settings From b7996f6271774fa2a143b917ab87e2d33e26fe9e Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 10:34:31 -0400 Subject: [PATCH 3/8] fix: normalize embedded scope IDs and ensure scope resources do not receive scope-prefixed URLs like other resources do --- addons/api/addon/adapters/application.js | 14 +++++++++---- addons/api/addon/serializers/application.js | 20 ++++++++++++++++++- .../tests/unit/adapters/application-test.js | 10 ++++++++++ .../unit/serializers/application-test.js | 15 +++++++------- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index 214b80d3c..bfc109b3f 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -71,10 +71,16 @@ export default class ApplicationAdapter extends RESTAdapter.extend( */ urlPrefix(path, parentURL, modelName, id, snapshot={}) { const prefix = super.urlPrefix(...arguments); - let scopePath = getWithDefault(snapshot, 'adapterOptions.scopeID', ''); - // Ensure a slash is added between prefix + scope path if needed. - if (scopePath && prefix.charAt(prefix.length - 1) !== '/') { - scopePath = `/scopes/${scopePath}`; + const isScope = modelName === 'scope'; + let scopePath = ''; + // Only non-scope resources need a scope path, since scope resources + // aren't technically "scoped" the same way. + if (!isScope) { + scopePath = getWithDefault(snapshot, 'adapterOptions.scopeID', ''); + // Ensure a slash is added between prefix + scope path if needed. + if (scopePath && prefix.charAt(prefix.length - 1) !== '/') { + scopePath = `/scopes/${scopePath}`; + } } return `${prefix}${scopePath}`; } diff --git a/addons/api/addon/serializers/application.js b/addons/api/addon/serializers/application.js index 84bcc6ed5..c839eb78c 100644 --- a/addons/api/addon/serializers/application.js +++ b/addons/api/addon/serializers/application.js @@ -1,5 +1,6 @@ import RESTSerializer from '@ember-data/serializer/rest'; import { underscore } from '@ember/string'; +import { get } from '@ember/object'; import { copy } from 'ember-copy'; /** @@ -117,7 +118,7 @@ export default class ApplicationSerializer extends RESTSerializer { primaryModelClass.modelName ); // Copy the unrooted payload under the expected root key name. - transformedPayload[payloadKey] = copy(payload); + transformedPayload[payloadKey] = copy(payload, true); // Return the result of normalizing the transformed payload. return super.normalizeSingleResponse( store, @@ -128,4 +129,21 @@ export default class ApplicationSerializer extends RESTSerializer { ); } + /** + * One consequence of using model fragments to represent the embedded scope + * is that they cannot have `id` fields. We still need to know scope IDs, + * so we copy the `scope.id` value into the `scope.scope_id` field. + * @override + * @see FragmentScope + * @param {Model} typeClass + * @param {Object} hash + * @return {Object} + */ + normalize(typeClass, hash) { + const normalizedHash = copy(hash, true); + const scopeID = get(normalizedHash, 'scope.id'); + if (scopeID) normalizedHash.scope.scope_id = scopeID; + return super.normalize(typeClass, normalizedHash); + } + } diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js index 4ea1b2ec5..bea9b215c 100644 --- a/addons/api/tests/unit/adapters/application-test.js +++ b/addons/api/tests/unit/adapters/application-test.js @@ -35,6 +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); + const scopeID = 'global'; + const adapter = this.owner.lookup('adapter:application'); + const prefix = adapter.urlPrefix(null, null, 'scope', null, { + adapterOptions: {scopeID} + }); + assert.equal(prefix, `v1`); + }); + test('it generates correct default URL suffixes', function (assert) { assert.expect(1); const adapter = this.owner.lookup('adapter:application'); diff --git a/addons/api/tests/unit/serializers/application-test.js b/addons/api/tests/unit/serializers/application-test.js index 83c75341b..4769cfa3f 100644 --- a/addons/api/tests/unit/serializers/application-test.js +++ b/addons/api/tests/unit/serializers/application-test.js @@ -36,9 +36,9 @@ module('Unit | Serializer | application', function (hooks) { const userModelClass = store.createRecord('user').constructor; const payload = { items: [ - { id: 1, name: 'User 1' }, - { id: 2, name: 'User 2' }, - { id: 3, name: 'User 3' }, + { id: 1, name: 'User 1', scope: { id: 'o_123' } }, + { id: 2, name: 'User 2', scope: { id: 'o_123' } }, + { id: 3, name: 'User 3', scope: { id: 'o_123' } }, ], }; const normalizedArray = serializer.normalizeArrayResponse( @@ -52,19 +52,19 @@ module('Unit | Serializer | application', function (hooks) { { id: '1', type: 'user', - attributes: { name: 'User 1' }, + attributes: { name: 'User 1', scope: { scope_id: 'o_123' } }, relationships: {}, }, { id: '2', type: 'user', - attributes: { name: 'User 2' }, + attributes: { name: 'User 2', scope: { scope_id: 'o_123' } }, relationships: {}, }, { id: '3', type: 'user', - attributes: { name: 'User 3' }, + attributes: { name: 'User 3', scope: { scope_id: 'o_123' } }, relationships: {}, }, ], @@ -79,6 +79,7 @@ module('Unit | Serializer | application', function (hooks) { const payload = { id: '1', name: 'User 1', + scope: { id: 'o_123' } }; const normalized = serializer.normalizeSingleResponse( store, @@ -90,7 +91,7 @@ module('Unit | Serializer | application', function (hooks) { data: { id: '1', type: 'user', - attributes: { name: 'User 1' }, + attributes: { name: 'User 1', scope: { scope_id: 'o_123' } }, relationships: {}, }, }); From b4e8b1be8b90573925285ae4df98aaa05b31d1ba Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 11:31:46 -0400 Subject: [PATCH 4/8] chore: add projects CRUDL functionality --- ui/core/app/routes/scopes/scope/projects.js | 4 +++- ui/core/mirage/config.js | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/ui/core/app/routes/scopes/scope/projects.js b/ui/core/app/routes/scopes/scope/projects.js index f2a5fa77b..ebc5e97e5 100644 --- a/ui/core/app/routes/scopes/scope/projects.js +++ b/ui/core/app/routes/scopes/scope/projects.js @@ -55,6 +55,7 @@ export default class ScopesScopeProjectsRoute extends Route { async save(project) { try { await project.save(); + this.refresh(); // TODO: replace with translated strings this.notify.success('Save succeeded.'); this.transitionTo('scopes.scope.projects.project', project); @@ -72,7 +73,8 @@ export default class ScopesScopeProjectsRoute extends Route { async delete(project) { try { await project.destroyRecord(); - this.transitionTo('orgs.org.projects'); + this.refresh(); + this.transitionTo('scopes.scope.projects'); // TODO: replace with translated strings this.notify.success('Deleted project succesfully.'); } catch (error) { diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index d27476462..6e554c34a 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -24,7 +24,16 @@ export default function() { return scope.scope ? scope.scope.id === scope_id : false }); }); - this.post('/scopes'); + this.post('/scopes', function ({ scopes }, request) { + // Parent scope comes through the payload via `scope_id`, but this needs + // to be expanded to a scope object `scope: {id:..., type:...}` before + // saving, which is the gist of this function. + const attrs = this.normalizedRequestAttrs(); + const parentScopeAttrs = this.serialize(scopes.find(attrs.scopeId)); + delete attrs.scopeId; + attrs.scope = parentScopeAttrs; + return scopes.create(attrs); + }); // To simulate a possible real-world case, org scopes are not returned, // but project scopes are. // TODO this should be expanded to support other scenarios. From 279d552eca49fe21338d2eca5a44cdb7670aaa08 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 14:50:48 -0400 Subject: [PATCH 5/8] test: update projects acceptance tests to work with scopes --- ui/core/tests/acceptance/projects-test.js | 88 +++++++++++++---------- ui/core/tests/helpers/.gitkeep | 0 2 files changed, 50 insertions(+), 38 deletions(-) delete mode 100644 ui/core/tests/helpers/.gitkeep diff --git a/ui/core/tests/acceptance/projects-test.js b/ui/core/tests/acceptance/projects-test.js index 0e7877c97..580a4a443 100644 --- a/ui/core/tests/acceptance/projects-test.js +++ b/ui/core/tests/acceptance/projects-test.js @@ -9,41 +9,57 @@ module('Acceptance | projects', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - hooks.beforeEach(function () { - this.server.create('org'); - }); + let org; + let orgID; + let projectsURL; + let newProjectURL; + let existingProject; + let existingProjectURL; + let getProjectScopesCount; + let getFirstProjectScope; + let initialProjectScopesCount; + hooks.beforeEach(function () { + org = this.server.create('scope', {type: 'org'}, 'withChildren'); + getProjectScopesCount = () => this.server.schema.scopes.where({type: 'project'}).length; + getFirstProjectScope = () => this.server.schema.scopes.where({type: 'project'}).models[0]; + initialProjectScopesCount = getProjectScopesCount(); + orgID = org.id; + projectsURL = `/scopes/${orgID}/projects`; + newProjectURL = `/scopes/${orgID}/projects/new`; + existingProject = getFirstProjectScope(); + existingProjectURL = `/scopes/${orgID}/projects/${existingProject.id}`; + }); test('visiting projects', async function (assert) { assert.expect(1); this.server.createList('project', 1); - await visit('/orgs/1/projects'); + await visit(projectsURL); await a11yAudit(); - assert.equal(currentURL(), '/orgs/1/projects'); + assert.equal(currentURL(), projectsURL); }); test('can create new projects', async function (assert) { - assert.expect(3); - assert.equal(this.server.db.projects.length, 0); - await visit('/orgs/1/projects/new'); + assert.expect(1); + await visit(newProjectURL); + await a11yAudit(); await fillIn('[name="name"]', 'random string'); await click('[type="submit"]'); - assert.equal(currentURL(), '/orgs/1/projects/1'); - assert.equal(this.server.db.projects.length, 1); + assert.equal(getProjectScopesCount(), initialProjectScopesCount + 1); }); test('can cancel create new projects', async function (assert) { assert.expect(3); - assert.equal(this.server.db.projects.length, 0); - await visit('/orgs/1/projects/new'); + assert.equal(getProjectScopesCount(), initialProjectScopesCount); + await visit(newProjectURL); await fillIn('[name="name"]', 'random string'); await click('.rose-form-actions [type="button"]'); - assert.equal(currentURL(), '/orgs/1/projects'); - assert.equal(this.server.db.projects.length, 0); + assert.equal(currentURL(), projectsURL); + assert.equal(getProjectScopesCount(), initialProjectScopesCount); }); test('saving a new project with invalid fields displays error messages', async function (assert) { assert.expect(2); - this.server.post('/orgs/:org_id/projects', () => { + this.server.post('/scopes', () => { return new Response( 400, {}, @@ -62,8 +78,7 @@ module('Acceptance | projects', function (hooks) { } ); }); - await visit('/orgs/1/projects/new'); - await fillIn('[name="name"]', 'random string'); + await visit(newProjectURL); await click('[type="submit"]'); assert.ok( find('[role="alert"]').textContent.trim(), @@ -78,37 +93,35 @@ module('Acceptance | projects', function (hooks) { }); test('can save changes to existing project', async function (assert) { - assert.expect(2); - this.server.createList('project', 1, {name: 'MyProject'}); - await visit('/orgs/1/projects/1'); + assert.expect(3); + assert.notEqual(existingProject.name, 'random string'); + await visit(existingProjectURL); await fillIn('[name="name"]', 'random string'); await click('.rose-form-actions [type="submit"]'); - assert.equal(currentURL(), '/orgs/1/projects/1'); - assert.equal(this.server.db.projects[0].name, 'random string'); + assert.equal(currentURL(), existingProjectURL); + assert.equal(getFirstProjectScope().name, 'random string'); }); test('can cancel changes to existing project', async function (assert) { - assert.expect(1); - this.server.createList('project', 1, {name: 'MyProject'}); - await visit('/orgs/1/projects/1'); + assert.expect(2); + await visit(existingProjectURL); await fillIn('[name="name"]', 'random string'); await click('.rose-form-actions [type="button"]'); - assert.equal(find('[name="name"]').value, 'MyProject'); + assert.notEqual(existingProject.name, 'random string'); + assert.equal(find('[name="name"]').value, existingProject.name); }); test('can delete project', async function(assert) { - assert.expect(2); - this.server.createList('project', 1); - assert.equal(this.server.db.projects.length, 1); - await visit('/orgs/1/projects/1'); + assert.expect(1); + await visit(existingProjectURL); await click('.rose-button-warning'); - assert.equal(this.server.db.projects.length, 0); + assert.equal(getProjectScopesCount(), initialProjectScopesCount - 1); }); test('saving an existing project with invalid fields displays error messages', async function (assert) { assert.expect(2); this.server.createList('project', 1); - this.server.patch('/orgs/:org_id/projects/:id', () => { + this.server.patch('/scopes/:id', () => { return new Response( 400, {}, @@ -127,7 +140,7 @@ module('Acceptance | projects', function (hooks) { } ); }); - await visit('/orgs/1/projects/1'); + await visit(existingProjectURL); await fillIn('[name="name"]', 'random string'); await click('[type="submit"]'); assert.ok( @@ -144,8 +157,7 @@ module('Acceptance | projects', function (hooks) { test('errors are displayed when save project fails', async function (assert) { assert.expect(1); - this.server.createList('project', 1); - this.server.patch('/orgs/:org_id/projects/:id', () => { + this.server.patch('/scopes/:id', () => { return new Response( 490, {}, @@ -156,7 +168,7 @@ module('Acceptance | projects', function (hooks) { } ); }); - await visit('/orgs/1/projects/1'); + await visit(existingProjectURL); await fillIn('[name="name"]', 'random string'); await click('[type="submit"]'); assert.ok( @@ -169,7 +181,7 @@ module('Acceptance | projects', function (hooks) { test('errors are displayed when delete project fails', async function (assert) { assert.expect(1); this.server.createList('project', 1); - this.server.del('/orgs/:org_id/projects/:id', () => { + this.server.del('/scopes/:id', () => { return new Response( 490, {}, @@ -180,7 +192,7 @@ module('Acceptance | projects', function (hooks) { } ); }); - await visit('/orgs/1/projects/1'); + await visit(existingProjectURL); await click('.rose-button-warning'); assert.ok( find('[role="alert"]').textContent.trim(), diff --git a/ui/core/tests/helpers/.gitkeep b/ui/core/tests/helpers/.gitkeep deleted file mode 100644 index e69de29bb..000000000 From 1b7a8902df7be46e0d9f62b37fa840d4ba5f4381 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 16:11:26 -0400 Subject: [PATCH 6/8] test: add projects path redirection test --- ui/core/tests/acceptance/projects-test.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/ui/core/tests/acceptance/projects-test.js b/ui/core/tests/acceptance/projects-test.js index 580a4a443..57ba7c17e 100644 --- a/ui/core/tests/acceptance/projects-test.js +++ b/ui/core/tests/acceptance/projects-test.js @@ -30,18 +30,26 @@ module('Acceptance | projects', function (hooks) { existingProject = getFirstProjectScope(); existingProjectURL = `/scopes/${orgID}/projects/${existingProject.id}`; }); + test('visiting projects', async function (assert) { assert.expect(1); - this.server.createList('project', 1); await visit(projectsURL); await a11yAudit(); assert.equal(currentURL(), projectsURL); }); + test('visiting projects within a project scope redirects to the parent org scope', async function (assert) { + assert.expect(2); + const wrongProjectsURL = `/scopes/${existingProject.id}/projects`; + assert.notEqual(wrongProjectsURL, projectsURL); + await visit(wrongProjectsURL); + await a11yAudit(); + assert.equal(currentURL(), projectsURL, 'Wrong projects path was redirected to correct path.'); + }); + test('can create new projects', async function (assert) { assert.expect(1); await visit(newProjectURL); - await a11yAudit(); await fillIn('[name="name"]', 'random string'); await click('[type="submit"]'); assert.equal(getProjectScopesCount(), initialProjectScopesCount + 1); @@ -120,7 +128,6 @@ module('Acceptance | projects', function (hooks) { test('saving an existing project with invalid fields displays error messages', async function (assert) { assert.expect(2); - this.server.createList('project', 1); this.server.patch('/scopes/:id', () => { return new Response( 400, @@ -180,7 +187,6 @@ module('Acceptance | projects', function (hooks) { test('errors are displayed when delete project fails', async function (assert) { assert.expect(1); - this.server.createList('project', 1); this.server.del('/scopes/:id', () => { return new Response( 490, From d08d52e0900688b5626044cc4d1233f9e0d5ef0a Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 19:01:31 -0400 Subject: [PATCH 7/8] refactor: send scope_id query param on scope create, rather than serializing into the payload --- addons/api/addon/adapters/scope.js | 27 ++++++++++++++ addons/api/addon/serializers/scope.js | 36 ------------------- addons/api/app/adapters/scope.js | 1 + addons/api/app/serializers/scope.js | 1 - addons/api/tests/unit/adapters/scope-test.js | 23 ++++++++++++ .../api/tests/unit/serializers/scope-test.js | 32 ----------------- ui/core/mirage/config.js | 3 +- 7 files changed, 52 insertions(+), 71 deletions(-) create mode 100644 addons/api/addon/adapters/scope.js delete mode 100644 addons/api/addon/serializers/scope.js create mode 100644 addons/api/app/adapters/scope.js delete mode 100644 addons/api/app/serializers/scope.js create mode 100644 addons/api/tests/unit/adapters/scope-test.js delete mode 100644 addons/api/tests/unit/serializers/scope-test.js diff --git a/addons/api/addon/adapters/scope.js b/addons/api/addon/adapters/scope.js new file mode 100644 index 000000000..6d4549844 --- /dev/null +++ b/addons/api/addon/adapters/scope.js @@ -0,0 +1,27 @@ +import ApplicationAdapter from './application'; +import { serializeIntoHash } from '@ember-data/adapter/-private'; + +export default class ScopeAdapter extends ApplicationAdapter { + + /** + * Identical to default method except that we pass a query parameter + * indicating the parent scope. + * @override + * @param {Store} store + * @param {Model} type + * @param {Snapshot} snapshot + * @return {Promise} promise + */ + createRecord(store, type, snapshot) { + const parentScopeID = snapshot.attr('scope').attr('scope_id'); + let url = + this.buildURL(type.modelName, null, snapshot, 'createRecord'); + // TODO this should be generalized into `buildURL`, but currently Ember Data + // doesn't provide a way to manage query parameters. They are passed on + // to the underlying ajax/fetch service. + url = `${url}?scope_id=${parentScopeID}`; + const data = serializeIntoHash(store, type, snapshot); + return this.ajax(url, 'POST', { data }); + } + +} diff --git a/addons/api/addon/serializers/scope.js b/addons/api/addon/serializers/scope.js deleted file mode 100644 index 5767a8015..000000000 --- a/addons/api/addon/serializers/scope.js +++ /dev/null @@ -1,36 +0,0 @@ -import ApplicationSerializer from './application'; - -export default class ScopeSerializer extends ApplicationSerializer { - - // =attributes - - attrs = { - scope: { serialize: true } - }; - - // =methods - - /** - * While most resources do not serialize scope into a request body (scope - * is expressed in the URL), scopes themselves _do serialize scope_ as a - * string field `scope_id`. - * Otherwise delegate to default attribute serializer. - * - * @override - * @method serializeAttribute - * @param {Snapshot} snapshot - * @param {Object} json - * @param {String} key - * @param {Object} attribute - */ - serializeAttribute(snapshot, json, key) { - const value = super.serializeAttribute(...arguments); - const parentScope = snapshot.attr('scope'); - if (key === 'scope' && this._canSerialize(key) && parentScope) { - json.scope_id = parentScope.attr('scope_id'); - delete json[key]; - } - return value; - } - -} diff --git a/addons/api/app/adapters/scope.js b/addons/api/app/adapters/scope.js new file mode 100644 index 000000000..10878851f --- /dev/null +++ b/addons/api/app/adapters/scope.js @@ -0,0 +1 @@ +export { default } from 'api/adapters/scope'; diff --git a/addons/api/app/serializers/scope.js b/addons/api/app/serializers/scope.js deleted file mode 100644 index cdc15f3a2..000000000 --- a/addons/api/app/serializers/scope.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from 'api/serializers/scope'; diff --git a/addons/api/tests/unit/adapters/scope-test.js b/addons/api/tests/unit/adapters/scope-test.js new file mode 100644 index 000000000..7dcd482c7 --- /dev/null +++ b/addons/api/tests/unit/adapters/scope-test.js @@ -0,0 +1,23 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; + +module('Unit | Adapter | scope', function(hooks) { + setupTest(hooks); + setupMirage(hooks); + + test('it generates correct createRecord URLs with a scope_id query parameter', function (assert) { + assert.expect(1); + this.server.post('/v1/scopes', (schema, { queryParams }) =>{ + assert.equal(queryParams.scope_id, 'o_1'); + }); + const store = this.owner.lookup('service:store'); + const scope = store.createRecord('scope', { + scopeID: 'o_1' + }); + const type = scope.constructor; + const snapshot = scope._createSnapshot(); + const adapter = this.owner.lookup('adapter:scope'); + adapter.createRecord(store, type, snapshot); + }); +}); diff --git a/addons/api/tests/unit/serializers/scope-test.js b/addons/api/tests/unit/serializers/scope-test.js deleted file mode 100644 index 3993238fe..000000000 --- a/addons/api/tests/unit/serializers/scope-test.js +++ /dev/null @@ -1,32 +0,0 @@ -import { module, test } from 'qunit'; -import { setupTest } from 'ember-qunit'; - -module('Unit | Serializer | scope', function(hooks) { - setupTest(hooks); - - // Replace this with your real tests. - test('it exists', function(assert) { - let store = this.owner.lookup('service:store'); - let serializer = store.serializerFor('scope'); - - assert.ok(serializer); - }); - - test('it serializes records', function (assert) { - assert.expect(1); - const store = this.owner.lookup('service:store'); - const record = store.createRecord('scope', { - type: 'org', - name: 'User', - description: 'Description', - scopeID: 'global' - }); - const serializedRecord = record.serialize(); - assert.deepEqual(serializedRecord, { - type: 'org', - name: 'User', - description: 'Description', - scope_id: 'global' - }); - }); -}); diff --git a/ui/core/mirage/config.js b/ui/core/mirage/config.js index 6e554c34a..ed6859c83 100644 --- a/ui/core/mirage/config.js +++ b/ui/core/mirage/config.js @@ -29,8 +29,7 @@ export default function() { // to be expanded to a scope object `scope: {id:..., type:...}` before // saving, which is the gist of this function. const attrs = this.normalizedRequestAttrs(); - const parentScopeAttrs = this.serialize(scopes.find(attrs.scopeId)); - delete attrs.scopeId; + const parentScopeAttrs = this.serialize(scopes.find(request.queryParams.scope_id)); attrs.scope = parentScopeAttrs; return scopes.create(attrs); }); From 26388b8527239f34b6c2591e0a8daaf0d754b5d8 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Thu, 23 Jul 2020 19:31:34 -0400 Subject: [PATCH 8/8] refactor: deduce scope path prefix in adapter rather than passing it via the model save method --- addons/api/addon/adapters/application.js | 32 +++++++++++++------ addons/api/addon/models/base.js | 28 ---------------- .../tests/unit/adapters/application-test.js | 10 +++--- 3 files changed, 29 insertions(+), 41 deletions(-) diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index bfc109b3f..09b772cb7 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -59,8 +59,13 @@ export default class ApplicationAdapter extends RESTAdapter.extend( // =methods /** - * Prepends a scope to the URL prefix, if a scope is passed via the - * snapshot's `adapterOptions.scopeID` field. + * Prepends a scope to the URL prefix. Precedence is given to the ID passed + * via `adapterOptions.scopeID`. If this value isn't found in + * `adapterOptions`, we look to the `scope.scope_id` field on the snapshot, + * which most resources have. + * + * Note: scopes themselves never receive a scope URL prefix. + * * @override * @param {string} path * @param {string} parentURL @@ -69,18 +74,27 @@ export default class ApplicationAdapter extends RESTAdapter.extend( * @param {object} snapshot * @return {string} */ - urlPrefix(path, parentURL, modelName, id, snapshot={}) { + urlPrefix(path, parentURL, modelName, id, snapshot) { const prefix = super.urlPrefix(...arguments); const isScope = modelName === 'scope'; let scopePath = ''; + let scopeID = ''; + if (snapshot) { + // Not all snapshots have `attr` (such as array snapshots), + // so we do this sort of ugly check. + if (snapshot.attr) { + const parentScope = snapshot.attr('scope'); + if (parentScope) scopeID = parentScope.attr('scope_id'); + } + // Attempt to get scopeID from adapterOptions and fallback on its current + // value if it wasn't passed through options. + scopeID = getWithDefault(snapshot, 'adapterOptions.scopeID', scopeID); + } // Only non-scope resources need a scope path, since scope resources // aren't technically "scoped" the same way. - if (!isScope) { - scopePath = getWithDefault(snapshot, 'adapterOptions.scopeID', ''); - // Ensure a slash is added between prefix + scope path if needed. - if (scopePath && prefix.charAt(prefix.length - 1) !== '/') { - scopePath = `/scopes/${scopePath}`; - } + // Ensure a slash is added between prefix + scope path if needed. + if (!isScope && scopeID && prefix.charAt(prefix.length - 1) !== '/') { + scopePath = `/scopes/${scopeID}`; } return `${prefix}${scopePath}`; } diff --git a/addons/api/addon/models/base.js b/addons/api/addon/models/base.js index 635d3b831..cfbcce14b 100644 --- a/addons/api/addon/models/base.js +++ b/addons/api/addon/models/base.js @@ -84,32 +84,4 @@ export default class BaseModel extends Model { return !this.canSave; } - // =methods - - /** - * If this model instance has a related scope, the scope's ID will be passed - * via `adapterOptions.scopeID` automatically. End users may optionally - * pass a custom scope ID to override. - * - * @example - * model.save(); // passes the model's current scope ID - * model.save({adapterOptions: {scopeID: 'global'}}); // override scope ID - * - * End users may specify a _custom method_ via `adapterOptions.method`. - * Custom methods are appended to the record's URL by the adapter. - * - * @example - * // Results in a URL: `/v1/model-name/id:my-custom-method` - * model.save({adapterOptions: {method: 'my-custom-method'}}); - * - * @return {Promise} - */ - save(options={adapterOptions: {}}) { - const scopeID = this.scopeID; - if (scopeID && !options.adapterOptions.scopeID) { - options.adapterOptions.scopeID = scopeID; - } - return super.save(options); - } - } diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js index bea9b215c..c3d72d469 100644 --- a/addons/api/tests/unit/adapters/application-test.js +++ b/addons/api/tests/unit/adapters/application-test.js @@ -29,7 +29,7 @@ module('Unit | Adapter | application', function (hooks) { const scopeID = 'global'; const adapter = this.owner.lookup('adapter:application'); const prefix = adapter.urlPrefix(null, null, null, null, { - adapterOptions: {scopeID} + adapterOptions: { scopeID } }); assert.ok(config.api.namespace); assert.equal(prefix, `v1/scopes/${scopeID}`); @@ -40,7 +40,7 @@ module('Unit | Adapter | application', function (hooks) { const scopeID = 'global'; const adapter = this.owner.lookup('adapter:application'); const prefix = adapter.urlPrefix(null, null, 'scope', null, { - adapterOptions: {scopeID} + adapterOptions: { scopeID } }); assert.equal(prefix, `v1`); }); @@ -56,7 +56,7 @@ module('Unit | Adapter | application', function (hooks) { const method = 'set-something'; const adapter = this.owner.lookup('adapter:application'); const suffix = adapter.urlSuffix(null, null, { - adapterOptions: {method} + adapterOptions: { method } }); assert.ok(config.api.namespace); assert.equal(suffix, ':set-something'); @@ -95,7 +95,9 @@ module('Unit | Adapter | application', function (hooks) { assert.ok(true, 'Scoped resource URL was requested.'); return {items: []}; }); - await store.findAll('group', {adapterOptions: {scopeID: 'p_456'}}); + await store.findAll('group', { + adapterOptions: { scopeID: 'p_456' } + }); }); test('it rewrites PUT to PATCH, but leaves others unchanged', function (assert) {