REFACTOR: various refactoring applied to group pages (#9440)

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,21 +14,20 @@ export default Controller.extend(ModalFunctionality, {
     return loading || !usernames || !(usernames.length > 0);
   },
 
-  actions: {
+  @action
   addMembers() {
     this.set("loading", true);
 
-      const model = this.model;
-      const usernames = model.get("usernames");
+    const usernames = this.model.usernames;
     if (isEmpty(usernames)) {
       return;
     }
     let promise;
 
     if (this.setAsOwner) {
-        promise = model.addOwners(usernames, true);
+      promise = this.model.addOwners(usernames, true);
     } else {
-        promise = model.addMembers(usernames, true);
+      promise = this.model.addMembers(usernames, true);
     }
 
     promise
@@ -36,13 +36,10 @@ export default Controller.extend(ModalFunctionality, {
           queryParams: { filter: usernames }
         });
 
-          model.set("usernames", null);
+        this.model.set("usernames", null);
         this.send("closeModal");
       })
-        .catch(error => {
-          this.flash(extractError(error), "error");
-        })
+      .catch(error => this.flash(extractError(error), "error"))
       .finally(() => this.set("loading", false));
   }
-  }
 });

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: {
+  @action
   loadMore() {
     this.findMembers();
   },
 
+  @action
   toggleActions() {
     this.toggleProperty("showActions");
   },
 
+  @action
   actOnGroup(member, actionId) {
     switch (actionId) {
       case "removeMember":
-          this.send("removeMember", member);
+        this.removeMember(member);
         break;
       case "makeOwner":
-          this.send("makeOwner", member.username);
+        this.makeOwner(member.username);
         break;
       case "removeOwner":
-          this.send("removeOwner", member);
+        this.removeOwner(member);
         break;
     }
   },
 
+  @action
   removeMember(user) {
     this.model.removeMember(user, this.memberParams);
   },
 
+  @action
   makeOwner(username) {
     this.model.addOwners(username);
   },
 
+  @action
   removeOwner(user) {
     this.model.removeOwner(user);
   },
 
+  @action
   addMembers() {
-      const usernames = this.usernames;
-      if (usernames && usernames.length > 0) {
+    if (this.usernames && this.usernames.length > 0) {
       this.model
-          .addMembers(usernames)
+        .addMembers(this.usernames)
         .then(() => this.set("usernames", []))
         .catch(popupAjaxError);
     }
   }
-  }
 });

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,7 +54,7 @@ export default Controller.extend({
     });
   },
 
-  actions: {
+  @action
   loadMore() {
     if (this.get("model.all_loaded")) return;
 
@@ -69,13 +69,11 @@ export default Controller.extend({
         this.incrementProperty("offset");
         this.set("model.all_loaded", results.all_loaded);
       })
-        .finally(() => {
-          this.set("loading", false);
-        });
+      .finally(() => this.set("loading", false));
   },
 
+  @action
   clearFilter(key) {
     this.set(`filters.${key}`, "");
   }
-  }
 });

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,13 +129,13 @@ export default Controller.extend({
     );
   },
 
-  actions: {
+  @action
   messageGroup() {
     this.send("createNewMessageViaParams", this.get("model.name"));
   },
 
-    destroy() {
-      const group = this.model;
+  @action
+  destroyGroup() {
     this.set("destroying", true);
 
     bootbox.confirm(
@@ -142,11 +144,9 @@ export default Controller.extend({
       I18n.t("yes_value"),
       confirmed => {
         if (confirmed) {
-            group
+          this.model
             .destroy()
-              .then(() => {
-                this.transitionToRoute("groups.index");
-              })
+            .then(() => this.transitionToRoute("groups.index"))
             .catch(error => {
               // eslint-disable-next-line no-console
               console.error(error);
@@ -158,6 +158,16 @@ export default Controller.extend({
         }
       }
     );
+  },
+
+  actions: {
+    destroy() {
+      deprecated("Use `destroyGroup` action instead of `destroy`.", {
+        since: "2.5.0",
+        dropFrom: "2.6.0"
+      });
+
+      this.destroyGroup();
     }
   }
 });

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: {
+  @action
   save() {
     this.set("saving", true);
-      const group = this.model;
 
-      group
+    this.model
       .create()
       .then(() => {
-          this.transitionToRoute("group.members", group.name);
+        this.transitionToRoute("group.members", this.model.name);
       })
       .catch(popupAjaxError)
       .finally(() => this.set("saving", false));
   }
-  }
 });

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: {
+  @action
   showAddMembersModal() {
     showModal("group-add-members", { model: this.modelFor("group") });
   },
 
+  @action
   showBulkAddModal() {
     showModal("group-bulk-add", { model: this.modelFor("group") });
   },
 
+  @action
   didTransition() {
     this.controllerFor("group-index").set("filterInput", this._params.filter);
     return true;
   }
-  }
 });

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");
   }
 });

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);
   }
 });

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
+          }}
         </span>
       {{/if}}
 
@@ -34,22 +35,26 @@
       {{group-membership-button
         class="inline"
         model=model
-          showLogin=(route-action  "showLogin")}}
+        showLogin=(route-action "showLogin")
+      }}
 
       {{#if displayGroupMessageButton}}
         {{d-button
           action=(action "messageGroup")
           class="btn-primary group-message-button inline"
           icon="envelope"
-            label="groups.message"}}
+          label="groups.message"
+        }}
       {{/if}}
 
       {{#if currentUser.admin}}
-        {{d-button action=(action "destroy")
+        {{d-button
+          action=(action "destroy")
           disabled=destroying
           icon="trash-alt"
           class="btn-danger"
-            label="admin.groups.delete"}}
+          label="admin.groups.delete"
+        }}
       {{/if}}
     </div>
   </div>