From ea460c139100e71079aa85a9e1830e936b427dbe Mon Sep 17 00:00:00 2001 From: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> Date: Mon, 10 Jun 2024 17:00:57 -0700 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20remove=20route-actio?= =?UTF-8?q?n=20usage=20for=20users=20(#2299)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 remove route-action usage for users ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-12612 --- .../components/users/user/actions/index.hbs | 2 +- .../controllers/scopes/scope/users/index.js | 72 +++++++++++++++ .../app/controllers/scopes/scope/users/new.js | 5 ++ .../scopes/scope/users/user/accounts.js | 5 ++ .../scopes/scope/users/user/add-accounts.js | 36 ++++++++ .../scopes/scope/users/user/index.js | 5 ++ ui/admin/app/routes/scopes/scope/users.js | 53 ----------- .../scopes/scope/users/user/accounts.js | 20 ----- .../scopes/scope/users/user/add-accounts.js | 30 ------- .../app/templates/scopes/scope/users/new.hbs | 4 +- .../scopes/scope/users/user/accounts.hbs | 7 +- .../scopes/scope/users/user/add-accounts.hbs | 4 +- .../scopes/scope/users/user/index.hbs | 6 +- .../scopes/scope/users/index-test.js | 87 ++++++++++++++++++- .../scopes/scope/users/new-test.js | 11 +++ .../scopes/scope/users/user/accounts-test.js | 16 ++++ .../scope/users/user/add-accounts-test.js | 63 ++++++++++++++ .../scopes/scope/users/user/index-test.js | 13 +++ 18 files changed, 323 insertions(+), 116 deletions(-) create mode 100644 ui/admin/app/controllers/scopes/scope/users/new.js create mode 100644 ui/admin/app/controllers/scopes/scope/users/user/accounts.js create mode 100644 ui/admin/app/controllers/scopes/scope/users/user/add-accounts.js create mode 100644 ui/admin/app/controllers/scopes/scope/users/user/index.js create mode 100644 ui/admin/tests/unit/controllers/scopes/scope/users/new-test.js create mode 100644 ui/admin/tests/unit/controllers/scopes/scope/users/user/accounts-test.js create mode 100644 ui/admin/tests/unit/controllers/scopes/scope/users/user/add-accounts-test.js create mode 100644 ui/admin/tests/unit/controllers/scopes/scope/users/user/index-test.js diff --git a/ui/admin/app/components/users/user/actions/index.hbs b/ui/admin/app/components/users/user/actions/index.hbs index bd5564e4d..7e6ca2d24 100644 --- a/ui/admin/app/components/users/user/actions/index.hbs +++ b/ui/admin/app/components/users/user/actions/index.hbs @@ -18,7 +18,7 @@ {{t 'resources.user.actions.delete'}} diff --git a/ui/admin/app/controllers/scopes/scope/users/index.js b/ui/admin/app/controllers/scopes/scope/users/index.js index a8ab88da9..b7fe9b731 100644 --- a/ui/admin/app/controllers/scopes/scope/users/index.js +++ b/ui/admin/app/controllers/scopes/scope/users/index.js @@ -4,11 +4,20 @@ */ import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { debounce } from 'core/decorators/debounce'; +import { loading } from 'ember-loading'; +import { confirm } from 'core/decorators/confirm'; +import { notifySuccess, notifyError } from 'core/decorators/notify'; export default class ScopesScopeUsersIndexController extends Controller { + // =services + + @service router; + @service can; + // =attributes queryParams = ['search', 'page', 'pageSize']; @@ -30,4 +39,67 @@ export default class ScopesScopeUsersIndexController extends Controller { this.search = value; this.page = 1; } + + // =actions + + /** + * Rollback changes on an user. + * @param {UserModel} user + */ + @action + cancel(user) { + const { isNew } = user; + user.rollbackAttributes(); + if (isNew) this.router.transitionTo('scopes.scope.users'); + } + + /** + * Save an user in current scope. + * @param {UserModel} user + * @param {Event} e + */ + @action + @loading + @notifyError(({ message }) => message) + @notifySuccess(({ isNew }) => + isNew ? 'notifications.create-success' : 'notifications.save-success', + ) + async save(user) { + await user.save(); + if (this.can.can('read model', user)) { + await this.router.transitionTo('scopes.scope.users.user', user); + } else { + await this.router.transitionTo('scopes.scope.users'); + } + await this.router.refresh(); + } + + /** + * Delete user in current scope and redirect to index. + * @param {UserModel} user + */ + @action + @loading + @confirm('questions.delete-confirm') + @notifyError(({ message }) => message, { catch: true }) + @notifySuccess('notifications.delete-success') + async delete(user) { + await user.destroyRecord(); + this.router.replaceWith('scopes.scope.users'); + await this.router.refresh(); + } + + /** + * Remove an account from the current role and redirect to accounts index. + * @param {UserModel} user + * @param {AccountModel} account + */ + @action + @loading + @notifyError(({ message }) => message, { catch: true }) + @notifySuccess('notifications.remove-success') + async removeAccount(user, account) { + await user.removeAccount(account.id); + await this.router.refresh(); + } } diff --git a/ui/admin/app/controllers/scopes/scope/users/new.js b/ui/admin/app/controllers/scopes/scope/users/new.js new file mode 100644 index 000000000..cfe41994f --- /dev/null +++ b/ui/admin/app/controllers/scopes/scope/users/new.js @@ -0,0 +1,5 @@ +import Controller, { inject as controller } from '@ember/controller'; + +export default class ScopesScopeUsersNewController extends Controller { + @controller('scopes/scope/users/index') users; +} diff --git a/ui/admin/app/controllers/scopes/scope/users/user/accounts.js b/ui/admin/app/controllers/scopes/scope/users/user/accounts.js new file mode 100644 index 000000000..51bb29afb --- /dev/null +++ b/ui/admin/app/controllers/scopes/scope/users/user/accounts.js @@ -0,0 +1,5 @@ +import Controller, { inject as controller } from '@ember/controller'; + +export default class ScopesScopeUsersUserAccountsController extends Controller { + @controller('scopes/scope/users/index') users; +} diff --git a/ui/admin/app/controllers/scopes/scope/users/user/add-accounts.js b/ui/admin/app/controllers/scopes/scope/users/user/add-accounts.js new file mode 100644 index 000000000..f12ce84ad --- /dev/null +++ b/ui/admin/app/controllers/scopes/scope/users/user/add-accounts.js @@ -0,0 +1,36 @@ +import Controller from '@ember/controller'; +import { inject as service } from '@ember/service'; +import { action } from '@ember/object'; +import { loading } from 'ember-loading'; +import { notifySuccess, notifyError } from 'core/decorators/notify'; + +export default class ScopesScopeUsersUserAddAccountsController extends Controller { + // =services + + @service router; + + // =actions + + /** + * Adds accounts to the user and saves, replaces with the accounts index + * route, and notifies the user of success or error. + * @param {UserModel} user + * @param {[string]} accountIDs + */ + @action + @loading + @notifyError(({ message }) => message, { catch: true }) + @notifySuccess('notifications.add-success') + async addAccounts(user, accountIDs) { + await user.addAccounts(accountIDs); + await this.router.replaceWith('scopes.scope.users.user.accounts'); + } + + /** + * Redirect to user accounts as if nothing ever happened. + */ + @action + cancel() { + this.router.replaceWith('scopes.scope.users.user.accounts'); + } +} diff --git a/ui/admin/app/controllers/scopes/scope/users/user/index.js b/ui/admin/app/controllers/scopes/scope/users/user/index.js new file mode 100644 index 000000000..77a081cfe --- /dev/null +++ b/ui/admin/app/controllers/scopes/scope/users/user/index.js @@ -0,0 +1,5 @@ +import Controller, { inject as controller } from '@ember/controller'; + +export default class ScopesScopeUsersUserIndexController extends Controller { + @controller('scopes/scope/users/index') users; +} diff --git a/ui/admin/app/routes/scopes/scope/users.js b/ui/admin/app/routes/scopes/scope/users.js index ed0ce6cc6..fea406f6f 100644 --- a/ui/admin/app/routes/scopes/scope/users.js +++ b/ui/admin/app/routes/scopes/scope/users.js @@ -5,16 +5,11 @@ import Route from '@ember/routing/route'; import { inject as service } from '@ember/service'; -import { action } from '@ember/object'; -import { loading } from 'ember-loading'; -import { confirm } from 'core/decorators/confirm'; -import { notifySuccess, notifyError } from 'core/decorators/notify'; export default class ScopesScopeUsersRoute extends Route { // =services @service session; - @service can; @service router; // =methods @@ -25,52 +20,4 @@ export default class ScopesScopeUsersRoute extends Route { beforeModel() { if (!this.session.isAuthenticated) this.router.transitionTo('index'); } - - // =actions - /** - * Rollback changes on an user. - * @param {UserModel} user - */ - @action - cancel(user) { - const { isNew } = user; - user.rollbackAttributes(); - if (isNew) this.router.transitionTo('scopes.scope.users'); - } - - /** - * Save an user in current scope. - * @param {UserModel} user - * @param {Event} e - */ - @action - @loading - @notifyError(({ message }) => message) - @notifySuccess(({ isNew }) => - isNew ? 'notifications.create-success' : 'notifications.save-success', - ) - async save(user) { - await user.save(); - if (this.can.can('read model', user)) { - await this.router.transitionTo('scopes.scope.users.user', user); - } else { - await this.router.transitionTo('scopes.scope.users'); - } - this.refresh(); - } - - /** - * Delete user in current scope and redirect to index. - * @param {UserModel} user - */ - @action - @loading - @confirm('questions.delete-confirm') - @notifyError(({ message }) => message, { catch: true }) - @notifySuccess('notifications.delete-success') - async delete(user) { - await user.destroyRecord(); - this.router.replaceWith('scopes.scope.users'); - this.refresh(); - } } diff --git a/ui/admin/app/routes/scopes/scope/users/user/accounts.js b/ui/admin/app/routes/scopes/scope/users/user/accounts.js index bbb4667d4..8a66ccad9 100644 --- a/ui/admin/app/routes/scopes/scope/users/user/accounts.js +++ b/ui/admin/app/routes/scopes/scope/users/user/accounts.js @@ -4,17 +4,13 @@ */ import Route from '@ember/routing/route'; -import { action } from '@ember/object'; import { inject as service } from '@ember/service'; import { hash, all } from 'rsvp'; -import { loading } from 'ember-loading'; -import { notifySuccess, notifyError } from 'core/decorators/notify'; export default class ScopesScopeUsersUserAccountsRoute extends Route { // =services @service store; - @service intl; // =methods @@ -34,20 +30,4 @@ export default class ScopesScopeUsersUserAccountsRoute extends Route { ), }); } - - // =actions - - /** - * Remove an account from the current role and redirect to accounts index. - * @param {UserModel} user - * @param {AccountModel} account - */ - @action - @loading - @notifyError(({ message }) => message, { catch: true }) - @notifySuccess('notifications.remove-success') - async removeAccount(user, account) { - await user.removeAccount(account.id); - await this.refresh(); - } } diff --git a/ui/admin/app/routes/scopes/scope/users/user/add-accounts.js b/ui/admin/app/routes/scopes/scope/users/user/add-accounts.js index 56acd7d9c..25ff45c00 100644 --- a/ui/admin/app/routes/scopes/scope/users/user/add-accounts.js +++ b/ui/admin/app/routes/scopes/scope/users/user/add-accounts.js @@ -5,16 +5,11 @@ import Route from '@ember/routing/route'; import { inject as service } from '@ember/service'; -import { action } from '@ember/object'; -import { loading } from 'ember-loading'; -import { notifySuccess, notifyError } from 'core/decorators/notify'; export default class ScopesScopeUsersUserAddAccountsRoute extends Route { // =services @service store; - @service intl; - @service router; // =methods @@ -44,29 +39,4 @@ export default class ScopesScopeUsersUserAddAccountsRoute extends Route { const accounts = this.store.peekAll('account'); return { user, authMethods, accounts }; } - - // =actions - - /** - * Adds accounts to the user and saves, replaces with the accounts index - * route, and notifies the user of success or error. - * @param {UserModel} user - * @param {[string]} accountIDs - */ - @action - @loading - @notifyError(({ message }) => message, { catch: true }) - @notifySuccess('notifications.add-success') - async addAccounts(user, accountIDs) { - await user.addAccounts(accountIDs); - await this.router.replaceWith('scopes.scope.users.user.accounts'); - } - - /** - * Redirect to user accounts as if nothing ever happened. - */ - @action - cancel() { - this.router.replaceWith('scopes.scope.users.user.accounts'); - } } diff --git a/ui/admin/app/templates/scopes/scope/users/new.hbs b/ui/admin/app/templates/scopes/scope/users/new.hbs index ca7dbaca5..97232f753 100644 --- a/ui/admin/app/templates/scopes/scope/users/new.hbs +++ b/ui/admin/app/templates/scopes/scope/users/new.hbs @@ -26,8 +26,8 @@ diff --git a/ui/admin/app/templates/scopes/scope/users/user/accounts.hbs b/ui/admin/app/templates/scopes/scope/users/user/accounts.hbs index efa8f9164..f773afe14 100644 --- a/ui/admin/app/templates/scopes/scope/users/user/accounts.hbs +++ b/ui/admin/app/templates/scopes/scope/users/user/accounts.hbs @@ -24,7 +24,10 @@ - + @@ -79,7 +82,7 @@ @text={{t 'actions.remove'}} {{on 'click' - (route-action 'removeAccount' @model.user B.data) + (fn this.users.removeAccount @model.user B.data) }} /> diff --git a/ui/admin/app/templates/scopes/scope/users/user/add-accounts.hbs b/ui/admin/app/templates/scopes/scope/users/user/add-accounts.hbs index 4c6d35c5d..0b6ae827d 100644 --- a/ui/admin/app/templates/scopes/scope/users/user/add-accounts.hbs +++ b/ui/admin/app/templates/scopes/scope/users/user/add-accounts.hbs @@ -26,8 +26,8 @@ @model={{@model.user}} @authMethods={{@model.authMethods}} @accounts={{@model.accounts}} - @submit={{route-action 'addAccounts' @model.user}} - @cancel={{route-action 'cancel'}} + @submit={{fn this.addAccounts @model.user}} + @cancel={{this.cancel}} /> {{/if}} diff --git a/ui/admin/app/templates/scopes/scope/users/user/index.hbs b/ui/admin/app/templates/scopes/scope/users/user/index.hbs index 8616503f2..f32faf42a 100644 --- a/ui/admin/app/templates/scopes/scope/users/user/index.hbs +++ b/ui/admin/app/templates/scopes/scope/users/user/index.hbs @@ -18,14 +18,14 @@ - + diff --git a/ui/admin/tests/unit/controllers/scopes/scope/users/index-test.js b/ui/admin/tests/unit/controllers/scopes/scope/users/index-test.js index e0ae5db9d..9257038db 100644 --- a/ui/admin/tests/unit/controllers/scopes/scope/users/index-test.js +++ b/ui/admin/tests/unit/controllers/scopes/scope/users/index-test.js @@ -5,18 +5,57 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; -import { waitUntil } from '@ember/test-helpers'; +import { waitUntil, visit } from '@ember/test-helpers'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; +import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { authenticateSession } from 'ember-simple-auth/test-support'; module('Unit | Controller | scopes/scope/users/index', function (hooks) { setupTest(hooks); + setupMirage(hooks); + setupIndexedDb(hooks); + + let store; + let controller; + let getUserCount; + + const instances = { + scopes: { + global: null, + }, + user: null, + account: null, + }; + + const urls = { + globalScope: null, + users: null, + }; + + hooks.beforeEach(function () { + authenticateSession({}); + store = this.owner.lookup('service:store'); + controller = this.owner.lookup('controller:scopes/scope/users/index'); + + instances.scopes.global = this.server.create('scope', { id: 'global' }); + instances.user = this.server.create('user', { + scope: instances.scopes.global, + }); + instances.account = this.server.create('account', { + scope: instances.scopes.global, + }); + + urls.globalScope = `/scopes/global`; + urls.users = `${urls.globalScope}/users`; + + getUserCount = () => this.server.schema.users.all().models.length; + }); test('it exists', function (assert) { - let controller = this.owner.lookup('controller:scopes/scope/users/index'); assert.ok(controller); }); test('handleSearchInput action sets expected values correctly', async function (assert) { - let controller = this.owner.lookup('controller:scopes/scope/users/index'); const searchValue = 'test'; controller.handleSearchInput({ target: { value: searchValue } }); await waitUntil(() => controller.search === searchValue); @@ -24,4 +63,46 @@ module('Unit | Controller | scopes/scope/users/index', function (hooks) { assert.strictEqual(controller.page, 1); assert.strictEqual(controller.search, searchValue); }); + + test('cancel action rolls-back changes on the specified model', async function (assert) { + await visit(urls.users); + const user = await store.findRecord('user', instances.user.id); + user.name = 'test'; + + assert.strictEqual(user.name, 'test'); + + await controller.cancel(user); + + assert.notEqual(user.name, 'test'); + }); + + test('save action saves changes on the specified model', async function (assert) { + await visit(urls.users); + const user = await store.findRecord('user', instances.user.id); + user.name = 'test'; + + await controller.save(user); + + assert.strictEqual(user.name, 'test'); + }); + + test('delete action destroys specified model', async function (assert) { + const user = await store.findRecord('user', instances.user.id); + const userCount = getUserCount(); + + await controller.delete(user); + + assert.strictEqual(getUserCount(), userCount - 1); + }); + + test('removeAccount action removes specified account from model', async function (assert) { + const user = await store.findRecord('user', instances.user.id); + await user.addAccounts([instances.account.id]); + + assert.deepEqual(user.account_ids, [instances.account.id]); + + await controller.removeAccount(user, instances.account); + + assert.deepEqual(user.account_ids, []); + }); }); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/users/new-test.js b/ui/admin/tests/unit/controllers/scopes/scope/users/new-test.js new file mode 100644 index 000000000..de3dce931 --- /dev/null +++ b/ui/admin/tests/unit/controllers/scopes/scope/users/new-test.js @@ -0,0 +1,11 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Controller | scopes/scope/users/new', function (hooks) { + setupTest(hooks); + + test('it exists', function (assert) { + let controller = this.owner.lookup('controller:scopes/scope/users/new'); + assert.ok(controller); + }); +}); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/users/user/accounts-test.js b/ui/admin/tests/unit/controllers/scopes/scope/users/user/accounts-test.js new file mode 100644 index 000000000..7964dcf0d --- /dev/null +++ b/ui/admin/tests/unit/controllers/scopes/scope/users/user/accounts-test.js @@ -0,0 +1,16 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module( + 'Unit | Controller | scopes/scope/users/user/accounts', + function (hooks) { + setupTest(hooks); + + test('it exists', function (assert) { + let controller = this.owner.lookup( + 'controller:scopes/scope/users/user/accounts', + ); + assert.ok(controller); + }); + }, +); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/users/user/add-accounts-test.js b/ui/admin/tests/unit/controllers/scopes/scope/users/user/add-accounts-test.js new file mode 100644 index 000000000..08f77b258 --- /dev/null +++ b/ui/admin/tests/unit/controllers/scopes/scope/users/user/add-accounts-test.js @@ -0,0 +1,63 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { visit } from '@ember/test-helpers'; +import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; +import { authenticateSession } from 'ember-simple-auth/test-support'; + +module( + 'Unit | Controller | scopes/scope/users/user/add-accounts', + function (hooks) { + setupTest(hooks); + setupMirage(hooks); + + let store; + let controller; + + const instances = { + scopes: { + global: null, + }, + user: null, + account: null, + }; + + const urls = { + globalScope: null, + addAccounts: null, + }; + + hooks.beforeEach(function () { + authenticateSession({}); + store = this.owner.lookup('service:store'); + controller = this.owner.lookup( + 'controller:scopes/scope/users/user/add-accounts', + ); + + instances.scopes.global = this.server.create('scope', { id: 'global' }); + instances.user = this.server.create('user', { + scope: instances.scopes.global, + }); + instances.account = this.server.create('account', { + scope: instances.scopes.global, + }); + + urls.globalScope = `/scopes/global`; + urls.addAccounts = `${urls.globalScope}/users/add-accounts`; + }); + + test('it exists', function (assert) { + assert.ok(controller); + }); + + test('addAccount action adds specified account to model', async function (assert) { + await visit(urls.addAccounts); + const user = await store.findRecord('user', instances.user.id); + + assert.deepEqual(user.account_ids, []); + + await controller.addAccounts(user, [instances.account.id]); + + assert.deepEqual(user.account_ids, [instances.account.id]); + }); + }, +); diff --git a/ui/admin/tests/unit/controllers/scopes/scope/users/user/index-test.js b/ui/admin/tests/unit/controllers/scopes/scope/users/user/index-test.js new file mode 100644 index 000000000..edb371faf --- /dev/null +++ b/ui/admin/tests/unit/controllers/scopes/scope/users/user/index-test.js @@ -0,0 +1,13 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; + +module('Unit | Controller | scopes/scope/users/user/index', function (hooks) { + setupTest(hooks); + + test('it exists', function (assert) { + let controller = this.owner.lookup( + 'controller:scopes/scope/users/user/index', + ); + assert.ok(controller); + }); +});