FIX: Missing/incorrect flag reason in account_deleted emails (#34955)

`UserNotifications#account_deleted` builds a localized flag reason from
the reviewable’s first `reviewable_score` (falling back to `spam` if
none is found).

Currently, some score types (e.g. `needs_approval`) are not covered. In
cases where a "Needs Approval" flag led to an account deletion, the
lookup key could end up as `flag_reasons.`, causing the entire
`flag_reasons`
 section to be rendered instead of a single entry.

This change adds support for additional score types (like
`needs_approval`). If no translation exists (e.g. custom flags), the
flag’s description will be used if available, before falling back to the
default `spam` reason.

More context:
https://meta.discourse.org/t/ruby-hash-syntax-being-displayed-in-emails-sent-to-deleted-users/382411
This commit is contained in:
Selase Krakani
2025-11-17 15:22:22 +00:00
committed by GitHub
parent 1182c41d14
commit 467fcb460a
5 changed files with 89 additions and 8 deletions
+14 -8
View File
@@ -179,14 +179,20 @@ class UserNotifications < ActionMailer::Base
end
def account_deleted(email, reviewable)
post_action_type_id =
reviewable.reviewable_scores.first&.reviewable_score_type ||
PostActionTypeView.new.types[:spam]
build_email(
email,
template: "user_notifications.account_deleted",
flag_reason: I18n.t("flag_reasons.#{PostActionTypeView.new.types[post_action_type_id]}"),
)
post_action_type_view = PostActionTypeView.new
post_action_type_id = reviewable.reviewable_scores.first&.reviewable_score_type
flag_reason =
if post_action_type_id
reason_key = post_action_type_view.flag_and_score_types[post_action_type_id]
reason = I18n.t("flag_reasons.#{reason_key}", default: nil).presence if reason_key
reason || post_action_type_view.descriptions[post_action_type_id].presence
end
flag_reason ||= I18n.t("flag_reasons.spam")
build_email(email, template: "user_notifications.account_deleted", flag_reason: flag_reason)
end
def account_suspended(user, opts = nil)
+2
View File
@@ -3379,11 +3379,13 @@ en:
illegal: "Your post was flagged as **illegal**: the community thinks it might be breaking the law."
spam: "Your post was flagged as **spam**: the community feels it is an advertisement, something that is overly promotional in nature instead of being useful or relevant to the topic as expected."
notify_moderators: "Your post was flagged **for moderator attention**: the community feels something about the post requires manual intervention by a staff member."
needs_approval: "Your post was flagged **for approval**: it has been automatically queued for review by a staff member to ensure it meets [our community guidelines](%{base_path}/guidelines)."
responder:
off_topic: "The post was flagged as **off-topic**: the community feels it is not a good fit for the topic, as currently defined by the title and the first post."
inappropriate: "The post was flagged as **inappropriate**: the community feels it is offensive, abusive, to be hateful conduct or a violation of [our community guidelines](%{base_path}/guidelines)."
spam: "The post was flagged as **spam**: the community feels it is an advertisement, something that is overly promotional in nature instead of being useful or relevant to the topic as expected."
notify_moderators: "The post was flagged **for moderator attention**: the community feels something about the post requires manual intervention by a staff member."
needs_approval: "The post was flagged **for approval**: it has been automatically queued for review by a staff member to ensure it meets [our community guidelines](%{base_path}/guidelines)."
flags_dispositions:
agreed: "Thanks for letting us know. We agree there is an issue and we're looking into it."
+4
View File
@@ -79,6 +79,10 @@ class PostActionTypeView
flag_enum(all_flags.filter { |flag| flag[:score_type] })
end
def flag_and_score_types
flag_types.merge(score_types)
end
# flags resulting in mod notifications
def notify_flag_type_ids
notify_flag_types.values
+12
View File
@@ -29,6 +29,18 @@ RSpec.describe PostActionTypeView do
)
expect(post_action_type_view.score_types).to eq({ needs_approval: 9 })
expect(post_action_type_view.flag_and_score_types).to eq(
{
illegal: 10,
inappropriate: 4,
notify_moderators: 7,
notify_user: 6,
off_topic: 3,
spam: 8,
needs_approval: 9,
},
)
flag = Fabricate(:flag, name: "flag", enabled: false)
expect(PostActionTypeView.new.disabled_flag_types).to eq({ custom_flag: flag.id })
flag.destroy!
+57
View File
@@ -1577,6 +1577,63 @@ RSpec.describe UserNotifications do
end
end
describe ".account_deleted" do
fab!(:reviewable) { Fabricate(:reviewable_flagged_post, reviewable_scores: []) }
it "includes flag reason for standard flags" do
Fabricate(
:reviewable_score,
reviewable: reviewable,
reviewable_score_type: PostActionType.types[:spam],
)
reviewable.reload
mail = UserNotifications.account_deleted("user@example.com", reviewable)
expect(mail.body).to include(I18n.t("flag_reasons.spam"))
expect(mail.body).to_not include(I18n.t("flag_reasons.illegal"))
end
it "includes flag reason for automated system flags" do
Fabricate(
:reviewable_score,
reviewable: reviewable,
reviewable_score_type: ReviewableScore.types[:needs_approval],
)
reviewable.reload
mail = UserNotifications.account_deleted("user@example.com", reviewable)
expect(mail.body).to include(I18n.t("flag_reasons.needs_approval"))
expect(mail.body).to_not include(I18n.t("flag_reasons.spam"))
end
it "falls back to the default spam reason if no score is present" do
mail = UserNotifications.account_deleted("user@example.com", reviewable)
expect(mail.body).to include(I18n.t("flag_reasons.spam"))
expect(mail.body).to_not include(I18n.t("flag_reasons.illegal"))
end
it "falls back to the flag's description for custom flags" do
custom_flag =
Fabricate(
:flag,
name_key: "custom_delete_flag",
description: "This is a custom deletion reason.",
applies_to: %w[Post],
)
Fabricate(:reviewable_score, reviewable: reviewable, reviewable_score_type: custom_flag.id)
reviewable.reload
mail = UserNotifications.account_deleted("user@example.com", reviewable)
expect(mail.body).to include(custom_flag.description)
expect(mail.body).to_not include(I18n.t("flag_reasons.spam"))
end
end
describe ".account_suspended" do
fab!(:user_history) { Fabricate(:user_history, action: UserHistory.actions[:suspend_user]) }