- {{else}}
-
- {{/if}}
-{{/d-modal-body}}
-
-
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index aa5ddd5d59f..1eabd40e785 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -99,6 +99,10 @@ class Admin::GroupsController < Admin::AdminController
end
group.group_users.where(user_id: user.id).update_all(owner: true)
group_action_logger.log_make_user_group_owner(user)
+
+ if group_params[:notify_users] == "true" || group_params[:notify_users] == true
+ group.notify_added_to_group(user, owner: true)
+ end
end
group.restore_user_count!
@@ -176,7 +180,8 @@ class Admin::GroupsController < Admin::AdminController
:membership_request_template,
:owner_usernames,
:usernames,
- :publish_read_state
+ :publish_read_state,
+ :notify_users
]
custom_fields = DiscoursePluginRegistry.editable_group_custom_fields
permitted << { custom_fields: custom_fields } unless custom_fields.blank?
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index b15dc18a061..3e4ecdb1fb9 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -298,8 +298,7 @@ class GroupsController < ApplicationController
def add_members
group = Group.find(params[:id])
group.public_admission ? ensure_logged_in : guardian.ensure_can_edit!(group)
-
- users = users_from_params
+ users = users_from_params.to_a
if group.public_admission
if !guardian.can_log_group_changes?(group) && current_user != users.first
@@ -311,17 +310,33 @@ class GroupsController < ApplicationController
end
end
- if (usernames = group.users.where(id: users.pluck(:id)).pluck(:username)).present?
+ emails = []
+ if params[:emails]
+ params[:emails].split(",").each do |email|
+ existing_user = User.find_by_email(email)
+ existing_user.present? ? users.push(existing_user) : emails.push(email)
+ end
+ end
+
+ if users.empty? && emails.empty?
+ raise Discourse::InvalidParameters.new(
+ 'usernames or emails must be present'
+ )
+ end
+
+ if (usernames = group.users.where(id: users.map(&:id)).pluck(:username)).present?
render_json_error(I18n.t(
"groups.errors.member_already_exist",
username: usernames.sort.join(", "),
count: usernames.size
))
else
- users.each do |user|
+ uniq_users = users.uniq
+ uniq_users.each do |user|
begin
group.add(user)
GroupActionLogger.new(current_user, group).log_add_user_to_group(user)
+ group.notify_added_to_group(user) if params[:notify_users]&.to_s == "true"
rescue ActiveRecord::RecordNotUnique
# Under concurrency, we might attempt to insert two records quickly and hit a DB
# constraint. In this case we can safely ignore the error and act as if the user
@@ -329,8 +344,13 @@ class GroupsController < ApplicationController
end
end
+ emails.each do |email|
+ Invite.invite_by_email(email, current_user, nil, [group.id])
+ end
+
render json: success_json.merge!(
- usernames: users.map(&:username)
+ usernames: uniq_users.map(&:username),
+ emails: emails
)
end
end
@@ -401,6 +421,9 @@ class GroupsController < ApplicationController
params[:user_emails] = params[:user_email] if params[:user_email].present?
users = users_from_params
+ raise Discourse::InvalidParameters.new(
+ 'user_ids or usernames or user_emails must be present'
+ ) if users.empty?
if group.public_exit
if !guardian.can_log_group_changes?(group) && current_user != users.first
@@ -605,9 +628,7 @@ class GroupsController < ApplicationController
users = User.with_email(params[:user_emails].split(","))
raise Discourse::InvalidParameters.new(:user_emails) if users.blank?
else
- raise Discourse::InvalidParameters.new(
- 'user_ids or usernames or user_emails must be present'
- )
+ users = []
end
users
end
diff --git a/app/models/group.rb b/app/models/group.rb
index ded3acbe37f..ce3dd234ffb 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -800,6 +800,16 @@ class Group < ActiveRecord::Base
end
end
+ def notify_added_to_group(user, owner: false)
+ SystemMessage.create_from_system_user(
+ user,
+ :user_added_to_group,
+ group_name: self.full_name.presence || self.name,
+ group_path: "/g/#{self.name}",
+ membership_level: owner ? "an owner" : "a member"
+ )
+ end
+
protected
def name_format_validator
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0fdccece691..4ec15414a52 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -626,9 +626,11 @@ en:
member_added: "Added"
member_requested: "Requested at"
add_members:
- title: "Add Members"
- description: "Manage the membership of this group"
- usernames: "Usernames"
+ title: "Add members to %{group_name}"
+ description: "You can also paste in a comma separated list."
+ usernames: "Enter usernames or email addresses"
+ input_placeholder: "Usernames or emails"
+ notify_users: "Notify users"
requests:
title: "Requests"
reason: "Reason"
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 9bf1ce3b970..129d66c64d6 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2912,6 +2912,11 @@ en:
``` text
%{logs}
```
+ user_added_to_group:
+ title: "Added to Group"
+ subject_template: "You have been added as %{membership_level} of the %{group_name} group"
+ text_body_template: |
+ You have been added as a %{membership_level} of the [%{group_name}](%{base_url}%{group_path}) group.
csv_export_succeeded:
title: "CSV Export Succeeded"
diff --git a/spec/requests/admin/groups_controller_spec.rb b/spec/requests/admin/groups_controller_spec.rb
index c8cedd06bac..d51f3e74408 100644
--- a/spec/requests/admin/groups_controller_spec.rb
+++ b/spec/requests/admin/groups_controller_spec.rb
@@ -122,6 +122,32 @@ RSpec.describe Admin::GroupsController do
expect(response.status).to eq(422)
expect(response.parsed_body["errors"]).to eq(["You cannot modify an automatic group"])
end
+
+ it 'does not notify users when the param is not present' do
+ put "/admin/groups/#{group.id}/owners.json", params: {
+ group: {
+ usernames: user.username
+ }
+ }
+ expect(response.status).to eq(200)
+
+ topic = Topic.find_by(title: "You have been added as an owner of the #{group.name} group", archetype: "private_message")
+ expect(topic.nil?).to eq(true)
+ end
+
+ it 'notifies users when the param is present' do
+ put "/admin/groups/#{group.id}/owners.json", params: {
+ group: {
+ usernames: user.username,
+ notify_users: true
+ }
+ }
+ expect(response.status).to eq(200)
+
+ topic = Topic.find_by(title: "You have been added as an owner of the #{group.name} group", archetype: "private_message")
+ expect(topic.nil?).to eq(false)
+ expect(topic.topic_users.map(&:user_id)).to include(-1, user.id)
+ end
end
describe '#remove_owner' do
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index ba268e505ef..0129bef8592 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -1053,6 +1053,28 @@ describe GroupsController do
expect(response.status).to eq(403)
end
+ it "does not notify users when the param is not present" do
+ user2 = Fabricate(:user)
+
+ expect {
+ put "/groups/#{group.id}/members.json", params: { usernames: user2.username }
+ }.to change { Topic.where(archetype: "private_message").count }.by(0)
+
+ expect(response.status).to eq(200)
+ end
+
+ it "notifies users when the param is present" do
+ user2 = Fabricate(:user)
+
+ expect {
+ put "/groups/#{group.id}/members.json", params: { usernames: user2.username, notify_users: true }
+ }.to change { Topic.where(archetype: "private_message").count }.by(1)
+
+ expect(response.status).to eq(200)
+
+ expect(Topic.last.topic_users.map(&:user_id)).to include(Discourse::SYSTEM_USER_ID, user2.id)
+ end
+
context "is able to add several members to a group" do
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user, username: "UsEr2") }
@@ -1132,6 +1154,53 @@ describe GroupsController do
end
end
+ it "return a 400 if no user or emails are present" do
+ [
+ { usernames: "nouserwiththisusername", emails: "" },
+ { usernames: "", emails: "" }
+ ].each do |params|
+ put "/groups/#{group.id}/members.json", params: params
+ expect(response.status).to eq(400)
+ body = response.parsed_body
+
+ expect(body["error_type"]).to eq("invalid_parameters")
+ end
+ end
+
+ it "will send invites to each email with group_id set" do
+ emails = ["something@gmail.com", "anotherone@yahoo.com"]
+ put "/groups/#{group.id}/members.json", params: { emails: emails.join(",") }
+
+ expect(response.status).to eq(200)
+ body = response.parsed_body
+
+ expect(body["emails"]).to eq(emails)
+
+ emails.each do |email|
+ invite = Invite.find_by(email: email)
+ expect(invite.groups).to eq([group])
+ end
+ end
+
+ it "will find users by email, and invite the correct user" do
+ new_user = Fabricate(:user)
+ expect(new_user.group_ids.include?(group.id)).to eq(false)
+
+ put "/groups/#{group.id}/members.json", params: { emails: new_user.email }
+
+ expect(new_user.reload.group_ids.include?(group.id)).to eq(true)
+ end
+
+ it "will invite the user if their username and email are both invited" do
+ new_user = Fabricate(:user)
+ put "/groups/#{group.id}/members.json", params: { usernames: new_user.username, emails: new_user.email }
+
+ expect(response.status).to eq(200)
+ body = response.parsed_body
+
+ expect(new_user.reload.group_ids.include?(group.id)).to eq(true)
+ end
+
context 'public group' do
fab!(:other_user) { Fabricate(:user) }