FEATURE: reason to reject user signup (#11700)

Feature for `Must Approve Users` setup. When a user is rejected, a staff member can optionally set a reason for audit purposes. In addition, feedback email can be sent to the user.

Meta: https://meta.discourse.org/t/account-rejection-email/103112/8
This commit is contained in:
Krzysztof Kotlarek 2021-01-15 09:43:26 +11:00 committed by GitHub
parent add125aacf
commit 06b7c44593
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 178 additions and 6 deletions

View File

@ -110,6 +110,10 @@ export default Component.extend({
`/review/${reviewable.id}/perform/${action.id}?version=${version}`, `/review/${reviewable.id}/perform/${action.id}?version=${version}`,
{ {
type: "PUT", type: "PUT",
data: {
send_email: reviewable.sendEmail,
reject_reason: reviewable.rejectReason,
},
} }
) )
.then((result) => { .then((result) => {
@ -222,12 +226,21 @@ export default Component.extend({
} }
let msg = action.get("confirm_message"); let msg = action.get("confirm_message");
let requireRejectReason = action.get("require_reject_reason");
if (msg) { if (msg) {
bootbox.confirm(msg, (answer) => { bootbox.confirm(msg, (answer) => {
if (answer) { if (answer) {
return this._performConfirmed(action); return this._performConfirmed(action);
} }
}); });
} else if (requireRejectReason) {
showModal("reject-reason-reviewable", {
title: "review.reject_reason.title",
model: this.reviewable,
}).setProperties({
performConfirmed: this._performConfirmed.bind(this),
action,
});
} else { } else {
return this._performConfirmed(action); return this._performConfirmed(action);
} }

View File

@ -0,0 +1,22 @@
import Controller from "@ember/controller";
import ModalFunctionality from "discourse/mixins/modal-functionality";
import { action } from "@ember/object";
export default Controller.extend(ModalFunctionality, {
rejectReason: null,
sendEmail: false,
onShow() {
this.setProperties({ rejectReason: null, sendEmail: false });
},
@action
perform() {
this.model.setProperties({
rejectReason: this.rejectReason,
sendEmail: this.sendEmail,
});
this.send("closeModal");
this.performConfirmed(this.action);
},
});

View File

@ -34,6 +34,10 @@
</div> </div>
{{/if}} {{/if}}
{{reviewable-field classes="reviewable-user-details reject-reason"
name=(i18n "review.user.reject_reason")
value=reviewable.reject_reason}}
{{#each userFields as |f|}} {{#each userFields as |f|}}
{{reviewable-field classes="reviewable-user-details user-field" {{reviewable-field classes="reviewable-user-details user-field"
name=f.name name=f.name

View File

@ -0,0 +1,19 @@
{{#d-modal-body class="explain-reviewable"}}
{{textarea value=rejectReason}}
<div class="control-group">
<label>
{{input type="checkbox"
class="inline"
checked=sendEmail
}}
{{i18n "review.reject_reason.send_email"}}
</label>
</div>
{{/d-modal-body}}
<div class="modal-footer">
{{d-button action=(route-action "closeModal") label="close"}}
<div class="pull-right">
{{d-button icon="trash-alt" class="btn-danger" action=(action "perform")}}
</div>
</div>

View File

@ -1,5 +1,6 @@
import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers"; import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers";
import { click, fillIn, visit } from "@ember/test-helpers"; import { click, fillIn, visit } from "@ember/test-helpers";
import I18n from "I18n";
import selectKit from "discourse/tests/helpers/select-kit-helper"; import selectKit from "discourse/tests/helpers/select-kit-helper";
import { test } from "qunit"; import { test } from "qunit";
@ -35,6 +36,33 @@ acceptance("Review", function (needs) {
); );
}); });
test("Reject user", async function (assert) {
await visit("/review");
await click(
`${user} .reviewable-actions button[data-name="Delete User..."]`
);
await click(`${user} li[data-value="reject_user_delete"]`);
assert.ok(
queryAll(".reject-reason-reviewable-modal:visible .title")
.html()
.includes(I18n.t("review.reject_reason.title")),
"it opens reject reason modal when user is rejected"
);
await click(".modal-footer button[aria-label='Close']");
await click(
`${user} .reviewable-actions button[data-name="Delete User..."]`
);
await click(`${user} li[data-value="reject_user_block"]`);
assert.ok(
queryAll(".reject-reason-reviewable-modal:visible .title")
.html()
.includes(I18n.t("review.reject_reason.title")),
"it opens reject reason modal when user is rejected and blocked"
);
});
test("Settings", async function (assert) { test("Settings", async function (assert) {
await visit("/review/settings"); await visit("/review/settings");

View File

@ -24,7 +24,7 @@ export default function (helpers) {
created_at: "2019-01-14T19:49:53.571Z", created_at: "2019-01-14T19:49:53.571Z",
username: "newbie", username: "newbie",
email: "newbie@example.com", email: "newbie@example.com",
bundled_action_ids: ["approve", "reject"], bundled_action_ids: ["approve", "reject", "reject_user"],
}, },
{ {
id: 4321, id: 4321,
@ -58,6 +58,12 @@ export default function (helpers) {
id: "reject", id: "reject",
action_ids: ["reject"], action_ids: ["reject"],
}, },
{
id: "reject_user",
icon: "user-times",
label: "Delete User...",
action_ids: ["reject_user_delete", "reject_user_block"],
},
], ],
actions: [ actions: [
{ {
@ -70,6 +76,23 @@ export default function (helpers) {
label: "Reject", label: "Reject",
icon: "far-thumbs-down", icon: "far-thumbs-down",
}, },
{
id: "reject_user_delete",
icon: "user-times",
button_class: null,
label: "Delete User",
description: "The user will be deleted from the forum.",
require_reject_reason: true,
},
{
id: "reject_user_block",
icon: "ban",
button_class: null,
label: "Delete and Block User",
description:
"The user will be deleted, and we'll block their IP and email address.",
require_reject_reason: true,
},
], ],
reviewable_scores: [{ id: 1 }, { id: 2 }], reviewable_scores: [{ id: 1 }, { id: 2 }],
users: [{ id: 1, username: "eviltrout" }], users: [{ id: 1, username: "eviltrout" }],

View File

@ -189,6 +189,8 @@ class ReviewablesController < ApplicationController
return render_json_error(error) return render_json_error(error)
end end
args.merge!(reject_reason: params[:reject_reason], send_email: params[:send_email] == "true") if reviewable.type == 'ReviewableUser'
result = reviewable.perform(current_user, params[:action_id].to_sym, args) result = reviewable.perform(current_user, params[:action_id].to_sym, args)
rescue Reviewable::InvalidAction => e rescue Reviewable::InvalidAction => e
# Consider InvalidAction an InvalidAccess # Consider InvalidAction an InvalidAccess

View File

@ -193,6 +193,8 @@ module Jobs
email_args[:user_history] = UserHistory.where(id: args[:user_history_id]).first email_args[:user_history] = UserHistory.where(id: args[:user_history_id]).first
end end
email_args[:reject_reason] = args[:reject_reason]
message = EmailLog.unique_email_per_post(post, user) do message = EmailLog.unique_email_per_post(post, user) do
UserNotifications.public_send(type, user, email_args) UserNotifications.public_send(type, user, email_args)
end end

View File

@ -37,6 +37,14 @@ class UserNotifications < ActionMailer::Base
new_user_tips: tips) new_user_tips: tips)
end end
def signup_after_reject(user, opts = {})
locale = user_locale(user)
build_email(user.email,
template: 'user_notifications.signup_after_reject',
locale: locale,
reject_reason: opts[:reject_reason])
end
def suspicious_login(user, opts = {}) def suspicious_login(user, opts = {})
ipinfo = DiscourseIpInfo.get(opts[:client_ip]) ipinfo = DiscourseIpInfo.get(opts[:client_ip])
location = ipinfo[:location] location = ipinfo[:location]

View File

@ -704,6 +704,8 @@ end
# latest_score :datetime # latest_score :datetime
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# force_review :boolean default(FALSE), not null
# reject_reason :text
# #
# Indexes # Indexes
# #

View File

@ -27,11 +27,13 @@ class ReviewableUser < Reviewable
actions.add(:reject_user_delete, bundle: reject) do |a| actions.add(:reject_user_delete, bundle: reject) do |a|
a.icon = 'user-times' a.icon = 'user-times'
a.label = "reviewables.actions.reject_user.delete.title" a.label = "reviewables.actions.reject_user.delete.title"
a.require_reject_reason = true
a.description = "reviewables.actions.reject_user.delete.description" a.description = "reviewables.actions.reject_user.delete.description"
end end
actions.add(:reject_user_block, bundle: reject) do |a| actions.add(:reject_user_block, bundle: reject) do |a|
a.icon = 'ban' a.icon = 'ban'
a.label = "reviewables.actions.reject_user.block.title" a.label = "reviewables.actions.reject_user.block.title"
a.require_reject_reason = true
a.description = "reviewables.actions.reject_user.block.description" a.description = "reviewables.actions.reject_user.block.description"
end end
end end
@ -64,6 +66,17 @@ class ReviewableUser < Reviewable
end end
begin begin
self.reject_reason = args[:reject_reason]
if args[:send_email] != false && SiteSetting.must_approve_users?
# Execute job instead of enqueue because user has to exists to send email
Jobs::CriticalUserEmail.new.execute({
type: :signup_after_reject,
user_id: target.id,
reject_reason: self.reject_reason
})
end
delete_args = {} delete_args = {}
delete_args[:block_ip] = true if args[:block_ip] delete_args[:block_ip] = true if args[:block_ip]
delete_args[:block_email] = true if args[:block_email] delete_args[:block_email] = true if args[:block_email]

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class ReviewableActionSerializer < ApplicationSerializer class ReviewableActionSerializer < ApplicationSerializer
attributes :id, :icon, :button_class, :label, :confirm_message, :description, :client_action attributes :id, :icon, :button_class, :label, :confirm_message, :description, :client_action, :require_reject_reason
def label def label
I18n.t(object.label) I18n.t(object.label)
@ -27,4 +27,7 @@ class ReviewableActionSerializer < ApplicationSerializer
object.client_action.present? object.client_action.present?
end end
def include_require_reject_reason?
object.require_reject_reason.present?
end
end end

View File

@ -2,7 +2,7 @@
class ReviewableUserSerializer < ReviewableSerializer class ReviewableUserSerializer < ReviewableSerializer
attributes :link_admin, :user_fields attributes :link_admin, :user_fields, :reject_reason
payload_attributes( payload_attributes(
:username, :username,

View File

@ -468,6 +468,7 @@ en:
email: "Email" email: "Email"
name: "Name" name: "Name"
fields: "Fields" fields: "Fields"
reject_reason: "Reason"
user_percentage: user_percentage:
summary: summary:
@ -566,6 +567,9 @@ en:
other: "You have <strong>%{count}</strong> posts pending." other: "You have <strong>%{count}</strong> posts pending."
ok: "OK" ok: "OK"
example_username: "username" example_username: "username"
reject_reason:
title: "Why are you rejecting this user?"
send_email: "Send rejection email"
user_action: user_action:
user_posted_topic: "<a href='%{userUrl}'>%{user}</a> posted <a href='%{topicUrl}'>the topic</a>" user_posted_topic: "<a href='%{userUrl}'>%{user}</a> posted <a href='%{topicUrl}'>the topic</a>"

View File

@ -3842,6 +3842,14 @@ en:
Enjoy your stay! Enjoy your stay!
signup_after_reject:
title: "Signup After Reject"
subject_template: "You've been rejected on %{site_name}"
text_body_template: |
A staff member rejected your account on %{site_name}.
%{reject_reason}
signup: signup:
title: "Signup" title: "Signup"
subject_template: "[%{email_prefix}] Confirm your new account" subject_template: "[%{email_prefix}] Confirm your new account"

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddRejectReasonToReviewables < ActiveRecord::Migration[6.0]
def change
add_column :reviewables, :reject_reason, :text
end
end

View File

@ -33,7 +33,7 @@ class Reviewable < ActiveRecord::Base
end end
class Action < Item class Action < Item
attr_accessor :icon, :button_class, :label, :description, :confirm_message, :client_action attr_accessor :icon, :button_class, :label, :description, :confirm_message, :client_action, :require_reject_reason
def initialize(id, icon = nil, button_class = nil, label = nil) def initialize(id, icon = nil, button_class = nil, label = nil)
super(id) super(id)

View File

@ -67,7 +67,7 @@ RSpec.describe ReviewableUser, type: :model do
end end
it "allows us to reject a user" do it "allows us to reject a user" do
result = reviewable.perform(moderator, :reject_user_delete) result = reviewable.perform(moderator, :reject_user_delete, reject_reason: "reject reason")
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(reviewable.pending?).to eq(false) expect(reviewable.pending?).to eq(false)
@ -76,13 +76,14 @@ RSpec.describe ReviewableUser, type: :model do
# Rejecting deletes the user record # Rejecting deletes the user record
reviewable.reload reviewable.reload
expect(reviewable.target).to be_blank expect(reviewable.target).to be_blank
expect(reviewable.reject_reason).to eq("reject reason")
end end
it "allows us to reject and block a user" do it "allows us to reject and block a user" do
email = reviewable.target.email email = reviewable.target.email
ip = reviewable.target.ip_address ip = reviewable.target.ip_address
result = reviewable.perform(moderator, :reject_user_block) result = reviewable.perform(moderator, :reject_user_block, reject_reason: "reject reason")
expect(result.success?).to eq(true) expect(result.success?).to eq(true)
expect(reviewable.pending?).to eq(false) expect(reviewable.pending?).to eq(false)
@ -91,11 +92,24 @@ RSpec.describe ReviewableUser, type: :model do
# Rejecting deletes the user record # Rejecting deletes the user record
reviewable.reload reviewable.reload
expect(reviewable.target).to be_blank expect(reviewable.target).to be_blank
expect(reviewable.reject_reason).to eq("reject reason")
expect(ScreenedEmail.should_block?(email)).to eq(true) expect(ScreenedEmail.should_block?(email)).to eq(true)
expect(ScreenedIpAddress.should_block?(ip)).to eq(true) expect(ScreenedIpAddress.should_block?(ip)).to eq(true)
end end
it "is not sending email to the user about rejection" do
SiteSetting.must_approve_users = true
Jobs::CriticalUserEmail.any_instance.expects(:execute).never
reviewable.perform(moderator, :reject_user_block, reject_reason: "reject reason", send_email: false)
end
it "optionaly sends email with reject reason" do
SiteSetting.must_approve_users = true
Jobs::CriticalUserEmail.any_instance.expects(:execute).with(type: :signup_after_reject, user_id: reviewable.target_id, reject_reason: "reject reason").once
reviewable.perform(moderator, :reject_user_block, reject_reason: "reject reason", send_email: true)
end
it "allows us to reject a user who has posts" do it "allows us to reject a user who has posts" do
Fabricate(:post, user: reviewable.target) Fabricate(:post, user: reviewable.target)
result = reviewable.perform(moderator, :reject_user_delete) result = reviewable.perform(moderator, :reject_user_delete)