Skip to content

Commit 72d7c44

Browse files
authored
Exclude users with a negative score from email eligibility. (forem#22818)
* Exclude users with a negative score from email eligibility. * Add throttle
1 parent 8250c1e commit 72d7c44

File tree

4 files changed

+49
-3
lines changed

4 files changed

+49
-3
lines changed

app/models/user.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ class User < ApplicationRecord
189189
.without_role(:spam)
190190
.where(notification_setting: { email_newsletter: true })
191191
.where.not(email: ["", nil])
192+
.where("users.score >= 0")
192193
end
193194
}
194195
scope :invited, -> { where(registered: false) }
@@ -266,7 +267,7 @@ class User < ApplicationRecord
266267

267268
after_create_commit :send_welcome_notification
268269

269-
after_save :sync_base_email_eligible!, if: -> { saved_changes.key?(:email) || saved_changes.key?(:registered) }
270+
after_save :sync_base_email_eligible!, if: -> { saved_changes.key?(:email) || saved_changes.key?(:registered) || saved_changes.key?(:score) }
270271
after_save :create_conditional_autovomits
271272
after_save :generate_social_images
272273
after_commit :subscribe_to_mailchimp_newsletter
@@ -346,6 +347,7 @@ def calculate_score
346347
calculated_score = (badge_achievements_count * 10) + user_reaction_points
347348
calculated_score -= 500 if spam?
348349
update_column(:score, calculated_score)
350+
sync_base_email_eligible!
349351
AlgoliaSearch::SearchIndexWorker.perform_async(self.class.name, id, false)
350352
end
351353

@@ -748,12 +750,13 @@ def update_presence!
748750

749751
def sync_base_email_eligible!
750752
# User is eligible if they are registered, have an email, are not suspended,
751-
# not marked as spam, and have email_newsletter set to true.
753+
# not marked as spam, have email_newsletter set to true, and their score is not below zero.
752754
is_eligible = registered? &&
753755
email.present? &&
754756
!has_role?(:suspended) &&
755757
!has_role?(:spam) &&
756-
notification_setting&.email_newsletter?
758+
notification_setting&.email_newsletter? &&
759+
score.to_i >= 0
757760

758761
if has_attribute?(:base_email_eligible) && self[:base_email_eligible] != is_eligible
759762
update_column(:base_email_eligible, is_eligible)

app/workers/badge_achievements/send_email_notification_worker.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
module BadgeAchievements
22
class SendEmailNotificationWorker
33
include Sidekiq::Job
4+
include Sidekiq::Throttled::Job
5+
6+
sidekiq_throttle(concurrency: { limit: 8 })
47

58
sidekiq_options queue: :low_priority, retry: 10
69

spec/models/user_email_eligible_spec.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@
3939
expect(user.base_email_eligible).to eq(false)
4040
end
4141

42+
it "sets to false when score is below zero" do
43+
user.update!(score: -1)
44+
expect(user.base_email_eligible).to eq(false)
45+
end
46+
47+
it "sets to true when score is directly set to zero or above" do
48+
user.update!(score: 0)
49+
expect(user.base_email_eligible).to eq(true)
50+
user.update!(score: 1)
51+
expect(user.base_email_eligible).to eq(true)
52+
end
53+
4254
it "syncs correctly when role is removed" do
4355
user.add_role(:suspended)
4456
expect(user.base_email_eligible).to eq(false)
@@ -102,6 +114,14 @@
102114
user.notification_setting.update!(email_newsletter: true)
103115
expect(user.reload.base_email_eligible).to eq(true)
104116
end
117+
118+
it "syncs when score is updated" do
119+
user.update!(score: -10)
120+
expect(user.reload.base_email_eligible).to eq(false)
121+
122+
user.update!(score: 10)
123+
expect(user.reload.base_email_eligible).to eq(true)
124+
end
105125
end
106126

107127
describe ".email_eligible scope" do
@@ -117,6 +137,12 @@
117137
end
118138
end
119139

140+
let!(:negative_score_user) do
141+
create(:user, registered: true, email: "negative@example.com", score: -10).tap do |u|
142+
u.notification_setting.update!(email_newsletter: true)
143+
end
144+
end
145+
120146
context "when USE_BASE_EMAIL_ELIGIBLE_COLUMN is true" do
121147
before do
122148
stub_const("ENV", ENV.to_hash.merge("USE_BASE_EMAIL_ELIGIBLE_COLUMN" => "true"))
@@ -126,9 +152,11 @@
126152
# We manually update one parameter without firing callbacks to truly test the column itself
127153
eligible_user.update_column(:base_email_eligible, true)
128154
ineligible_user.update_column(:base_email_eligible, false)
155+
negative_score_user.update_column(:base_email_eligible, false)
129156

130157
expect(User.email_eligible).to include(eligible_user)
131158
expect(User.email_eligible).not_to include(ineligible_user)
159+
expect(User.email_eligible).not_to include(negative_score_user)
132160
end
133161
end
134162

@@ -140,6 +168,7 @@
140168
it "uses the legacy associations" do
141169
expect(User.email_eligible).to include(eligible_user)
142170
expect(User.email_eligible).not_to include(ineligible_user)
171+
expect(User.email_eligible).not_to include(negative_score_user)
143172
end
144173
end
145174
end

spec/models/user_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,17 @@ def provider_username(service_name)
736736
user.calculate_score
737737
expect(user.score).to eq(-470)
738738
end
739+
740+
it "syncs base_email_eligible! after calculating score" do
741+
user.update!(email: "test@example.com", registered: true)
742+
user.notification_setting.update!(email_newsletter: true)
743+
expect(user.base_email_eligible).to eq(true)
744+
745+
user.add_role(:spam)
746+
user.calculate_score
747+
748+
expect(user.base_email_eligible).to eq(false)
749+
end
739750
end
740751

741752
describe "cache counts" do

0 commit comments

Comments
 (0)