From 0a9f283e7e8e336f3af602ad576c1b44be5cd58c Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 16 Apr 2020 22:12:37 +0200 Subject: [PATCH] REFACTOR: various refactoring applied to group pages (#9440) --- .../controllers/group-add-members.js | 55 +++++---- .../discourse/controllers/group-index.js | 109 +++++++++--------- .../controllers/group-manage-logs.js | 44 ++++--- .../discourse/controllers/group.js | 82 +++++++------ .../discourse/controllers/groups-new.js | 23 ++-- .../discourse/routes/group-index.js | 29 ++--- .../discourse/routes/group-members.js | 2 +- .../javascripts/discourse/routes/group.js | 2 +- .../javascripts/discourse/templates/group.hbs | 31 ++--- 9 files changed, 192 insertions(+), 185 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/group-add-members.js b/app/assets/javascripts/discourse/controllers/group-add-members.js index 9b1f8881139..0637ac79939 100644 --- a/app/assets/javascripts/discourse/controllers/group-add-members.js +++ b/app/assets/javascripts/discourse/controllers/group-add-members.js @@ -3,6 +3,7 @@ import { isEmpty } from "@ember/utils"; import Controller from "@ember/controller"; import { extractError } from "discourse/lib/ajax-error"; import ModalFunctionality from "discourse/mixins/modal-functionality"; +import { action } from "@ember/object"; export default Controller.extend(ModalFunctionality, { loading: false, @@ -13,36 +14,32 @@ export default Controller.extend(ModalFunctionality, { return loading || !usernames || !(usernames.length > 0); }, - actions: { - addMembers() { - this.set("loading", true); + @action + addMembers() { + this.set("loading", true); - const model = this.model; - const usernames = model.get("usernames"); - if (isEmpty(usernames)) { - return; - } - let promise; - - if (this.setAsOwner) { - promise = model.addOwners(usernames, true); - } else { - promise = model.addMembers(usernames, true); - } - - promise - .then(() => { - this.transitionToRoute("group.members", this.get("model.name"), { - queryParams: { filter: usernames } - }); - - model.set("usernames", null); - this.send("closeModal"); - }) - .catch(error => { - this.flash(extractError(error), "error"); - }) - .finally(() => this.set("loading", false)); + const usernames = this.model.usernames; + if (isEmpty(usernames)) { + return; } + let promise; + + if (this.setAsOwner) { + promise = this.model.addOwners(usernames, true); + } else { + promise = this.model.addMembers(usernames, true); + } + + promise + .then(() => { + this.transitionToRoute("group.members", this.get("model.name"), { + queryParams: { filter: usernames } + }); + + this.model.set("usernames", null); + this.send("closeModal"); + }) + .catch(error => this.flash(extractError(error), "error")) + .finally(() => this.set("loading", false)); } }); diff --git a/app/assets/javascripts/discourse/controllers/group-index.js b/app/assets/javascripts/discourse/controllers/group-index.js index b4168ba4c0d..6d661891441 100644 --- a/app/assets/javascripts/discourse/controllers/group-index.js +++ b/app/assets/javascripts/discourse/controllers/group-index.js @@ -1,8 +1,9 @@ import Controller, { inject as controller } from "@ember/controller"; -import { alias } from "@ember/object/computed"; +import { gt, readOnly } from "@ember/object/computed"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; import { popupAjaxError } from "discourse/lib/ajax-error"; import discourseDebounce from "discourse/lib/debounce"; +import { action } from "@ember/object"; export default Controller.extend({ application: controller(), @@ -15,7 +16,7 @@ export default Controller.extend({ filterInput: null, loading: false, - isOwner: alias("model.is_group_owner"), + isOwner: readOnly("model.is_group_owner"), showActions: false, @observes("filterInput") @@ -29,27 +30,22 @@ export default Controller.extend({ }, findMembers(refresh) { - if (this.loading) { + if (this.loading || !this.model) { return; } - const model = this.model; - if (!model) { - return; - } - - if (!refresh && model.members.length >= model.user_count) { + if (!refresh && this.model.members.length >= this.model.user_count) { this.set("application.showFooter", true); return; } this.set("loading", true); - model.findMembers(this.memberParams, refresh).finally(() => { - this.set( - "application.showFooter", - model.members.length >= model.user_count - ); - this.set("loading", false); + this.model.findMembers(this.memberParams, refresh).finally(() => { + this.setProperties({ + "application.showFooter": + this.model.members.length >= this.model.user_count, + loading: false + }); }); }, @@ -58,10 +54,7 @@ export default Controller.extend({ return { order, desc, filter }; }, - @discourseComputed("model.members.[]") - hasMembers(members) { - return members && members.length > 0; - }, + hasMembers: gt("model.members.length", 0), @discourseComputed("model") canManageGroup(model) { @@ -77,49 +70,53 @@ export default Controller.extend({ } }, - actions: { - loadMore() { - this.findMembers(); - }, + @action + loadMore() { + this.findMembers(); + }, - toggleActions() { - this.toggleProperty("showActions"); - }, + @action + toggleActions() { + this.toggleProperty("showActions"); + }, - actOnGroup(member, actionId) { - switch (actionId) { - case "removeMember": - this.send("removeMember", member); - break; - case "makeOwner": - this.send("makeOwner", member.username); - break; - case "removeOwner": - this.send("removeOwner", member); - break; - } - }, + @action + actOnGroup(member, actionId) { + switch (actionId) { + case "removeMember": + this.removeMember(member); + break; + case "makeOwner": + this.makeOwner(member.username); + break; + case "removeOwner": + this.removeOwner(member); + break; + } + }, - removeMember(user) { - this.model.removeMember(user, this.memberParams); - }, + @action + removeMember(user) { + this.model.removeMember(user, this.memberParams); + }, - makeOwner(username) { - this.model.addOwners(username); - }, + @action + makeOwner(username) { + this.model.addOwners(username); + }, - removeOwner(user) { - this.model.removeOwner(user); - }, + @action + removeOwner(user) { + this.model.removeOwner(user); + }, - addMembers() { - const usernames = this.usernames; - if (usernames && usernames.length > 0) { - this.model - .addMembers(usernames) - .then(() => this.set("usernames", [])) - .catch(popupAjaxError); - } + @action + addMembers() { + if (this.usernames && this.usernames.length > 0) { + this.model + .addMembers(this.usernames) + .then(() => this.set("usernames", [])) + .catch(popupAjaxError); } } }); diff --git a/app/assets/javascripts/discourse/controllers/group-manage-logs.js b/app/assets/javascripts/discourse/controllers/group-manage-logs.js index 80947cb9a61..ab3ff8ea819 100644 --- a/app/assets/javascripts/discourse/controllers/group-manage-logs.js +++ b/app/assets/javascripts/discourse/controllers/group-manage-logs.js @@ -1,5 +1,5 @@ import Controller, { inject as controller } from "@ember/controller"; -import EmberObject from "@ember/object"; +import EmberObject, { action } from "@ember/object"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; export default Controller.extend({ @@ -19,8 +19,8 @@ export default Controller.extend({ "filters.target_user", "filters.subject" ) - filterParams(action, acting_user, target_user, subject) { - return { action, acting_user, target_user, subject }; + filterParams(filtersAction, acting_user, target_user, subject) { + return { action: filtersAction, acting_user, target_user, subject }; }, @observes( @@ -54,28 +54,26 @@ export default Controller.extend({ }); }, - actions: { - loadMore() { - if (this.get("model.all_loaded")) return; + @action + loadMore() { + if (this.get("model.all_loaded")) return; - this.set("loading", true); + this.set("loading", true); - this.get("group.model") - .findLogs(this.offset + 1, this.filterParams) - .then(results => { - results.logs.forEach(result => - this.get("model.logs").addObject(result) - ); - this.incrementProperty("offset"); - this.set("model.all_loaded", results.all_loaded); - }) - .finally(() => { - this.set("loading", false); - }); - }, + this.get("group.model") + .findLogs(this.offset + 1, this.filterParams) + .then(results => { + results.logs.forEach(result => + this.get("model.logs").addObject(result) + ); + this.incrementProperty("offset"); + this.set("model.all_loaded", results.all_loaded); + }) + .finally(() => this.set("loading", false)); + }, - clearFilter(key) { - this.set(`filters.${key}`, ""); - } + @action + clearFilter(key) { + this.set(`filters.${key}`, ""); } }); diff --git a/app/assets/javascripts/discourse/controllers/group.js b/app/assets/javascripts/discourse/controllers/group.js index b66561f39ce..3ec4f40128c 100644 --- a/app/assets/javascripts/discourse/controllers/group.js +++ b/app/assets/javascripts/discourse/controllers/group.js @@ -1,15 +1,18 @@ -import EmberObject from "@ember/object"; +import EmberObject, { action } from "@ember/object"; import Controller, { inject as controller } from "@ember/controller"; import discourseComputed from "discourse-common/utils/decorators"; import { inject as service } from "@ember/service"; import { readOnly } from "@ember/object/computed"; +import deprecated from "discourse-common/lib/deprecated"; const Tab = EmberObject.extend({ init() { this._super(...arguments); - let name = this.name; - this.set("route", this.route || `group.` + name); - this.set("message", I18n.t(`groups.${this.i18nKey || name}`)); + + this.setProperties({ + route: this.route || `group.${this.name}`, + message: I18n.t(`groups.${this.i18nKey || this.name}`) + }); } }); @@ -19,7 +22,7 @@ export default Controller.extend({ showing: "members", destroying: null, router: service(), - currentPath: readOnly("router._router.currentPath"), + currentPath: readOnly("router.currentRouteName"), @discourseComputed( "showMessages", @@ -39,11 +42,10 @@ export default Controller.extend({ name: "members", route: "group.index", icon: "users", - i18nKey: "members.title" + i18nKey: "members.title", + count: userCount }); - membersTab.set("count", userCount); - const defaultTabs = [membersTab, Tab.create({ name: "activity" })]; if (canManageGroup && allowMembershipRequests) { @@ -127,37 +129,45 @@ export default Controller.extend({ ); }, - actions: { - messageGroup() { - this.send("createNewMessageViaParams", this.get("model.name")); - }, + @action + messageGroup() { + this.send("createNewMessageViaParams", this.get("model.name")); + }, - destroy() { - const group = this.model; - this.set("destroying", true); + @action + destroyGroup() { + this.set("destroying", true); - bootbox.confirm( - I18n.t("admin.groups.delete_confirm"), - I18n.t("no_value"), - I18n.t("yes_value"), - confirmed => { - if (confirmed) { - group - .destroy() - .then(() => { - this.transitionToRoute("groups.index"); - }) - .catch(error => { - // eslint-disable-next-line no-console - console.error(error); - bootbox.alert(I18n.t("admin.groups.delete_failed")); - }) - .finally(() => this.set("destroying", false)); - } else { - this.set("destroying", false); - } + bootbox.confirm( + I18n.t("admin.groups.delete_confirm"), + I18n.t("no_value"), + I18n.t("yes_value"), + confirmed => { + if (confirmed) { + this.model + .destroy() + .then(() => this.transitionToRoute("groups.index")) + .catch(error => { + // eslint-disable-next-line no-console + console.error(error); + bootbox.alert(I18n.t("admin.groups.delete_failed")); + }) + .finally(() => this.set("destroying", false)); + } else { + this.set("destroying", false); } - ); + } + ); + }, + + actions: { + destroy() { + deprecated("Use `destroyGroup` action instead of `destroy`.", { + since: "2.5.0", + dropFrom: "2.6.0" + }); + + this.destroyGroup(); } } }); diff --git a/app/assets/javascripts/discourse/controllers/groups-new.js b/app/assets/javascripts/discourse/controllers/groups-new.js index f93c08959da..05013cb7da3 100644 --- a/app/assets/javascripts/discourse/controllers/groups-new.js +++ b/app/assets/javascripts/discourse/controllers/groups-new.js @@ -1,21 +1,20 @@ +import { action } from "@ember/object"; import Controller from "@ember/controller"; import { popupAjaxError } from "discourse/lib/ajax-error"; export default Controller.extend({ saving: null, - actions: { - save() { - this.set("saving", true); - const group = this.model; + @action + save() { + this.set("saving", true); - group - .create() - .then(() => { - this.transitionToRoute("group.members", group.name); - }) - .catch(popupAjaxError) - .finally(() => this.set("saving", false)); - } + this.model + .create() + .then(() => { + this.transitionToRoute("group.members", this.model.name); + }) + .catch(popupAjaxError) + .finally(() => this.set("saving", false)); } }); diff --git a/app/assets/javascripts/discourse/routes/group-index.js b/app/assets/javascripts/discourse/routes/group-index.js index a3aed428165..a543c89613c 100644 --- a/app/assets/javascripts/discourse/routes/group-index.js +++ b/app/assets/javascripts/discourse/routes/group-index.js @@ -1,5 +1,6 @@ import DiscourseRoute from "discourse/routes/discourse"; import showModal from "discourse/lib/show-modal"; +import { action } from "@ember/object"; export default DiscourseRoute.extend({ titleToken() { @@ -12,28 +13,28 @@ export default DiscourseRoute.extend({ }, setupController(controller, model) { - this.controllerFor("group").set("showing", "members"); - controller.setProperties({ model, - filterInput: this._params.filter + filterInput: this._params.filter, + showing: "members" }); controller.findMembers(true); }, - actions: { - showAddMembersModal() { - showModal("group-add-members", { model: this.modelFor("group") }); - }, + @action + showAddMembersModal() { + showModal("group-add-members", { model: this.modelFor("group") }); + }, - showBulkAddModal() { - showModal("group-bulk-add", { model: this.modelFor("group") }); - }, + @action + showBulkAddModal() { + showModal("group-bulk-add", { model: this.modelFor("group") }); + }, - didTransition() { - this.controllerFor("group-index").set("filterInput", this._params.filter); - return true; - } + @action + didTransition() { + this.controllerFor("group-index").set("filterInput", this._params.filter); + return true; } }); diff --git a/app/assets/javascripts/discourse/routes/group-members.js b/app/assets/javascripts/discourse/routes/group-members.js index 872052de679..a3031c03bb7 100644 --- a/app/assets/javascripts/discourse/routes/group-members.js +++ b/app/assets/javascripts/discourse/routes/group-members.js @@ -1,7 +1,7 @@ import DiscourseRoute from "discourse/routes/discourse"; export default DiscourseRoute.extend({ - beforeModel: function() { + beforeModel() { this.transitionTo("group.index"); } }); diff --git a/app/assets/javascripts/discourse/routes/group.js b/app/assets/javascripts/discourse/routes/group.js index bd58f6d3a7f..8a60e8654ea 100644 --- a/app/assets/javascripts/discourse/routes/group.js +++ b/app/assets/javascripts/discourse/routes/group.js @@ -14,6 +14,6 @@ export default DiscourseRoute.extend({ }, setupController(controller, model) { - controller.setProperties({ model }); + controller.set("model", model); } }); diff --git a/app/assets/javascripts/discourse/templates/group.hbs b/app/assets/javascripts/discourse/templates/group.hbs index 0a53f7f1c04..ba6ed2b8cc9 100644 --- a/app/assets/javascripts/discourse/templates/group.hbs +++ b/app/assets/javascripts/discourse/templates/group.hbs @@ -9,7 +9,8 @@ flairURL=model.flair_url flairBgColor=model.flair_bg_color flairColor=model.flair_color - groupName=model.name}} + groupName=model.name + }} {{/if}} @@ -32,24 +33,28 @@
{{group-membership-button - class="inline" - model=model - showLogin=(route-action "showLogin")}} + class="inline" + model=model + showLogin=(route-action "showLogin") + }} {{#if displayGroupMessageButton}} {{d-button - action=(action "messageGroup") - class="btn-primary group-message-button inline" - icon="envelope" - label="groups.message"}} + action=(action "messageGroup") + class="btn-primary group-message-button inline" + icon="envelope" + label="groups.message" + }} {{/if}} {{#if currentUser.admin}} - {{d-button action=(action "destroy") - disabled=destroying - icon="trash-alt" - class="btn-danger" - label="admin.groups.delete"}} + {{d-button + action=(action "destroy") + disabled=destroying + icon="trash-alt" + class="btn-danger" + label="admin.groups.delete" + }} {{/if}}