FIX: Skip notifications about beginner badges (#12819)

This commit is contained in:
Andrei Prigorshnev 2021-04-26 11:41:51 +04:00 committed by GitHub
parent 886f4b589e
commit f7aeb257ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 11 deletions

View File

@ -305,6 +305,10 @@ class Badge < ActiveRecord::Base
end end
end end
def for_beginners?
id == Welcome || (badge_grouping_id == BadgeGrouping::GettingStarted && id != NewUserOfTheMonth)
end
protected protected
def ensure_not_system def ensure_not_system

View File

@ -46,7 +46,6 @@ class BadgeGranter
return if @granted_by && !Guardian.new(@granted_by).can_grant_badges?(@user) return if @granted_by && !Guardian.new(@granted_by).can_grant_badges?(@user)
return unless @badge.present? && @badge.enabled? return unless @badge.present? && @badge.enabled?
return if @user.blank? return if @user.blank?
return if @badge.badge_grouping_id == BadgeGrouping::GettingStarted && @badge.id != Badge::NewUserOfTheMonth && @user.user_option.skip_new_user_tips
find_by = { badge_id: @badge.id, user_id: @user.id } find_by = { badge_id: @badge.id, user_id: @user.id }
@ -77,7 +76,8 @@ class BadgeGranter
StaffActionLogger.new(@granted_by).log_badge_grant(user_badge) StaffActionLogger.new(@granted_by).log_badge_grant(user_badge)
end end
unless @badge.badge_type_id == BadgeType::Bronze && user_badge.granted_at < 2.days.ago skip_new_user_tips = @user.user_option.skip_new_user_tips
unless self.class.suppress_notification?(@badge, user_badge.granted_at, skip_new_user_tips)
notification = self.class.send_notification(@user.id, @user.username, @user.effective_locale, @badge) notification = self.class.send_notification(@user.id, @user.username, @user.effective_locale, @badge)
user_badge.update!(notification_id: notification.id) user_badge.update!(notification_id: notification.id)
end end
@ -345,9 +345,10 @@ class BadgeGranter
ON CONFLICT DO NOTHING ON CONFLICT DO NOTHING
RETURNING id, user_id, granted_at RETURNING id, user_id, granted_at
) )
SELECT w.*, username, locale, (u.admin OR u.moderator) AS staff SELECT w.*, username, locale, (u.admin OR u.moderator) AS staff, uo.skip_new_user_tips
FROM w FROM w
JOIN users u on u.id = w.user_id JOIN users u on u.id = w.user_id
JOIN user_options uo ON uo.user_id = w.user_id
SQL SQL
builder = DB.build(sql) builder = DB.build(sql)
@ -375,8 +376,7 @@ class BadgeGranter
post_ids: post_ids || [-2], post_ids: post_ids || [-2],
user_ids: user_ids || [-2]).each do |row| user_ids: user_ids || [-2]).each do |row|
# old bronze badges do not matter next if suppress_notification?(badge, row.granted_at, row.skip_new_user_tips)
next if badge.badge_type_id == BadgeType::Bronze && row.granted_at < 2.days.ago
next if row.staff && badge.awarded_for_trust_level? next if row.staff && badge.awarded_for_trust_level?
notification = send_notification(row.user_id, row.username, row.locale, badge) notification = send_notification(row.user_id, row.username, row.locale, badge)
@ -450,4 +450,10 @@ class BadgeGranter
notification notification
end end
def self.suppress_notification?(badge, granted_at, skip_new_user_tips)
is_old_bronze_badge = badge.badge_type_id == BadgeType::Bronze && granted_at < 2.days.ago
skip_beginner_badge = skip_new_user_tips && badge.for_beginners?
is_old_bronze_badge || skip_beginner_badge
end
end end

View File

@ -195,6 +195,16 @@ describe BadgeGranter do
badge.query = Badge.find(1).query + "\n-- a comment" badge.query = Badge.find(1).query + "\n-- a comment"
expect { BadgeGranter.backfill(badge) }.not_to raise_error expect { BadgeGranter.backfill(badge) }.not_to raise_error
end end
it 'does not notify about badges "for beginners" when user skipped new user tips' do
user.user_option.update!(skip_new_user_tips: true)
post = Fabricate(:post)
PostActionCreator.like(user, post)
expect {
BadgeGranter.backfill(Badge.find(Badge::FirstLike))
}.to_not change { Notification.where(user_id: user.id).count }
end
end end
describe 'grant' do describe 'grant' do
@ -222,22 +232,25 @@ describe BadgeGranter do
expect(user_badge).to eq(nil) expect(user_badge).to eq(nil)
end end
it "doesn't grant 'getting started' badges when user skipped new user tips" do it "doesn't notify about badges 'for beginners' when user skipped new user tips" do
freeze_time freeze_time
UserBadge.destroy_all
user.user_option.update!(skip_new_user_tips: true) user.user_option.update!(skip_new_user_tips: true)
badge = Fabricate(:badge, badge_grouping_id: BadgeGrouping::GettingStarted) badge = Fabricate(:badge, badge_grouping_id: BadgeGrouping::GettingStarted)
user_badge = BadgeGranter.grant(badge, user, created_at: 1.year.ago) expect {
expect(user_badge).to eq(nil) BadgeGranter.grant(badge, user)
}.to_not change { Notification.where(user_id: user.id).count }
end end
it "grants the New User of the Month badge when user skipped new user tips" do it "notifies about the New User of the Month badge when user skipped new user tips" do
freeze_time freeze_time
user.user_option.update!(skip_new_user_tips: true) user.user_option.update!(skip_new_user_tips: true)
badge = Badge.find(Badge::NewUserOfTheMonth) badge = Badge.find(Badge::NewUserOfTheMonth)
user_badge = BadgeGranter.grant(badge, user, created_at: 1.year.ago) expect {
expect(user_badge).to be_present BadgeGranter.grant(badge, user)
}.to change { Notification.where(user_id: user.id).count }
end end
it 'grants multiple badges' do it 'grants multiple badges' do