diff --git a/app/jobs/regular/backfill_badge.rb b/app/jobs/regular/backfill_badge.rb index 6915fe1e015..442dafb64c9 100644 --- a/app/jobs/regular/backfill_badge.rb +++ b/app/jobs/regular/backfill_badge.rb @@ -20,10 +20,14 @@ module Jobs ) affected_user_ids = (revoked_user_ids | granted_user_ids).to_a + revoked_user_ids = revoked_user_ids.to_a - BadgeGranter.revoke_ungranted_titles!(revoked_user_ids.to_a) - UserBadge.ensure_consistency!(affected_user_ids) - UserStat.update_distinct_badge_count(affected_user_ids) + BadgeGranter.revoke_ungranted_titles!(revoked_user_ids) if revoked_user_ids.present? + + if affected_user_ids.present? + UserBadge.ensure_consistency!(affected_user_ids) + UserStat.update_distinct_badge_count(affected_user_ids) + end end end end diff --git a/app/models/user_badge.rb b/app/models/user_badge.rb index a16c13d4e75..8bec4281ca1 100644 --- a/app/models/user_badge.rb +++ b/app/models/user_badge.rb @@ -72,12 +72,13 @@ class UserBadge < ActiveRecord::Base DiscourseEvent.trigger(:user_badge_revoked, user_badge: self) end - def self.ensure_consistency!(user_ids = nil) + def self.ensure_consistency!(user_ids = []) self.update_featured_ranks!(user_ids) end - def self.update_featured_ranks!(user_ids = nil) - user_ids = user_ids.join(", ") if user_ids + def self.update_featured_ranks!(user_ids = []) + user_ids = user_ids.join(", ") + has_user_ids = !user_ids.empty? query = <<~SQL WITH featured_tl_badge AS -- Find the best trust level badge for each user @@ -85,7 +86,7 @@ class UserBadge < ActiveRecord::Base SELECT user_id, max(badge_id) as badge_id FROM user_badges WHERE badge_id IN (1,2,3,4) - #{"AND user_id IN (#{user_ids})" if user_ids} + #{"AND user_id IN (#{user_ids})" if has_user_ids} GROUP BY user_id ), ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one @@ -105,7 +106,7 @@ class UserBadge < ActiveRecord::Base FROM user_badges INNER JOIN badges ON badges.id = user_badges.badge_id LEFT JOIN featured_tl_badge ON featured_tl_badge.user_id = user_badges.user_id AND featured_tl_badge.badge_id = user_badges.badge_id - #{"WHERE user_badges.user_id IN (#{user_ids})" if user_ids} + #{"WHERE user_badges.user_id IN (#{user_ids})" if has_user_ids} GROUP BY user_badges.user_id, user_badges.badge_id ) -- Now use that data to update the featured_rank column diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb index dbc03246840..86895027680 100644 --- a/app/models/user_stat.rb +++ b/app/models/user_stat.rb @@ -192,8 +192,8 @@ class UserStat < ActiveRecord::Base SQL end - def self.update_distinct_badge_count(user_ids = nil) - user_ids = user_ids.join(", ") if user_ids + def self.update_distinct_badge_count(user_ids = []) + user_ids = user_ids.join(", ") sql = <<~SQL UPDATE user_stats @@ -206,7 +206,7 @@ class UserStat < ActiveRecord::Base GROUP BY users.id ) x WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count - #{"AND user_stats.user_id IN (#{user_ids})" if user_ids} + #{"AND user_stats.user_id IN (#{user_ids})" if !user_ids.empty?} SQL DB.exec sql diff --git a/spec/models/user_badge_spec.rb b/spec/models/user_badge_spec.rb index 406c8fc0025..2778ae855b1 100644 --- a/spec/models/user_badge_spec.rb +++ b/spec/models/user_badge_spec.rb @@ -49,6 +49,7 @@ RSpec.describe UserBadge do describe "featured rank" do fab!(:user) + fab!(:user_2) { Fabricate(:user) } fab!(:user_badge_tl1) do UserBadge.create!( badge_id: Badge::BasicUser, @@ -82,6 +83,15 @@ RSpec.describe UserBadge do ) end + fab!(:user_2_badge_tl1) do + UserBadge.create!( + badge_id: Badge::BasicUser, + user: user_2, + granted_by: Discourse.system_user, + granted_at: Time.now, + ) + end + it "gives user badges the correct rank" do expect(user_badge_tl2.reload.featured_rank).to eq(1) expect(user_badge_wiki.reload.featured_rank).to eq(2) @@ -121,15 +131,25 @@ RSpec.describe UserBadge do it "can ensure consistency per user" do user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check + user_2_badge_tl1.update_column(:featured_rank, 20) # Update without hooks + expect(user_2_badge_tl1.reload.featured_rank).to eq(20) # Double check + UserBadge.update_featured_ranks!([user.id]) + expect(user_badge_tl2.reload.featured_rank).to eq(1) + expect(user_2_badge_tl1.reload.featured_rank).to eq(20) end it "can ensure consistency for all users" do user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check - UserBadge.update_featured_ranks!([user.id]) + user_2_badge_tl1.update_column(:featured_rank, 20) # Update without hooks + expect(user_2_badge_tl1.reload.featured_rank).to eq(20) # Double check + + UserBadge.update_featured_ranks! + expect(user_badge_tl2.reload.featured_rank).to eq(1) + expect(user_2_badge_tl1.reload.featured_rank).to eq(1) end end end diff --git a/spec/models/user_stat_spec.rb b/spec/models/user_stat_spec.rb index ed45c02d501..47bc518c828 100644 --- a/spec/models/user_stat_spec.rb +++ b/spec/models/user_stat_spec.rb @@ -215,7 +215,7 @@ RSpec.describe UserStat do end end - describe "update_distinct_badge_count" do + describe ".update_distinct_badge_count" do fab!(:user) let(:stat) { user.user_stat } fab!(:badge1) { Fabricate(:badge) }