FEATURE: Invite emails to groups from add member modal (#10308)

This commit is contained in:
Mark VanLandingham 2020-08-04 10:02:01 -05:00 committed by GitHub
parent 413fa49032
commit b76731d722
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 246 additions and 135 deletions

View File

@ -4,42 +4,101 @@ import Controller from "@ember/controller";
import { extractError } from "discourse/lib/ajax-error";
import ModalFunctionality from "discourse/mixins/modal-functionality";
import { action } from "@ember/object";
import { emailValid } from "discourse/lib/utilities";
import I18n from "I18n";
export default Controller.extend(ModalFunctionality, {
loading: false,
setAsOwner: false,
notifyUsers: false,
usernamesAndEmails: null,
usernames: null,
emails: null,
@discourseComputed("model.usernames", "loading")
disableAddButton(usernames, loading) {
return loading || !usernames || !(usernames.length > 0);
onShow() {
this.setProperties({
usernamesAndEmails: "",
usernames: [],
emails: [],
setAsOwner: false,
notifyUsers: false
});
},
@discourseComputed("usernamesAndEmails", "loading")
disableAddButton(usernamesAndEmails, loading) {
return loading || !usernamesAndEmails || !(usernamesAndEmails.length > 0);
},
@discourseComputed("usernamesAndEmails")
emailsPresent() {
this._splitEmailsAndUsernames();
return this.emails.length;
},
@discourseComputed("usernamesAndEmails")
notifyUsersDisabled() {
this._splitEmailsAndUsernames();
return this.usernames.length === 0 && this.emails.length > 0;
},
@discourseComputed("model.name", "model.full_name")
title(name, fullName) {
return I18n.t("groups.add_members.title", { group_name: fullName || name });
},
@action
addMembers() {
this.set("loading", true);
const usernames = this.model.usernames;
if (isEmpty(usernames)) {
if (this.emailsPresent) {
this.set("setAsOwner", false);
}
if (this.notifyUsersDisabled) {
this.set("notifyUsers", false);
}
if (isEmpty(this.usernamesAndEmails)) {
return;
}
let promise;
if (this.setAsOwner) {
promise = this.model.addOwners(usernames, true);
} else {
promise = this.model.addMembers(usernames, true);
}
const promise = this.setAsOwner
? this.model.addOwners(this.usernames, true, this.notifyUsers)
: this.model.addMembers(
this.usernames,
true,
this.notifyUsers,
this.emails
);
promise
.then(() => {
let queryParams = {};
if (this.usernames) {
queryParams.filter = this.usernames;
}
this.transitionToRoute("group.members", this.get("model.name"), {
queryParams: { filter: usernames }
queryParams
});
this.model.set("usernames", null);
this.send("closeModal");
})
.catch(error => this.flash(extractError(error), "error"))
.finally(() => this.set("loading", false));
},
_splitEmailsAndUsernames() {
let emails = [];
let usernames = [];
this.usernamesAndEmails.split(",").forEach(u => {
emailValid(u) ? emails.push(u) : usernames.push(u);
});
this.set("emails", emails.join(","));
this.set("usernames", usernames.join(","));
}
});

View File

@ -1,51 +0,0 @@
import discourseComputed from "discourse-common/utils/decorators";
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 { ajax } from "discourse/lib/ajax";
export default Controller.extend(ModalFunctionality, {
loading: false,
@discourseComputed("input", "loading", "result")
disableAddButton(input, loading, result) {
return loading || isEmpty(input) || input.length <= 0 || result;
},
actions: {
cancel() {
this.set("result", null);
},
add() {
this.setProperties({
loading: true,
result: null
});
const users = this.input
.split("\n")
.uniq()
.reject(x => x.length === 0);
ajax("/admin/groups/bulk", {
data: { users, group_id: this.get("model.id") },
type: "PUT"
})
.then(result => {
this.set("result", result);
if (result.users_not_added) {
this.set("result.invalidUsers", result.users_not_added.join(", "));
}
})
.catch(error => {
this.flash(extractError(error), "error");
})
.finally(() => {
this.set("loading", false);
});
}
}
});

View File

@ -113,10 +113,10 @@ const Group = RestModel.extend({
}).then(() => this.findMembers(params, true));
},
addMembers(usernames, filter) {
addMembers(usernames, filter, notifyUsers, emails = []) {
return ajax(`/groups/${this.id}/members.json`, {
type: "PUT",
data: { usernames }
data: { usernames, emails, notify_users: notifyUsers }
}).then(response => {
if (filter) {
this._filterMembers(response);
@ -126,10 +126,10 @@ const Group = RestModel.extend({
});
},
addOwners(usernames, filter) {
addOwners(usernames, filter, notifyUsers) {
return ajax(`/admin/groups/${this.id}/owners.json`, {
type: "PUT",
data: { group: { usernames } }
data: { group: { usernames, notify_users: notifyUsers } }
}).then(response => {
if (filter) {
this._filterMembers(response);

View File

@ -28,11 +28,6 @@ export default DiscourseRoute.extend({
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);

View File

@ -13,14 +13,8 @@
{{#if canManageGroup}}
{{d-button icon="plus"
action=(route-action "showAddMembersModal")
label="groups.add_members.title"
label="groups.manage.add_members"
class="group-members-add"}}
{{#if currentUser.admin}}
{{d-button icon="plus"
action=(route-action "showBulkAddModal")
label="admin.groups.bulk_add.title"
class="group-bulk-add"}}
{{/if}}
{{/if}}
</div>
</div>

View File

@ -16,7 +16,7 @@
{{else}}
{{d-button
icon="plus"
label="groups.add_members.title"
label="groups.manage.add_members"
class="group-members-add"
action=(route-action "showAddMembersModal")
}}

View File

@ -1,14 +1,18 @@
{{#d-modal-body title="groups.add_members.title"}}
{{#d-modal-body rawTitle=title}}
<form class="form-vertical group-add-members">
<div class="control-group">
<label class="control-label">
{{i18n "groups.add_members.usernames"}}
</label>
<p class="description">
{{i18n "groups.add_members.description"}}
</p>
{{user-selector
class="input-xxlarge"
usernames=model.usernames
placeholderKey="groups.selector_placeholder"
usernames=usernamesAndEmails
allowEmails=true
placeholderKey="groups.add_members.input_placeholder"
id="group-add-members-user-selector"}}
</div>
@ -18,11 +22,21 @@
{{input type="checkbox"
class="inline"
checked=setAsOwner
disabled=bulkAdd}}
disabled=emailsPresent}}
{{i18n "admin.groups.add_members.as_owner"}}
</label>
</div>
{{/if}}
<div class="control-group group-add-members-notify-users">
<label>
{{input type="checkbox"
class="inline"
checked=notifyUsers
disabled=notifyUsersDisabled}}
{{i18n "groups.add_members.notify_users"}}
</label>
</div>
</form>
{{/d-modal-body}}

View File

@ -1,38 +0,0 @@
{{#d-modal-body title="admin.groups.bulk_add.title"}}
{{#if result}}
{{#if result.message}}
<div class="alert alert-success">
{{result.message}}
</div>
{{/if}}
{{#if result.invalidUsers}}
<div class="alert alert-error">
{{i18n "admin.groups.bulk_add.complete_users_not_added"}}
{{result.invalidUsers}}
</div>
{{/if}}
<div class="">
<a href {{action "cancel"}}>{{i18n "cancel"}}</a>
</div>
{{else}}
<form class="form-vertical group-bulk-add">
<div class="control-group">
<label class="control-label">
{{i18n "admin.groups.bulk_add.paste"}}
</label>
{{textarea value=input}}
</div>
</form>
{{/if}}
{{/d-modal-body}}
<div class="modal-footer">
{{d-button action=(action "add")
class="add btn-primary"
icon="plus"
disabled=disableAddButton
label="groups.add"}}
</div>

View File

@ -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?

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -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"

View File

@ -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

View File

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