mirror of
https://github.com/discourse/discourse.git
synced 2025-05-25 19:29:34 +08:00
FIX: Query syntax error in UserBadge.update_featured_ranks!
(#30979)
This commit fixes an SQL syntax error in `UserBadge.update_featured_ranks!` when the `user_ids` param is an empty array `[]`. This was causing the `Jobs::BackfillBadge` job to raise the following exceptions: ``` Job exception: ERROR: syntax error at or near ")" LINE 6: AND user_id IN () ``` This commit fixes the same error in `UserState.update_distinct_badge_count` as well Follow-up to 3e4eac0fed05daedcdea50d6275e143469d55eda
This commit is contained in:

committed by
GitHub

parent
084ed7a457
commit
62c6ee0de7
@ -20,10 +20,14 @@ module Jobs
|
|||||||
)
|
)
|
||||||
|
|
||||||
affected_user_ids = (revoked_user_ids | granted_user_ids).to_a
|
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)
|
BadgeGranter.revoke_ungranted_titles!(revoked_user_ids) if revoked_user_ids.present?
|
||||||
|
|
||||||
|
if affected_user_ids.present?
|
||||||
UserBadge.ensure_consistency!(affected_user_ids)
|
UserBadge.ensure_consistency!(affected_user_ids)
|
||||||
UserStat.update_distinct_badge_count(affected_user_ids)
|
UserStat.update_distinct_badge_count(affected_user_ids)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
@ -72,12 +72,13 @@ class UserBadge < ActiveRecord::Base
|
|||||||
DiscourseEvent.trigger(:user_badge_revoked, user_badge: self)
|
DiscourseEvent.trigger(:user_badge_revoked, user_badge: self)
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.ensure_consistency!(user_ids = nil)
|
def self.ensure_consistency!(user_ids = [])
|
||||||
self.update_featured_ranks!(user_ids)
|
self.update_featured_ranks!(user_ids)
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.update_featured_ranks!(user_ids = nil)
|
def self.update_featured_ranks!(user_ids = [])
|
||||||
user_ids = user_ids.join(", ") if user_ids
|
user_ids = user_ids.join(", ")
|
||||||
|
has_user_ids = !user_ids.empty?
|
||||||
|
|
||||||
query = <<~SQL
|
query = <<~SQL
|
||||||
WITH featured_tl_badge AS -- Find the best trust level badge for each user
|
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
|
SELECT user_id, max(badge_id) as badge_id
|
||||||
FROM user_badges
|
FROM user_badges
|
||||||
WHERE badge_id IN (1,2,3,4)
|
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
|
GROUP BY user_id
|
||||||
),
|
),
|
||||||
ranks AS ( -- Take all user badges, group by user_id and badge_id, and calculate a rank for each one
|
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
|
FROM user_badges
|
||||||
INNER JOIN badges ON badges.id = user_badges.badge_id
|
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
|
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
|
GROUP BY user_badges.user_id, user_badges.badge_id
|
||||||
)
|
)
|
||||||
-- Now use that data to update the featured_rank column
|
-- Now use that data to update the featured_rank column
|
||||||
|
@ -192,8 +192,8 @@ class UserStat < ActiveRecord::Base
|
|||||||
SQL
|
SQL
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.update_distinct_badge_count(user_ids = nil)
|
def self.update_distinct_badge_count(user_ids = [])
|
||||||
user_ids = user_ids.join(", ") if user_ids
|
user_ids = user_ids.join(", ")
|
||||||
|
|
||||||
sql = <<~SQL
|
sql = <<~SQL
|
||||||
UPDATE user_stats
|
UPDATE user_stats
|
||||||
@ -206,7 +206,7 @@ class UserStat < ActiveRecord::Base
|
|||||||
GROUP BY users.id
|
GROUP BY users.id
|
||||||
) x
|
) x
|
||||||
WHERE user_stats.user_id = x.user_id AND user_stats.distinct_badge_count <> x.distinct_badge_count
|
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
|
SQL
|
||||||
|
|
||||||
DB.exec sql
|
DB.exec sql
|
||||||
|
@ -49,6 +49,7 @@ RSpec.describe UserBadge do
|
|||||||
|
|
||||||
describe "featured rank" do
|
describe "featured rank" do
|
||||||
fab!(:user)
|
fab!(:user)
|
||||||
|
fab!(:user_2) { Fabricate(:user) }
|
||||||
fab!(:user_badge_tl1) do
|
fab!(:user_badge_tl1) do
|
||||||
UserBadge.create!(
|
UserBadge.create!(
|
||||||
badge_id: Badge::BasicUser,
|
badge_id: Badge::BasicUser,
|
||||||
@ -82,6 +83,15 @@ RSpec.describe UserBadge do
|
|||||||
)
|
)
|
||||||
end
|
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
|
it "gives user badges the correct rank" do
|
||||||
expect(user_badge_tl2.reload.featured_rank).to eq(1)
|
expect(user_badge_tl2.reload.featured_rank).to eq(1)
|
||||||
expect(user_badge_wiki.reload.featured_rank).to eq(2)
|
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
|
it "can ensure consistency per user" do
|
||||||
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
|
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
|
||||||
expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check
|
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])
|
UserBadge.update_featured_ranks!([user.id])
|
||||||
|
|
||||||
expect(user_badge_tl2.reload.featured_rank).to eq(1)
|
expect(user_badge_tl2.reload.featured_rank).to eq(1)
|
||||||
|
expect(user_2_badge_tl1.reload.featured_rank).to eq(20)
|
||||||
end
|
end
|
||||||
|
|
||||||
it "can ensure consistency for all users" do
|
it "can ensure consistency for all users" do
|
||||||
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
|
user_badge_tl2.update_column(:featured_rank, 20) # Update without hooks
|
||||||
expect(user_badge_tl2.reload.featured_rank).to eq(20) # Double check
|
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_badge_tl2.reload.featured_rank).to eq(1)
|
||||||
|
expect(user_2_badge_tl1.reload.featured_rank).to eq(1)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -215,7 +215,7 @@ RSpec.describe UserStat do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "update_distinct_badge_count" do
|
describe ".update_distinct_badge_count" do
|
||||||
fab!(:user)
|
fab!(:user)
|
||||||
let(:stat) { user.user_stat }
|
let(:stat) { user.user_stat }
|
||||||
fab!(:badge1) { Fabricate(:badge) }
|
fab!(:badge1) { Fabricate(:badge) }
|
||||||
|
Reference in New Issue
Block a user