Skip to content
Permalink
Browse files

PERF: Cache ranks for featured badges, to simplify user serialization (

  • Loading branch information
davidtaylorhq committed Jan 14, 2020
1 parent e474cda commit cff6e941de7e7ccbeb0835b234f42b904441a5fa
@@ -22,6 +22,7 @@ def execute(args)
end

BadgeGranter.revoke_ungranted_titles!
UserBadge.ensure_consistency! # Badge granter sometimes uses raw SQL, so hooks do not run. Clean up data
end

end
@@ -115,6 +115,7 @@ def self.uses_post_ids?(trigger)
after_commit do
SvgSprite.expire_cache
UserStat.update_distinct_badge_count if saved_change_to_enabled?
UserBadge.ensure_consistency! if saved_change_to_enabled?
end

# fields that can not be edited on system badges
@@ -20,8 +20,14 @@ class User < ActiveRecord::Base
has_many :user_actions
has_many :post_actions

has_many :user_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }, dependent: :destroy
DEFAULT_FEATURED_BADGE_COUNT = 3

has_many :user_badges, -> { for_enabled_badges }, dependent: :destroy
has_many :badges, through: :user_badges
has_many :default_featured_user_badges,
-> { for_enabled_badges.grouped_with_count.where("featured_rank <= ?", DEFAULT_FEATURED_BADGE_COUNT) },
class_name: "UserBadge"

has_many :email_logs, dependent: :delete_all
has_many :incoming_emails, dependent: :delete_all
has_many :post_timings
@@ -961,28 +967,12 @@ def badge_count
user_stat&.distinct_badge_count
end

def featured_user_badges(limit = 3)
tl_badge_ids = Badge.trust_level_badge_ids

query = user_badges
.group(:badge_id)
.select(UserBadge.attribute_names.map { |x| "MAX(user_badges.#{x}) AS #{x}" },
'COUNT(*) AS "count"',
'MAX(badges.badge_type_id) AS badges_badge_type_id',
'MAX(badges.grant_count) AS badges_grant_count')
.joins(:badge)
.order('badges_badge_type_id ASC, badges_grant_count ASC, badge_id DESC')
.includes(:user, :granted_by, { badge: :badge_type }, post: :topic)

tl_badge = query.where("user_badges.badge_id IN (:tl_badge_ids)",
tl_badge_ids: tl_badge_ids)
.limit(1)

other_badges = query.where("user_badges.badge_id NOT IN (:tl_badge_ids)",
tl_badge_ids: tl_badge_ids)
.limit(limit)

(tl_badge + other_badges).take(limit)
def featured_user_badges(limit = DEFAULT_FEATURED_BADGE_COUNT)
if limit == DEFAULT_FEATURED_BADGE_COUNT
default_featured_user_badges
else
user_badges.grouped_with_count.where("featured_rank <= ?", limit)
end
end

def self.count_by_signup_date(start_date = nil, end_date = nil, group_id = nil)
@@ -7,6 +7,16 @@ class UserBadge < ActiveRecord::Base
belongs_to :notification, dependent: :destroy
belongs_to :post

scope :grouped_with_count, -> {
group(:badge_id, :user_id)
.select(UserBadge.attribute_names.map { |x| "MAX(user_badges.#{x}) AS #{x}" },
'COUNT(*) AS "count"')
.order('MAX(featured_rank) ASC')
.includes(:user, :granted_by, { badge: :badge_type }, post: :topic)
}

scope :for_enabled_badges, -> { where('user_badges.badge_id IN (SELECT id FROM badges WHERE enabled)') }

validates :badge_id,
presence: true,
uniqueness: { scope: :user_id },
@@ -19,15 +29,58 @@ class UserBadge < ActiveRecord::Base
after_create do
Badge.increment_counter 'grant_count', self.badge_id
UserStat.update_distinct_badge_count self.user_id
UserBadge.update_featured_ranks! 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
UserBadge.update_featured_ranks! self.user_id
DiscourseEvent.trigger(:user_badge_removed, self.badge_id, self.user_id)
end

def self.ensure_consistency!
self.update_featured_ranks!
end

def self.update_featured_ranks!(user_id = nil)
query = <<~SQL
WITH featured_tl_badge AS -- Find the best trust level badge for each user
(
SELECT user_id, max(badge_id) as badge_id
FROM user_badges
WHERE badge_id IN (1,2,3,4)
#{"AND user_id = #{user_id.to_i}" if user_id}
GROUP BY user_id
),
ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one
SELECT
user_badges.user_id,
user_badges.badge_id,
RANK() OVER (
PARTITION BY user_badges.user_id -- Do a separate rank for each user
ORDER BY BOOL_OR(badges.enabled) DESC, -- Disabled badges last
MAX(featured_tl_badge.user_id) NULLS LAST, -- Best tl badge first
CASE WHEN user_badges.badge_id IN (1,2,3,4) THEN 1 ELSE 0 END ASC, -- Non-featured tl badges last
MAX(badges.badge_type_id) ASC,
MAX(badges.grant_count) ASC,
user_badges.badge_id DESC
) rank_number
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 = #{user_id.to_i}" if user_id}
GROUP BY user_badges.user_id, user_badges.badge_id
)
-- Now use that data to update the featured_rank column
UPDATE user_badges SET featured_rank = rank_number
FROM ranks WHERE ranks.badge_id = user_badges.badge_id AND ranks.user_id = user_badges.user_id AND featured_rank IS DISTINCT FROM rank_number
SQL

DB.exec query
end

private

def single_grant_badge?
@@ -47,6 +100,7 @@ def single_grant_badge?
# post_id :integer
# notification_id :integer
# seq :integer default(0), not null
# featured_rank :integer
#
# Indexes
#
@@ -0,0 +1,37 @@
# frozen_string_literal: true
class AddFeaturedRankToUserBadges < ActiveRecord::Migration[6.0]
def change
add_column :user_badges, :featured_rank, :integer, null: true

execute <<~SQL
WITH featured_tl_badge AS -- Find the best trust level badge for each user
(
SELECT user_id, max(badge_id) as badge_id
FROM user_badges
WHERE badge_id IN (1,2,3,4)
GROUP BY user_id
),
ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one
SELECT
user_badges.user_id,
user_badges.badge_id,
RANK() OVER (
PARTITION BY user_badges.user_id -- Do a separate rank for each user
ORDER BY BOOL_OR(badges.enabled) DESC, -- Disabled badges last
MAX(featured_tl_badge.user_id) NULLS LAST, -- Best tl badge first
CASE WHEN user_badges.badge_id IN (1,2,3,4) THEN 1 ELSE 0 END ASC, -- Non-featured tl badges last
MAX(badges.badge_type_id) ASC,
MAX(badges.grant_count) ASC,
user_badges.badge_id DESC
) rank_number
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
GROUP BY user_badges.user_id, user_badges.badge_id
)
-- Now use that data to update the featured_rank column
UPDATE user_badges SET featured_rank = rank_number
FROM ranks WHERE ranks.badge_id = user_badges.badge_id AND ranks.user_id = user_badges.user_id
SQL
end
end
@@ -16,4 +16,50 @@
it { is_expected.to validate_uniqueness_of(:badge_id).scoped_to(:user_id) }
end

describe "featured rank" do
fab!(:user) { Fabricate(:user) }
fab!(:user_badge_tl1) { UserBadge.create!(badge_id: Badge::BasicUser, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_tl2) { UserBadge.create!(badge_id: Badge::Member, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_wiki) { UserBadge.create!(badge_id: Badge::WikiEditor, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
fab!(:user_badge_like) { UserBadge.create!(badge_id: Badge::FirstLike, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }

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)
expect(user_badge_like.reload.featured_rank).to eq(3)
expect(user_badge_tl1.reload.featured_rank).to eq(4) # Previous trust level badges last
end

it "gives duplicate user_badges the same rank" do
ub1 = UserBadge.create!(badge_id: Badge::GreatTopic, user: user, granted_by: Discourse.system_user, granted_at: Time.now)
ub2 = UserBadge.create!(badge_id: Badge::GreatTopic, user: user, granted_by: Discourse.system_user, granted_at: Time.now, seq: 1)

expect(ub1.reload.featured_rank).to eq(2)
expect(ub2.reload.featured_rank).to eq(2)
end

it "skips disabled badges" do
user_badge_wiki.badge.update(enabled: false)
expect(user_badge_tl2.reload.featured_rank).to eq(1)
expect(user_badge_like.reload.featured_rank).to eq(2)
expect(user_badge_tl1.reload.featured_rank).to eq(3) # Previous trust level badges last
expect(user_badge_wiki.reload.featured_rank).to eq(4) # Disabled
end

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
UserBadge.update_featured_ranks! user.id
expect(user_badge_tl2.reload.featured_rank).to eq(1)
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!
expect(user_badge_tl2.reload.featured_rank).to eq(1)
end

end

end
@@ -1706,15 +1706,12 @@ def hash(password, salt)

describe "#featured_user_badges" do
fab!(:user) { Fabricate(:user) }
let!(:user_badge_tl1) { UserBadge.create(badge_id: 1, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl2) { UserBadge.create(badge_id: 2, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl1) { UserBadge.create(badge_id: Badge::BasicUser, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_tl2) { UserBadge.create(badge_id: Badge::Member, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
let!(:user_badge_like) { UserBadge.create(badge_id: Badge::FirstLike, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }

it 'should display highest trust level badge first' do
expect(user.featured_user_badges[0].badge_id).to eq(2)
end

it 'should display only 1 trust level badge' do
expect(user.featured_user_badges.length).to eq(1)
it 'should display badges in the correct order' do
expect(user.featured_user_badges.map(&:badge_id)).to eq([Badge::Member, Badge::FirstLike, Badge::BasicUser])
end
end

1 comment on commit cff6e94

Please sign in to comment.
You can’t perform that action at this time.