diff --git a/app/models/badge.rb b/app/models/badge.rb index 606ecb02d2d..32a52c27549 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -305,6 +305,10 @@ class Badge < ActiveRecord::Base end end + def for_beginners? + id == Welcome || (badge_grouping_id == BadgeGrouping::GettingStarted && id != NewUserOfTheMonth) + end + protected def ensure_not_system diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb index f3bd9bb0f57..1d52ea6ec78 100644 --- a/app/services/badge_granter.rb +++ b/app/services/badge_granter.rb @@ -46,7 +46,6 @@ class BadgeGranter return if @granted_by && !Guardian.new(@granted_by).can_grant_badges?(@user) return unless @badge.present? && @badge.enabled? 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 } @@ -77,7 +76,8 @@ class BadgeGranter StaffActionLogger.new(@granted_by).log_badge_grant(user_badge) 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) user_badge.update!(notification_id: notification.id) end @@ -345,9 +345,10 @@ class BadgeGranter ON CONFLICT DO NOTHING 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 JOIN users u on u.id = w.user_id + JOIN user_options uo ON uo.user_id = w.user_id SQL builder = DB.build(sql) @@ -375,8 +376,7 @@ class BadgeGranter post_ids: post_ids || [-2], user_ids: user_ids || [-2]).each do |row| - # old bronze badges do not matter - next if badge.badge_type_id == BadgeType::Bronze && row.granted_at < 2.days.ago + next if suppress_notification?(badge, row.granted_at, row.skip_new_user_tips) next if row.staff && badge.awarded_for_trust_level? notification = send_notification(row.user_id, row.username, row.locale, badge) @@ -450,4 +450,10 @@ class BadgeGranter notification 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 diff --git a/spec/services/badge_granter_spec.rb b/spec/services/badge_granter_spec.rb index 929791eddd8..1679f067223 100644 --- a/spec/services/badge_granter_spec.rb +++ b/spec/services/badge_granter_spec.rb @@ -195,6 +195,16 @@ describe BadgeGranter do badge.query = Badge.find(1).query + "\n-- a comment" expect { BadgeGranter.backfill(badge) }.not_to raise_error 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 describe 'grant' do @@ -222,22 +232,25 @@ describe BadgeGranter do expect(user_badge).to eq(nil) 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 + UserBadge.destroy_all user.user_option.update!(skip_new_user_tips: true) badge = Fabricate(:badge, badge_grouping_id: BadgeGrouping::GettingStarted) - user_badge = BadgeGranter.grant(badge, user, created_at: 1.year.ago) - expect(user_badge).to eq(nil) + expect { + BadgeGranter.grant(badge, user) + }.to_not change { Notification.where(user_id: user.id).count } 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 user.user_option.update!(skip_new_user_tips: true) badge = Badge.find(Badge::NewUserOfTheMonth) - user_badge = BadgeGranter.grant(badge, user, created_at: 1.year.ago) - expect(user_badge).to be_present + expect { + BadgeGranter.grant(badge, user) + }.to change { Notification.where(user_id: user.id).count } end it 'grants multiple badges' do