diff --git a/app/assets/javascripts/discourse/components/group-navigation-dropdown.js.es6 b/app/assets/javascripts/discourse/components/group-navigation-dropdown.js.es6
new file mode 100644
index 0000000000..263fc426e4
--- /dev/null
+++ b/app/assets/javascripts/discourse/components/group-navigation-dropdown.js.es6
@@ -0,0 +1,25 @@
+import DropdownSelectBox from "select-kit/components/dropdown-select-box";
+
+export default DropdownSelectBox.extend({
+ classNames: ["group-navigation-dropdown", "pull-right"],
+ nameProperty: "label",
+ headerIcon: ["bars"],
+ showFullTitle: false,
+
+ computeContent() {
+ const content = [];
+
+ content.push({
+ id: "manageMembership",
+ icon: "user-plus",
+ label: I18n.t("groups.add_members.title"),
+ description: I18n.t("groups.add_members.description"),
+ });
+
+ return content;
+ },
+
+ mutateValue(value) {
+ this.get(value)(this.get('model'));
+ }
+});
diff --git a/app/assets/javascripts/discourse/controllers/group-index.js.es6 b/app/assets/javascripts/discourse/controllers/group-index.js.es6
index 83c6b9f3e8..fffc78e0ac 100644
--- a/app/assets/javascripts/discourse/controllers/group-index.js.es6
+++ b/app/assets/javascripts/discourse/controllers/group-index.js.es6
@@ -26,14 +26,16 @@ export default Ember.Controller.extend({
const model = this.get('model');
if (model) {
- model.findMembers({
- order: this.get('order'),
- desc: this.get('desc'),
- filter: this.get('filter'),
- }).finally(() => this.set('loading', false));
+ model.findMembers(this.get('memberParams'))
+ .finally(() => this.set('loading', false));
}
},
+ @computed('order', 'desc', 'filter')
+ memberParams(order, desc, filter) {
+ return { order, desc, filter };
+ },
+
@computed('model.members')
hasMembers(members) {
return members && members.length > 0;
@@ -59,7 +61,7 @@ export default Ember.Controller.extend({
},
removeMember(user) {
- this.get('model').removeMember(user);
+ this.get('model').removeMember(user, this.get('memberParams'));
},
makeOwner(username) {
diff --git a/app/assets/javascripts/discourse/controllers/group-membership.js.es6 b/app/assets/javascripts/discourse/controllers/group-membership.js.es6
new file mode 100644
index 0000000000..76dafb878a
--- /dev/null
+++ b/app/assets/javascripts/discourse/controllers/group-membership.js.es6
@@ -0,0 +1,30 @@
+import ModalFunctionality from 'discourse/mixins/modal-functionality';
+import computed from 'ember-addons/ember-computed-decorators';
+import { extractError } from 'discourse/lib/ajax-error';
+
+export default Ember.Controller.extend(ModalFunctionality, {
+ loading: false,
+
+ @computed('model.usernames')
+ disableAddButton(usernames) {
+ return !usernames || !(usernames.length > 0);
+ },
+
+ actions: {
+ addMembers() {
+ if (Em.isEmpty(this.get("model.usernames"))) { return; }
+
+ this.get("model").addMembers(this.get("model.usernames"))
+ .then(() => {
+ this.transitionToRoute(
+ "group.members",
+ this.get('model.name'),
+ { queryParams: { filter: this.get('model.usernames') } }
+ );
+ this.set("model.usernames", null);
+ this.send('closeModal');
+ })
+ .catch(error => this.flash(extractError(error), 'error'));
+ },
+ },
+});
diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6
index c0df0d6513..46faf5ae8b 100644
--- a/app/assets/javascripts/discourse/controllers/group.js.es6
+++ b/app/assets/javascripts/discourse/controllers/group.js.es6
@@ -84,6 +84,11 @@ export default Ember.Controller.extend({
return this.currentUser && messageable;
},
+ @computed('model')
+ canManageGroup(model) {
+ return this.currentUser && this.currentUser.canManageGroup(model);
+ },
+
actions: {
messageGroup() {
this.send('createNewMessageViaParams', this.get('model.name'));
diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6
index 34b5e75231..14844e58ec 100644
--- a/app/assets/javascripts/discourse/models/group.js.es6
+++ b/app/assets/javascripts/discourse/models/group.js.es6
@@ -65,24 +65,21 @@ const Group = RestModel.extend({
});
},
- removeMember(member) {
- var self = this;
+ removeMember(member, params) {
return ajax('/groups/' + this.get('id') + '/members.json', {
type: "DELETE",
data: { user_id: member.get("id") }
- }).then(function() {
- // reload member list
- self.findMembers();
+ }).then(() => {
+ this.findMembers(params);
});
},
addMembers(usernames) {
- var self = this;
return ajax('/groups/' + this.get('id') + '/members.json', {
type: "PUT",
data: { usernames: usernames }
- }).then(function() {
- self.findMembers();
+ }).then(response => {
+ this.findMembers({ filter: response.usernames.join(',') });
});
},
diff --git a/app/assets/javascripts/discourse/routes/group-index.js.es6 b/app/assets/javascripts/discourse/routes/group-index.js.es6
index 4d37501f30..815308f20a 100644
--- a/app/assets/javascripts/discourse/routes/group-index.js.es6
+++ b/app/assets/javascripts/discourse/routes/group-index.js.es6
@@ -17,5 +17,12 @@ export default Discourse.Route.extend({
});
controller.refreshMembers();
+ },
+
+ actions: {
+ didTransition() {
+ this.controllerFor("group-index").set("filterInput", this._params.filter);
+ return true;
+ }
}
});
diff --git a/app/assets/javascripts/discourse/routes/group.js.es6 b/app/assets/javascripts/discourse/routes/group.js.es6
index 3df20f794b..6bb9610aed 100644
--- a/app/assets/javascripts/discourse/routes/group.js.es6
+++ b/app/assets/javascripts/discourse/routes/group.js.es6
@@ -1,4 +1,5 @@
import Group from 'discourse/models/group';
+import showModal from 'discourse/lib/show-modal';
export default Discourse.Route.extend({
@@ -16,5 +17,12 @@ export default Discourse.Route.extend({
setupController(controller, model) {
controller.setProperties({ model, counts: this.get('counts') });
+ },
+
+ actions: {
+ showGroupMembershipModal(model) {
+ showModal('group-membership', { model });
+ this.controllerFor('modal').set('modalClass', 'group-membership-modal');
+ },
}
});
diff --git a/app/assets/javascripts/discourse/templates/group-edit.hbs b/app/assets/javascripts/discourse/templates/group-edit.hbs
index b149a4ad7f..7e56a0ead2 100644
--- a/app/assets/javascripts/discourse/templates/group-edit.hbs
+++ b/app/assets/javascripts/discourse/templates/group-edit.hbs
@@ -10,10 +10,6 @@
{{d-editor value=model.bio_raw class="group-edit-bio"}}
-
- {{group-members-input model=model}}
-
-
{{group-flair-inputs model=model}}
diff --git a/app/assets/javascripts/discourse/templates/group.hbs b/app/assets/javascripts/discourse/templates/group.hbs
index dfb900b91e..6fc009b830 100644
--- a/app/assets/javascripts/discourse/templates/group.hbs
+++ b/app/assets/javascripts/discourse/templates/group.hbs
@@ -42,6 +42,12 @@
{{group-navigation group=model currentPath=application.currentPath tabs=tabs}}
+ {{#if canManageGroup}}
+ {{group-navigation-dropdown
+ model=model
+ manageMembership=(route-action "showGroupMembershipModal")}}
+ {{/if}}
+
{{#if displayGroupMessageButton}}
{{d-button
action="messageGroup"
diff --git a/app/assets/javascripts/discourse/templates/modal/group-membership.hbs b/app/assets/javascripts/discourse/templates/modal/group-membership.hbs
new file mode 100644
index 0000000000..7c5b07bfc6
--- /dev/null
+++ b/app/assets/javascripts/discourse/templates/modal/group-membership.hbs
@@ -0,0 +1,13 @@
+{{#d-modal-body class='group-membership' title="groups.add_members.title"}}
+ {{user-selector usernames=model.usernames
+ placeholderKey="groups.selector_placeholder"
+ id="group-membership-user-selector"}}
+{{/d-modal-body}}
+
+
diff --git a/app/assets/stylesheets/common/base/group.scss b/app/assets/stylesheets/common/base/group.scss
index 059680df3e..6d42830f74 100644
--- a/app/assets/stylesheets/common/base/group.scss
+++ b/app/assets/stylesheets/common/base/group.scss
@@ -208,3 +208,9 @@ table.group-members {
}
}
}
+
+.group-membership {
+ .ac-wrap {
+ width: 100% !important;
+ }
+}
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 70d219afc0..b41f170edf 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -188,6 +188,16 @@ class GroupsController < ApplicationController
users = group.users.human_users
total = users.count
+ if (filter = params[:filter]).present?
+ filter = filter.split(',') if filter.include?(',')
+
+ if current_user&.admin
+ users = users.filter_by_username_or_email(filter)
+ else
+ users = users.filter_by_username(filter)
+ end
+ end
+
members = users
.order('NOT group_users.owner')
.order(order)
@@ -200,16 +210,6 @@ class GroupsController < ApplicationController
.order(username_lower: dir)
.where('group_users.owner')
- if (filter = params[:filter]).present?
- if current_user&.admin
- owners = owners.filter_by_username_or_email(filter)
- members = members.filter_by_username_or_email(filter)
- else
- owners = owners.filter_by_username(filter)
- members = members.filter_by_username(filter)
- end
- end
-
render json: {
members: serialize_data(members, GroupUserSerializer),
owners: serialize_data(owners, GroupUserSerializer),
@@ -250,19 +250,21 @@ class GroupsController < ApplicationController
end
end
- users.each do |user|
- if !group.users.include?(user)
+ if (usernames = group.users.where(id: users.pluck(:id)).pluck(:username)).present?
+ render_json_error(I18n.t(
+ "groups.errors.member_already_exist",
+ username: usernames.join(", "),
+ count: usernames.size
+ ))
+ else
+ users.each do |user|
group.add(user)
GroupActionLogger.new(current_user, group).log_add_user_to_group(user)
- else
- return render_json_error I18n.t('groups.errors.member_already_exist', username: user.username)
end
- end
- if group.save
- render json: success_json
- else
- render_json_error(group)
+ render json: success_json.merge!(
+ usernames: users.map(&:username)
+ )
end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index c89e5dd69c..a821e0c572 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -160,7 +160,11 @@ class User < ActiveRecord::Base
scope :activated, -> { where(active: true) }
scope :filter_by_username, ->(filter) do
- where('username_lower ILIKE ?', "%#{filter}%")
+ if filter.is_a?(Array)
+ where('username_lower ~* ?', "(#{filter.join('|')})")
+ else
+ where('username_lower ILIKE ?', "%#{filter}%")
+ end
end
scope :filter_by_username_or_email, ->(filter) do
@@ -171,11 +175,19 @@ class User < ActiveRecord::Base
end
end
- joins(:primary_email)
- .where(
+ users = joins(:primary_email)
+
+ if filter.is_a?(Array)
+ users.where(
+ 'username_lower ~* :filter OR lower(user_emails.email) SIMILAR TO :filter',
+ filter: "(#{filter.join('|')})"
+ )
+ else
+ users.where(
'username_lower ILIKE :filter OR lower(user_emails.email) ILIKE :filter',
filter: "%#{filter}%"
)
+ end
end
module NewTopicDuration
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index af7318e25e..e58c141552 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -464,6 +464,9 @@ en:
one: "Group"
other: "Groups"
activity: "Activity"
+ add_members:
+ title: "Add Members"
+ description: "Manage the membership of this group"
members:
title: "Members"
filter_placeholder_admin: "username or email"
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 1e8ae34d90..761d2591c7 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -296,7 +296,9 @@ en:
bulk_add: "%{users_added} users have been added to the group."
errors:
can_not_modify_automatic: "You cannot modify an automatic group"
- member_already_exist: "'%{username}' is already a member of this group."
+ member_already_exist:
+ one: "'%{username}' is already a member of this group."
+ other: "The following users are already members of this group: %{username}"
invalid_domain: "'%{domain}' is not a valid domain."
invalid_incoming_email: "'%{email}' is not a valid email address."
email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'."
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 17268fa7a1..2f0f117312 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1651,16 +1651,29 @@ describe User do
end
end
+ def filter_by(method)
+ username = 'someuniqueusername'
+ user.update!(username: username)
+
+ username2 = 'awesomeusername'
+ user2 = Fabricate(:user, username: username2)
+
+ expect(User.public_send(method, username))
+ .to eq([user])
+
+ expect(User.public_send(method, 'UNiQuE'))
+ .to eq([user])
+
+ expect(User.public_send(method, [username, username2]))
+ .to contain_exactly(user, user2)
+
+ expect(User.public_send(method, ['UNiQuE', 'sOME']))
+ .to contain_exactly(user, user2)
+ end
+
describe '#filter_by_username' do
it 'should be able to filter by username' do
- username = 'someuniqueusername'
- user.update!(username: username)
-
- expect(User.filter_by_username(username))
- .to eq([user])
-
- expect(User.filter_by_username('UNiQuE'))
- .to eq([user])
+ filter_by(:filter_by_username)
end
end
@@ -1677,14 +1690,7 @@ describe User do
end
it 'should be able to filter by username' do
- username = 'someuniqueusername'
- user.update!(username: username)
-
- expect(User.filter_by_username_or_email(username))
- .to eq([user])
-
- expect(User.filter_by_username_or_email('UNiQuE'))
- .to eq([user])
+ filter_by(:filter_by_username_or_email)
end
end
end
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index 494d149989..f859a8058d 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -427,12 +427,16 @@ describe GroupsController do
email = 'uniquetest@discourse.org'
user1.update!(email: email)
- [email.upcase, 'QUEtes'].each do |filter|
+ {
+ email.upcase => [user1.id],
+ 'QUEtes' => [user1.id],
+ "#{user1.email},#{user2.email}" => [user1.id, user2.id]
+ }.each do |filter, ids|
get "/groups/#{group.name}/members.json", params: { filter: filter }
expect(response.status).to eq(200)
members = JSON.parse(response.body)["members"]
- expect(members.map { |m| m["id"] }).to contain_exactly(user1.id)
+ expect(members.map { |m| m["id"] }).to contain_exactly(*ids)
end
end
@@ -519,7 +523,7 @@ describe GroupsController do
sign_in(admin)
end
- context 'add_members' do
+ context '#add_members' do
it "can make incremental adds" do
user2 = Fabricate(:user)
@@ -573,12 +577,36 @@ describe GroupsController do
expect(response).to be_success
end
+
+ it 'fails when multiple member already exists' do
+ user3 = Fabricate(:user)
+ [user2, user3].each { |user| group.add(user) }
+
+ expect do
+ put "/groups/#{group.id}/members.json",
+ params: { user_emails: [user1.email, user2.email, user3.email].join(",") }
+ end.to change { group.users.count }.by(0)
+
+ expect(response.status).to eq(422)
+
+ expect(JSON.parse(response.body)["errors"]).to include(I18n.t(
+ "groups.errors.member_already_exist",
+ username: "#{user2.username}, #{user3.username}",
+ count: 2
+ ))
+ end
end
it "returns 422 if member already exists" do
put "/groups/#{group.id}/members.json", params: { usernames: user.username }
expect(response.status).to eq(422)
+
+ expect(JSON.parse(response.body)["errors"]).to include(I18n.t(
+ "groups.errors.member_already_exist",
+ username: user.username,
+ count: 1
+ ))
end
it "returns 404 if member is not found" do
diff --git a/test/javascripts/acceptance/group-index-test.js.es6 b/test/javascripts/acceptance/group-index-test.js.es6
index caa0a3b8d9..701fb8af59 100644
--- a/test/javascripts/acceptance/group-index-test.js.es6
+++ b/test/javascripts/acceptance/group-index-test.js.es6
@@ -9,6 +9,11 @@ QUnit.test("Viewing Members as anon user", assert => {
assert.ok(count('.avatar-flair .fa-adjust') === 1, "it displays the group's avatar flair");
assert.ok(count('.group-members tr') > 0, "it lists group members");
+ assert.ok(
+ count('.group-navigation-dropdown') === 0,
+ 'it should not display the group navigation dropdown menu'
+ );
+
assert.ok(
count('.group-member-dropdown') === 0,
'it does not allow anon user to manage group members'
@@ -29,6 +34,11 @@ QUnit.test("Viewing Members as an admin user", assert => {
visit("/groups/discourse");
andThen(() => {
+ assert.ok(
+ count('.group-navigation-dropdown') === 1,
+ 'it should display the group navigation dropdown menu'
+ );
+
assert.ok(
count('.group-member-dropdown') > 0,
'it allows admin user to manage group members'