FEATURE: Improve group settings and members management (#11878)

This pull requests contains a series of improvements to groups
settings and member management such as:

- Showing which users have set a group as primary
- Moving similar settings together under Effects
- Adding bulk select and actions to members page

app/assets/javascripts/discourse/app/components/group-member-dropdown.js

@@ -11,7 +11,56 @@ export default DropdownSelectBoxComponent.extend({
     showFullTitle: false,
   },
 
-  content: computed("member.owner", function () {
+  contentBulk() {
+    const items = [];
+
+    items.push({
+      id: "removeMembers",
+      name: I18n.t("groups.members.remove_members"),
+      description: I18n.t("groups.members.remove_members_description"),
+      icon: "user-times",
+    });
+
+    if (this.bulkSelection.some((m) => !m.owner)) {
+      items.push({
+        id: "makeOwners",
+        name: I18n.t("groups.members.make_owners"),
+        description: I18n.t("groups.members.make_owners_description"),
+        icon: "shield-alt",
+      });
+    }
+
+    if (this.bulkSelection.some((m) => m.owner)) {
+      items.push({
+        id: "removeOwners",
+        name: I18n.t("groups.members.remove_owners"),
+        description: I18n.t("groups.members.remove_owners_description"),
+        icon: "shield-alt",
+      });
+    }
+
+    if (this.bulkSelection.some((m) => !m.primary)) {
+      items.push({
+        id: "setPrimary",
+        name: I18n.t("groups.members.make_all_primary"),
+        description: I18n.t("groups.members.make_all_primary_description"),
+        icon: "id-card",
+      });
+    }
+
+    if (this.bulkSelection.some((m) => m.primary)) {
+      items.push({
+        id: "unsetPrimary",
+        name: I18n.t("groups.members.remove_all_primary"),
+        description: I18n.t("groups.members.remove_all_primary_description"),
+        icon: "id-card",
+      });
+    }
+
+    return items;
+  },
+
+  contentSingle() {
     const items = [
       {
         id: "removeMember",
@@ -45,6 +94,39 @@ export default DropdownSelectBoxComponent.extend({
       }
     }
 
+    if (this.currentUser.staff) {
+      if (this.member.primary) {
+        items.push({
+          id: "removePrimary",
+          name: I18n.t("groups.members.remove_primary"),
+          description: I18n.t("groups.members.remove_primary_description", {
+            username: this.get("member.username"),
+          }),
+          icon: "id-card",
+        });
+      } else {
+        items.push({
+          id: "makePrimary",
+          name: I18n.t("groups.members.make_primary"),
+          description: I18n.t("groups.members.make_primary_description", {
+            username: this.get("member.username"),
+          }),
+          icon: "id-card",
+        });
+      }
+    }
+
     return items;
-  }),
+  },
+
+  content: computed(
+    "bulkSelection.[]",
+    "member.owner",
+    "member.primary",
+    function () {
+      return this.bulkSelection !== undefined
+        ? this.contentBulk()
+        : this.contentSingle();
+    }
+  ),
 });

app/assets/javascripts/discourse/app/controllers/group-index.js

@@ -1,6 +1,7 @@
 import Controller, { inject as controller } from "@ember/controller";
 import discourseComputed, { observes } from "discourse-common/utils/decorators";
 import { action } from "@ember/object";
+import { ajax } from "discourse/lib/ajax";
 import discourseDebounce from "discourse-common/lib/debounce";
 import { gt } from "@ember/object/computed";
 import { popupAjaxError } from "discourse/lib/ajax-error";
@@ -16,8 +17,11 @@ export default Controller.extend({
   filterInput: null,
 
   loading: false,
+  isBulk: false,
   showActions: false,
 
+  bulkSelection: null,
+
   @observes("filterInput")
   _setFilter() {
     discourseDebounce(
@@ -51,6 +55,10 @@ export default Controller.extend({
           this.model.members.length >= this.model.user_count,
         loading: false,
       });
+
+      if (this.refresh) {
+        this.set("bulkSelection", []);
+      }
     });
   },
 
@@ -97,6 +105,68 @@ export default Controller.extend({
       case "removeOwner":
         this.removeOwner(member);
         break;
+      case "makePrimary":
+        member
+          .setPrimaryGroup(this.model.id)
+          .then(() => member.set("primary", true));
+        break;
+      case "removePrimary":
+        member.setPrimaryGroup(null).then(() => member.set("primary", false));
+        break;
+    }
+  },
+
+  @action
+  actOnSelection(selection, actionId) {
+    if (!selection || selection.length === 0) {
+      return;
+    }
+
+    switch (actionId) {
+      case "removeMembers":
+        return ajax(`/groups/${this.model.id}/members.json`, {
+          type: "DELETE",
+          data: { user_ids: selection.map((u) => u.id).join(",") },
+        }).then(() => {
+          this.model.findMembers(this.memberParams, true);
+          this.set("isBulk", false);
+        });
+
+      case "makeOwners":
+        return ajax(`/admin/groups/${this.model.id}/owners.json`, {
+          type: "PUT",
+          data: {
+            group: { usernames: selection.map((u) => u.username).join(",") },
+          },
+        }).then(() => {
+          selection.forEach((s) => s.set("owner", true));
+          this.set("isBulk", false);
+        });
+
+      case "removeOwners":
+        return ajax(`/admin/groups/${this.model.id}/owners.json`, {
+          type: "DELETE",
+          data: {
+            group: { usernames: selection.map((u) => u.username).join(",") },
+          },
+        }).then(() => {
+          selection.forEach((s) => s.set("owner", false));
+          this.set("isBulk", false);
+        });
+
+      case "setPrimary":
+      case "unsetPrimary":
+        const primary = actionId === "setPrimary";
+        return ajax(`/admin/groups/${this.model.id}/primary.json`, {
+          type: "PUT",
+          data: {
+            group: { usernames: selection.map((u) => u.username).join(",") },
+            primary,
+          },
+        }).then(() => {
+          selection.forEach((s) => s.set("primary", primary));
+          this.set("isBulk", false);
+        });
     }
   },
 
@@ -124,4 +194,33 @@ export default Controller.extend({
         .catch(popupAjaxError);
     }
   },
+
+  @action
+  toggleBulkSelect() {
+    this.setProperties({
+      isBulk: !this.isBulk,
+      bulkSelection: [],
+    });
+  },
+
+  @action
+  bulkSelectAll() {
+    $("input.bulk-select:not(:checked)").click();
+  },
+
+  @action
+  bulkClearAll() {
+    $("input.bulk-select:checked").click();
+  },
+
+  @action
+  selectMember(member, e) {
+    this.set("bulkSelection", this.bulkSelection || []);
+
+    if (e.target.checked) {
+      this.bulkSelection.pushObject(member);
+    } else {
+      this.bulkSelection.removeObject(member);
+    }
+  },
 });

app/assets/javascripts/discourse/app/models/group.js

@@ -56,6 +56,7 @@ const Group = RestModel.extend({
       members.pushObjects(
         result.members.map((member) => {
           member.owner = ownerIds.has(member.id);
+          member.primary = member.primary_group_name === this.name;
           return User.create(member);
         })
       );

app/assets/javascripts/discourse/app/models/user.js

@@ -960,6 +960,13 @@ const User = RestModel.extend({
     }
   },
 
+  setPrimaryGroup(primaryGroupId) {
+    return ajax(`/admin/users/${this.id}/primary_group`, {
+      type: "PUT",
+      data: { primary_group_id: primaryGroupId },
+    });
+  },
+
   enterDoNotDisturbFor(duration) {
     return ajax({
       url: "/do-not-disturb.json",

app/assets/javascripts/discourse/app/templates/components/groups-form-membership-fields.hbs

@@ -1,3 +1,45 @@
+<div class="control-group">
+  <label class="control-label">{{i18n "groups.manage.membership.access"}}</label>
+
+  <label>
+    {{input type="checkbox"
+        class="group-form-public-admission"
+        checked=model.public_admission
+        disabled=disablePublicSetting}}
+
+    {{i18n "groups.public_admission"}}
+  </label>
+
+  <label>
+    {{input type="checkbox"
+        class="group-form-public-exit"
+        checked=model.public_exit}}
+
+    {{i18n "groups.public_exit"}}
+  </label>
+
+  <label>
+    {{input type="checkbox"
+        class="group-form-allow-membership-requests"
+        checked=model.allow_membership_requests
+        disabled=disableMembershipRequestSetting}}
+
+    {{i18n "groups.allow_membership_requests"}}
+  </label>
+
+  {{#if model.allow_membership_requests}}
+    <div>
+      <label for="membership-request-template">
+        {{i18n "groups.membership_request_template"}}
+      </label>
+
+      {{expanding-text-area name="membership-request-template"
+          class="group-form-membership-request-template input-xxlarge"
+          value=model.membership_request_template}}
+    </div>
+  {{/if}}
+</div>
+
 {{#if model.can_admin_group}}
   <div class="control-group">
     <label class="control-label">{{i18n "admin.groups.manage.membership.automatic"}}</label>
@@ -41,48 +83,21 @@
 
       {{i18n "admin.groups.manage.membership.primary_group"}}
     </label>
+  </div>
 
+  <div class="control-group">
+    <label class="control-label" for="title">
+      {{i18n "admin.groups.default_title"}}
+    </label>
+
+    {{input value=model.title name="title" class="input-xxlarge"}}
+
+    <div class="control-instructions">
+      {{i18n "admin.groups.default_title_description"}}
+    </div>
+  </div>
+
+  <div class="control-group">
+    {{group-flair-inputs model=model}}
   </div>
 {{/if}}
-
-<div class="control-group">
-  <label class="control-label">{{i18n "groups.manage.membership.access"}}</label>
-
-  <label>
-    {{input type="checkbox"
-        class="group-form-public-admission"
-        checked=model.public_admission
-        disabled=disablePublicSetting}}
-
-    {{i18n "groups.public_admission"}}
-  </label>
-
-  <label>
-    {{input type="checkbox"
-        class="group-form-public-exit"
-        checked=model.public_exit}}
-
-    {{i18n "groups.public_exit"}}
-  </label>
-
-  <label>
-    {{input type="checkbox"
-        class="group-form-allow-membership-requests"
-        checked=model.allow_membership_requests
-        disabled=disableMembershipRequestSetting}}
-
-    {{i18n "groups.allow_membership_requests"}}
-  </label>
-
-  {{#if model.allow_membership_requests}}
-    <div>
-      <label for="membership-request-template">
-        {{i18n "groups.membership_request_template"}}
-      </label>
-
-      {{expanding-text-area name="membership-request-template"
-          class="group-form-membership-request-template input-xxlarge"
-          value=model.membership_request_template}}
-    </div>
-  {{/if}}
-</div>

app/assets/javascripts/discourse/app/templates/components/groups-form-profile-fields.hbs

@@ -19,20 +19,6 @@
         class="input-xxlarge group-form-full-name"
         value=model.full_name}}
   </div>
-
-  {{#if this.currentUser.can_create_group}}
-    <div class="control-group">
-      <label class="control-label" for="title">
-        {{i18n "admin.groups.default_title"}}
-      </label>
-
-      {{input value=model.title name="title" class="input-xxlarge"}}
-
-      <div class="control-instructions">
-        {{i18n "admin.groups.default_title_description"}}
-      </div>
-    </div>
-  {{/if}}
 {{/if}}
 
 <div class="control-group">
@@ -43,9 +29,5 @@
 {{#if canEdit}}
   {{yield}}
 
-  <div class="control-group">
-    {{group-flair-inputs model=model}}
-  </div>
-
   {{plugin-outlet name="group-edit" args=(hash group=model)}}
 {{/if}}

app/assets/javascripts/discourse/app/templates/group-index.hbs

@@ -23,26 +23,50 @@
     {{#load-more selector=".group-members tr" action=(action "loadMore")}}
       <table class="group-members">
         <thead>
-          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username"}}
+          <th class="bulk-select">
-          <th class="group-owner">{{i18n "groups.members.owner"}}</th>
+            {{flat-button class="bulk-select" icon="list" action=(action "toggleBulkSelect") title="topics.bulk.toggle"}}
+          </th>
+          {{#if isBulk}}
+            <th class="bulk-select-buttons">
+              {{d-button action=(action "bulkSelectAll") label="topics.bulk.select_all"}}
+              {{d-button action=(action "bulkClearAll") label="topics.bulk.clear_all"}}
+            </th>
+          {{/if}}
+          {{table-header-toggle order=order asc=asc field="username_lower" labelKey="username" class="username"}}
+          <th class="group-owner"></th>
           {{table-header-toggle order=order asc=asc field="added_at" labelKey="groups.member_added"}}
           {{table-header-toggle order=order asc=asc field="last_posted_at" labelKey="last_post"}}
           {{table-header-toggle order=order asc=asc field="last_seen_at" labelKey="last_seen"}}
-          <th></th>
+          <th>
+            {{#if isBulk}}
+              {{group-member-dropdown
+                bulkSelection=bulkSelection
+                canAdminGroup=model.can_admin_group
+                onChange=(action "actOnSelection" bulkSelection)
+              }}
+            {{/if}}
+          </th>
         </thead>
 
         <tbody>
           {{#each model.members as |m|}}
             <tr>
-              <td class="avatar">
+              {{#if isBulk}}
+                <td class="bulk-select">
+                  {{input type="checkbox" class="bulk-select" click=(action "selectMember" m)}}
+                </td>
+              {{/if}}
+
+              <td class="avatar" colspan="2">
                 {{user-info user=m skipName=skipName}}
               </td>
 
               <td class="group-owner">
                 {{#if m.owner}}
-                  <strong class="group-owner-label">
+                  {{d-icon "shield-alt"}} {{i18n "groups.members.owner"}}<br>
-                    {{d-icon "shield-alt"}}
+                {{/if}}
-                  </strong>
+                {{#if m.primary}}
+                  {{i18n "groups.members.primary"}}
                 {{/if}}
               </td>
               <td>

app/assets/javascripts/discourse/tests/acceptance/group-index-test.js

@@ -6,6 +6,7 @@ import {
 } from "discourse/tests/helpers/qunit-helpers";
 import { click, visit } from "@ember/test-helpers";
 import I18n from "I18n";
+import selectKit from "discourse/tests/helpers/select-kit-helper";
 import { test } from "qunit";
 
 acceptance("Group Members - Anonymous", function () {
@@ -34,6 +35,12 @@ acceptance("Group Members - Anonymous", function () {
 acceptance("Group Members", function (needs) {
   needs.user();
 
+  needs.pretender((server, helper) => {
+    server.put("/admin/groups/47/owners.json", () => {
+      return helper.response({ success: true });
+    });
+  });
+
   test("Viewing Members as a group owner", async function (assert) {
     updateCurrentUser({ moderator: false, admin: false });
 
@@ -61,4 +68,18 @@ acceptance("Group Members", function (needs) {
       "it should display the right filter placehodler"
     );
   });
+
+  test("Shows bulk actions", async function (assert) {
+    await visit("/g/discourse");
+
+    assert.ok(count("button.bulk-select") > 0);
+    await click("button.bulk-select");
+
+    await click("input.bulk-select:nth(0)");
+    await click("input.bulk-select:nth(1)");
+
+    const memberDropdown = selectKit(".group-member-dropdown:first");
+    await memberDropdown.expand();
+    await memberDropdown.selectRowByValue("makeOwners");
+  });
 });

app/assets/javascripts/discourse/tests/acceptance/group-manage-membership-test.js

@@ -50,6 +50,11 @@ acceptance("Managing Group Membership", function (needs) {
       "it should disable group allow_membership_request input"
     );
 
+    assert.ok(
+      queryAll(".group-flair-inputs").length === 1,
+      "it should display avatar flair inputs"
+    );
+
     await click(".group-form-public-admission");
     await click(".group-form-allow-membership-requests");
 

app/assets/javascripts/discourse/tests/acceptance/group-manage-profile-test.js

@@ -24,10 +24,6 @@ acceptance("Managing Group Profile", function (needs) {
   test("As an admin", async function (assert) {
     await visit("/g/discourse/manage/profile");
 
-    assert.ok(
-      queryAll(".group-flair-inputs").length === 1,
-      "it should display avatar flair inputs"
-    );
     assert.ok(
       queryAll(".group-form-bio").length === 1,
       "it should display group bio input"

app/assets/stylesheets/common/base/group.scss

@@ -107,39 +107,48 @@ table.group-members {
   width: 100%;
   table-layout: fixed;
 
-  th:first-child {
-    width: 30%;
-    text-align: left;
-  }
-
-  th:last-child {
-    width: 5%;
-  }
-
-  th.group-members-actions {
-    width: 5%;
-  }
-
   th {
     text-align: center;
+
+    &.bulk-select {
+      height: 30px;
+      width: 30px;
+    }
+
+    &.bulk-select-buttons {
+      text-align: left;
+      width: 20%;
+    }
+
+    &.username {
+      text-align: left;
+      width: 30%;
+    }
+
+    &.bulk-select-buttons + .username {
+      width: 10%;
+    }
+
+    &.group-members-actions {
+      width: 5%;
+    }
   }
 
-  tr {
+  td {
-    .user-info {
+    color: var(--primary-medium);
-      display: block;
+    padding: 0.8em 0;
-      .avatar-flair {
+    text-align: center;
-        color: var(--primary);
-      }
-    }
 
-    td:first-child {
+    &.avatar {
       text-align: left;
     }
+  }
 
-    td {
+  .user-info {
-      text-align: center;
+    display: block;
-      color: var(--primary-medium);
+
-      padding: 0.8em 0;
+    .avatar-flair {
+      color: var(--primary);
     }
   }
 }

app/controllers/admin/groups_controller.rb

@@ -125,15 +125,35 @@ class Admin::GroupsController < Admin::AdminController
     return can_not_modify_automatic if group.automatic
     guardian.ensure_can_edit_group!(group)
 
-    user = User.find(params[:user_id].to_i)
+    if params[:user_id].present?
-    group.group_users.where(user_id: user.id).update_all(owner: false)
+      users = [User.find_by(id: params[:user_id].to_i)]
-    GroupActionLogger.new(current_user, group).log_remove_user_as_group_owner(user)
+    elsif usernames = group_params[:usernames].presence
+      users = User.where(username: usernames.split(","))
+    else
+      raise Discourse::InvalidParameters.new(:user_id)
+    end
+
+    users.each do |user|
+      group.group_users.where(user_id: user.id).update_all(owner: false)
+      GroupActionLogger.new(current_user, group).log_remove_user_as_group_owner(user)
+    end
 
     Group.reset_counters(group.id, :group_users)
 
     render json: success_json
   end
 
+  def set_primary
+    group = Group.find_by(id: params.require(:id))
+    raise Discourse::NotFound unless group
+
+    users = User.where(username: group_params[:usernames].split(","))
+    users.each { |user| guardian.ensure_can_change_primary_group!(user) }
+    users.update_all(primary_group_id: params[:primary] == "true" ? group.id : nil)
+
+    render json: success_json
+  end
+
   def automatic_membership_count
     domains = Group.get_valid_email_domains(params.require(:automatic_membership_email_domains))
     group_id = params[:id]

config/locales/client.en.yml

@@ -785,7 +785,22 @@ en:
         make_owner_description: "Make <b>%{username}</b> an owner of this group"
         remove_owner: "Remove as Owner"
         remove_owner_description: "Remove <b>%{username}</b> as an owner of this group"
+        make_primary: "Make Primary"
+        make_primary_description: "Make this the primary group for <b>%{username}</b>"
+        remove_primary: "Remove as Primary"
+        remove_primary_description: "Remove this as the primary group for <b>%{username}</b>"
+        remove_members: "Remove Members"
+        remove_members_description: "Remove selected users from this group"
+        make_owners: "Make Owners"
+        make_owners_description: "Make selected users owners of this group"
+        remove_owners: "Remove Owners"
+        remove_owners_description: "Remove selected users as owners of this group"
+        make_all_primary: "Make All Primary"
+        make_all_primary_description: "Make this the primary group for all selected users"
+        remove_all_primary: "Remove as Primary"
+        remove_all_primary_description: "Remove this group as primary"
         owner: "Owner"
+        primary: "Primary"
         forbidden: "You're not allowed to view the members."
       topics: "Topics"
       posts: "Posts"

config/routes.rb

@@ -95,6 +95,7 @@ Discourse::Application.routes.draw do
         member do
           put "owners" => "groups#add_owners"
           delete "owners" => "groups#remove_owner"
+          put "primary" => "groups#set_primary"
         end
       end
       resources :groups, except: [:create], constraints: AdminConstraint.new do

spec/requests/admin/groups_controller_spec.rb

@@ -157,6 +157,9 @@ RSpec.describe Admin::GroupsController do
   end
 
   describe '#remove_owner' do
+    let(:user2) { Fabricate(:user) }
+    let(:user3) { Fabricate(:user) }
+
     it 'should work' do
       group.add_owner(user)
 
@@ -168,6 +171,20 @@ RSpec.describe Admin::GroupsController do
       expect(group.group_users.where(owner: true)).to eq([])
     end
 
+    it 'should work with multiple users' do
+      group.add_owner(user)
+      group.add_owner(user3)
+
+      delete "/admin/groups/#{group.id}/owners.json", params: {
+        group: {
+          usernames: "#{user.username},#{user2.username},#{user3.username}"
+        }
+      }
+
+      expect(response.status).to eq(200)
+      expect(group.group_users.where(owner: true)).to eq([])
+    end
+
     it 'returns not-found error when there is no group' do
       group.destroy!
 
@@ -190,6 +207,36 @@ RSpec.describe Admin::GroupsController do
     end
   end
 
+  describe "#set_primary" do
+    let(:user2) { Fabricate(:user) }
+    let(:user3) { Fabricate(:user) }
+
+    it 'sets with multiple users' do
+      user2.update!(primary_group_id: group.id)
+
+      put "/admin/groups/#{group.id}/primary.json", params: {
+        group: { usernames: "#{user.username},#{user2.username},#{user3.username}" },
+        primary: "true"
+      }
+
+      expect(response.status).to eq(200)
+      expect(User.where(primary_group_id: group.id).size).to eq(3)
+    end
+
+    it 'unsets with multiple users' do
+      user.update!(primary_group_id: group.id)
+      user3.update!(primary_group_id: group.id)
+
+      put "/admin/groups/#{group.id}/primary.json", params: {
+        group: { usernames: "#{user.username},#{user2.username},#{user3.username}" },
+        primary: "false"
+      }
+
+      expect(response.status).to eq(200)
+      expect(User.where(primary_group_id: group.id).size).to eq(0)
+    end
+  end
+
   describe "#bulk_perform" do
     fab!(:group) do
       Fabricate(:group,