FIX: Skip notifications about beginner badges (#12819)

FIX: Skip notifications about beginner badges (#12819)

diff --git a/app/models/badge.rb b/app/models/badge.rb
index 606ecb0..32a52c2 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 f3bd9bb..1d52ea6 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 929791e..1679f06 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

GitHub sha: f7aeb257

This commit appears in #12819 which was approved by ZogStriP. It was merged by AndrewPrigorshnev.