Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Rejection email sent even if reject reason too long (#27529) 

Followup 6b872c4c53

Even though we were showing a validation error for a reject
reason that was too long, we were still sending an email and
doing other operations on the user which we are rejecting.

This commit fixes this by validating the reviewable model
before attempting to do anything else after the reason is set.
This commit is contained in:
Martin Brennan 2024-06-19 11:07:23 +10:00 committed by GitHub
parent cc4c199680
commit ebdbb199a5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 12 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/models/reviewable_user.rb
View File

@ -46,6 +46,9 @@ class ReviewableUser < Reviewable
begin begin
self.reject_reason = args[:reject_reason] self.reject_reason = args[:reject_reason]
# Without this, we end up sending the email even if this reject_reason is too long.
self.validate!
if args[:send_email] && SiteSetting.must_approve_users? if args[:send_email] && SiteSetting.must_approve_users?
# Execute job instead of enqueue because user has to exists to send email # Execute job instead of enqueue because user has to exists to send email
Jobs::CriticalUserEmail.new.execute( Jobs::CriticalUserEmail.new.execute(

15
spec/requests/reviewables_controller_spec.rb
View File

@ -187,7 +187,7 @@ RSpec.describe ReviewablesController do
expect(json["errors"][0]).to eq(I18n.t("reviewables.already_handled_and_user_not_exist")) expect(json["errors"][0]).to eq(I18n.t("reviewables.already_handled_and_user_not_exist"))
end end
it "returns a readable error message if reject_reason is too long" do it "returns a readable error message if reject_reason is too long, does not send email, and does not delete the user" do
sign_in(admin) sign_in(admin)
Jobs.run_immediately! Jobs.run_immediately!
SiteSetting.must_approve_users = true SiteSetting.must_approve_users = true
@ -195,11 +195,14 @@ RSpec.describe ReviewablesController do
user.activate user.activate
reviewable = ReviewableUser.find_by(target: user) reviewable = ReviewableUser.find_by(target: user)
put "/review/#{reviewable.id}/perform/delete_user.json?version=0", expect {
params: { put "/review/#{reviewable.id}/perform/delete_user.json?version=0",
send_email: true, params: {
reject_reason: "a" * 3000, send_email: true,
} reject_reason: "a" * 3000,
}
}.to not_change { ActionMailer::Base.deliveries.size }.and not_change { User.count }
expect(response.code).to eq("422") expect(response.code).to eq("422")
expect(response.parsed_body["errors"]).to eq( expect(response.parsed_body["errors"]).to eq(
["Reject reason " + I18n.t("errors.messages.too_long", count: 2000)], ["Reject reason " + I18n.t("errors.messages.too_long", count: 2000)],