FIX: Order tags shown in email subject by topics count and name (#22586)

Why this change?

Prior to this change, the ordering of the tags shown in the email subject
was non-deterministic as there was no specific order specified. This
problem was exposed by a flaky test which we had.

What is the fix?

This commit orders the tags used in the email subject first by the
`Tag#public_topic_count` column in descending order and then the `Tag#name`
column in ascending order.
This commit is contained in:
Alan Guo Xiang Tan 2023-07-13 15:39:58 +08:00 committed by GitHub
parent 82c03127df
commit 9c9058d0c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 13 additions and 9 deletions

View File

@ -613,6 +613,7 @@ class UserNotifications < ActionMailer::Base
.visible_tags(Guardian.new(user))
.joins(:topic_tags)
.where("topic_tags.topic_id = ?", post.topic_id)
.order("tags.public_topic_count DESC", "tags.name ASC")
.limit(max_tags)
.pluck(:name)

View File

@ -444,9 +444,9 @@ RSpec.describe UserNotifications do
describe ".user_replied" do
let(:response_by_user) { Fabricate(:user, name: "John Doe") }
let(:category) { Fabricate(:category, name: "India") }
let(:tag1) { Fabricate(:tag, name: "Taggo") }
let(:tag2) { Fabricate(:tag, name: "Taggie") }
let(:tag3) { Fabricate(:tag, name: "Teggo") }
let(:tag1) { Fabricate(:tag, name: "Taggo", public_topic_count: 1) }
let(:tag2) { Fabricate(:tag, name: "Taggie", public_topic_count: 3) }
let(:tag3) { Fabricate(:tag, name: "Teggo", public_topic_count: 2) }
let(:hidden_tag) { Fabricate(:tag, name: "hidden") }
let!(:hidden_tag_group) do
@ -569,6 +569,7 @@ RSpec.describe UserNotifications do
"[%{site_name}] %{optional_pm}%{optional_cat}%{optional_tags}%{topic_title}"
SiteSetting.max_tags_per_topic = 1
SiteSetting.max_tags_per_email_subject = 2
mail =
UserNotifications.user_replied(
user,
@ -576,9 +577,10 @@ RSpec.describe UserNotifications do
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash,
)
expect(mail.subject).to match(/Taggo/)
expect(mail.subject).to match(/Taggie/)
expect(mail.subject).not_to match(/Teggo/)
expect(mail.subject).to eq(
"[Discourse] [#{category.name}] #{tag2.name} #{tag3.name} #{topic.title}",
)
end
end
@ -598,9 +600,10 @@ RSpec.describe UserNotifications do
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash,
)
expect(mail.subject).to match(/Taggo/)
expect(mail.subject).to match(/Taggie/)
expect(mail.subject).not_to match(/Teggo/)
expect(mail.subject).to eq(
"[Discourse] [#{category.name}] #{tag2.name} #{tag3.name} #{topic.title}",
)
end
end
end