diff --git a/app/models/badge.rb b/app/models/badge.rb index 091d080dc86..10e228bbfaf 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -114,6 +114,7 @@ class Badge < ActiveRecord::Base after_commit do SvgSprite.expire_cache + UserStat.update_distinct_badge_count if saved_change_to_enabled? end # fields that can not be edited on system badges diff --git a/app/models/user.rb b/app/models/user.rb index b5247b85fdb..1c06d191609 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -945,7 +945,7 @@ class User < ActiveRecord::Base end def badge_count - user_badges.select('distinct badge_id').count + user_stat&.distinct_badge_count end def featured_user_badges(limit = 3) diff --git a/app/models/user_badge.rb b/app/models/user_badge.rb index e915a7c6e24..c9b84787f95 100644 --- a/app/models/user_badge.rb +++ b/app/models/user_badge.rb @@ -18,11 +18,13 @@ class UserBadge < ActiveRecord::Base after_create do Badge.increment_counter 'grant_count', self.badge_id + UserStat.update_distinct_badge_count self.user_id DiscourseEvent.trigger(:user_badge_granted, self.badge_id, self.user_id) end after_destroy do Badge.decrement_counter 'grant_count', self.badge_id + UserStat.update_distinct_badge_count self.user_id DiscourseEvent.trigger(:user_badge_removed, self.badge_id, self.user_id) end diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb index e76c8c113a8..ffd7057593a 100644 --- a/app/models/user_stat.rb +++ b/app/models/user_stat.rb @@ -6,6 +6,7 @@ class UserStat < ActiveRecord::Base def self.ensure_consistency!(last_seen = 1.hour.ago) reset_bounce_scores + update_distinct_badge_count update_view_counts(last_seen) update_first_unread(last_seen) end @@ -126,6 +127,29 @@ class UserStat < ActiveRecord::Base SQL end + def self.update_distinct_badge_count(user_id = nil) + sql = <<~SQL + UPDATE user_stats + SET distinct_badge_count = x.distinct_badge_count + FROM ( + SELECT users.id user_id, COUNT(distinct user_badges.badge_id) distinct_badge_count + FROM users + LEFT JOIN user_badges ON user_badges.user_id = users.id + AND (user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)) + GROUP BY users.id + ) x + WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count + SQL + + sql = sql + " AND user_stats.user_id = #{user_id.to_i}" if user_id + + DB.exec sql + end + + def update_distinct_badge_count + self.class.update_distinct_badge_count(self.user_id) + end + # topic_reply_count is a count of posts in other users' topics def update_topic_reply_count self.topic_reply_count = Topic diff --git a/db/migrate/20191220134101_add_distinct_badge_count_to_user_stats.rb b/db/migrate/20191220134101_add_distinct_badge_count_to_user_stats.rb new file mode 100644 index 00000000000..13afe64ad6d --- /dev/null +++ b/db/migrate/20191220134101_add_distinct_badge_count_to_user_stats.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true +class AddDistinctBadgeCountToUserStats < ActiveRecord::Migration[6.0] + def change + add_column :user_stats, :distinct_badge_count, :integer, default: 0, null: false + + execute <<~SQL + UPDATE user_stats + SET distinct_badge_count = x.distinct_badge_count + FROM ( + SELECT users.id user_id, COUNT(distinct user_badges.badge_id) distinct_badge_count + FROM users + LEFT JOIN user_badges ON user_badges.user_id = users.id + AND (user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)) + GROUP BY users.id + ) x + WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count + SQL + end +end diff --git a/spec/models/user_stat_spec.rb b/spec/models/user_stat_spec.rb index 296bb4bc3bc..d5c563770a2 100644 --- a/spec/models/user_stat_spec.rb +++ b/spec/models/user_stat_spec.rb @@ -130,4 +130,56 @@ describe UserStat do end end + + describe 'update_distinct_badge_count' do + fab!(:user) { Fabricate(:user) } + let(:stat) { user.user_stat } + fab!(:badge1) { Fabricate(:badge) } + fab!(:badge2) { Fabricate(:badge) } + + it "updates counts correctly" do + expect(stat.reload.distinct_badge_count).to eq(0) + BadgeGranter.grant(badge1, user) + expect(stat.reload.distinct_badge_count).to eq(1) + BadgeGranter.grant(badge1, user) + expect(stat.reload.distinct_badge_count).to eq(1) + BadgeGranter.grant(badge2, user) + expect(stat.reload.distinct_badge_count).to eq(2) + user.reload.user_badges.destroy_all + expect(stat.reload.distinct_badge_count).to eq(0) + end + + it "can update counts for all users simultaneously" do + user2 = Fabricate(:user) + stat2 = user2.user_stat + + BadgeGranter.grant(badge1, user) + BadgeGranter.grant(badge1, user) + BadgeGranter.grant(badge2, user) + + BadgeGranter.grant(badge1, user2) + + # Set some clearly incorrect values + stat.update(distinct_badge_count: 999) + stat2.update(distinct_badge_count: 999) + + UserStat.ensure_consistency! + + expect(stat.reload.distinct_badge_count).to eq(2) + expect(stat2.reload.distinct_badge_count).to eq(1) + end + + it "ignores disabled badges" do + BadgeGranter.grant(badge1, user) + BadgeGranter.grant(badge2, user) + expect(stat.reload.distinct_badge_count).to eq(2) + + badge2.update(enabled: false) + expect(stat.reload.distinct_badge_count).to eq(1) + + badge2.update(enabled: true) + expect(stat.reload.distinct_badge_count).to eq(2) + end + + end end