UX: Allow admins to set users as owners while adding users.

https://meta.discourse.org/t/adding-owners-members-ux-is-inconsistent-and-misleading/58084/9

app/assets/javascripts/discourse/controllers/group-membership.js.es6

@@ -4,6 +4,7 @@ import { extractError } from 'discourse/lib/ajax-error';
 
 export default Ember.Controller.extend(ModalFunctionality, {
   loading: false,
+  setAsOwner: false,
 
   @computed('model.usernames')
   disableAddButton(usernames) {
@@ -12,19 +13,27 @@ export default Ember.Controller.extend(ModalFunctionality, {
 
   actions: {
     addMembers() {
-      if (Em.isEmpty(this.get("model.usernames"))) { return; }
+      const model = this.get('model');
+      const usernames = model.get('usernames');
+      if (Em.isEmpty(usernames)) { return; }
+      let promise;
 
-      this.get("model").addMembers(this.get("model.usernames"))
+      if (this.get('setAsOwner')) {
-        .then(() => {
+        promise = model.addOwners(usernames, true);
-          this.transitionToRoute(
+      } else {
-            "group.members",
+        promise = model.addMembers(usernames, true);
-            this.get('model.name'),
+      }
-            { queryParams: { filter: this.get('model.usernames') } }
+
-          );
+      promise.then(() => {
-          this.set("model.usernames", null);
+        this.transitionToRoute(
-          this.send('closeModal');
+          "group.members",
-        })
+          this.get('model.name'),
-        .catch(error => this.flash(extractError(error), 'error'));
+          { queryParams: { filter: usernames } }
+        );
+        model.set("usernames", null);
+        this.send('closeModal');
+      })
+      .catch(error => this.flash(extractError(error), 'error'));
     },
   },
 });

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

@@ -74,24 +74,36 @@ const Group = RestModel.extend({
     });
   },
 
-  addMembers(usernames) {
+  addMembers(usernames, filter) {
     return ajax('/groups/' + this.get('id') + '/members.json', {
       type: "PUT",
       data: { usernames: usernames }
     }).then(response => {
-      this.findMembers({ filter: response.usernames.join(',') });
+      if (filter) {
+        this._filterMembers(response);
+      } else {
+        this.findMembers();
+      }
     });
   },
 
-  addOwners(usernames) {
+  addOwners(usernames, filter) {
     return ajax(`/admin/groups/${this.get('id')}/owners.json`, {
       type: "PUT",
       data: { group: { usernames: usernames } }
-    }).then(() => {
+    }).then(response => {
-      this.findMembers();
+      if (filter) {
+        this._filterMembers(response);
+      } else {
+        this.findMembers();
+      }
     });
   },
 
+  _filterMembers(response) {
+    return this.findMembers({ filter: response.usernames.join(",") });
+  },
+
   @computed("display_name", "name")
   displayName(groupDisplayName, name) {
     return groupDisplayName || name;

app/assets/javascripts/discourse/templates/modal/group-membership.hbs

@@ -1,7 +1,19 @@
 {{#d-modal-body class='group-membership' title="groups.add_members.title"}}
-  {{user-selector usernames=model.usernames
+  <div class="control-group">
-      placeholderKey="groups.selector_placeholder"
+    <label>{{i18n "groups.add_members.usernames"}}</label>
-      id="group-membership-user-selector"}}
+
+    {{user-selector
+        usernames=model.usernames
+        placeholderKey="groups.selector_placeholder"
+        id="group-membership-user-selector"}}
+  </div>
+
+  {{#if this.currentUser.admin}}
+    <div class="control-group group-membership-make-owner">
+      <label class="inline">{{i18n "groups.add_members.as_owner"}}</label>
+      {{input type="checkbox" class="inline" checked=setAsOwner}}
+    </div>
+  {{/if}}
 {{/d-modal-body}}
 
 <div class="modal-footer">

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

@@ -533,3 +533,7 @@ select {
     }
   }
 }
+
+.inline {
+  display: inline;
+}

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

@@ -213,4 +213,19 @@ table.group-members {
   .ac-wrap {
     width: 100% !important;
   }
+
+  label {
+    font-weight: bold;
+  }
+
+  .group-membership-make-owner {
+    label {
+      display: inline;
+      vertical-align: middle;
+    }
+
+    input[type='checkbox'] {
+      vertical-align: middle;
+    }
+  }
 }

app/controllers/admin/groups_controller.rb

@@ -166,7 +166,7 @@ class Admin::GroupsController < Admin::AdminController
 
     group.restore_user_count!
 
-    render json: success_json
+    render json: success_json.merge!(usernames: users.pluck(:username))
   end
 
   def remove_owner

config/locales/client.en.yml

@@ -467,6 +467,8 @@ en:
       add_members:
         title: "Add Members"
         description: "Manage the membership of this group"
+        usernames: "Usernames"
+        as_owner: "Set user(s) as owner(s) of this group"
       members:
         title: "Members"
         filter_placeholder_admin: "username or email"

spec/requests/admin/groups_controller_spec.rb

@@ -40,7 +40,11 @@ RSpec.describe Admin::GroupsController do
         }
       }
 
-      expect(response).to be_success
+      expect(response.status).to eq(200)
+
+      response_body = JSON.parse(response.body)
+
+      expect(response_body["usernames"]).to contain_exactly(user.username, admin.username)
 
       expect(group.group_users.where(owner: true).map(&:user))
         .to contain_exactly(user, admin)

test/javascripts/acceptance/group-index-test.js.es6

@@ -50,4 +50,23 @@ QUnit.test("Viewing Members as an admin user", assert => {
       'it should display the right filter placehodler'
     );
   });
+
+  selectKit('.group-navigation-dropdown').expand().selectRowByValue('manageMembership');
+
+  andThen(() => {
+    assert.ok(
+      count('.group-membership') === 1,
+      'it should display the right modal'
+    );
+
+    assert.ok(
+      count('#group-membership-user-selector') === 1,
+      'it should display the user selector'
+    );
+
+    assert.ok(
+      count(".group-membership-make-owner input[type='checkbox']") === 1,
+      'it should display the input to set users as owners'
+    );
+  });
 });