mirror of
https://github.com/discourse/discourse.git
synced 2026-08-02 09:29:37 -05:00
FIX: improve badge granter resilience (#37674)
Two issues were found in BadgeGranter that could prevent badges from being granted. First, `Array#compact!` returns `nil` when no elements are removed. For PostAction triggers, `[post_id, related_post_id].compact!` returns `nil` when `related_post_id` is present (nothing to compact), causing the post_ids payload to be silently lost. Using `compact` instead always returns the array. Second, `process_queue!` had no error isolation between badges. A single badge with a broken SQL query would raise an exception and abort processing of all remaining badges in the queue. Since queue items are already popped from Redis at that point, they are lost. Each badge is now rescued individually so one failure doesn't block the rest. https://meta.discourse.org/t/394444
This commit is contained in:
@@ -222,7 +222,7 @@ class BadgeGranter
|
||||
payload = { type: "TrustLevelChange", user_ids: [user.id] }
|
||||
when Badge::Trigger::PostAction
|
||||
action = opt[:post_action]
|
||||
payload = { type: "PostAction", post_ids: [action.post_id, action.related_post_id].compact! }
|
||||
payload = { type: "PostAction", post_ids: [action.post_id, action.related_post_id].compact }
|
||||
end
|
||||
|
||||
Discourse.redis.lpush queue_key, payload.to_json if payload
|
||||
@@ -248,7 +248,11 @@ class BadgeGranter
|
||||
|
||||
next if post_ids.blank? && user_ids.blank?
|
||||
|
||||
find_by_type(type).each { |badge| backfill(badge, post_ids: post_ids, user_ids: user_ids) }
|
||||
find_by_type(type).each do |badge|
|
||||
backfill(badge, post_ids:, user_ids:)
|
||||
rescue => e
|
||||
Rails.logger.warn("Failed to backfill '#{badge.name}' badge: #{e.message}")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -572,6 +572,51 @@ RSpec.describe BadgeGranter do
|
||||
end
|
||||
end
|
||||
|
||||
describe ".queue_badge_grant" do
|
||||
it "includes both post_id and related_post_id for PostAction triggers" do
|
||||
post_action = Struct.new(:post_id, :related_post_id).new(1, 2)
|
||||
|
||||
BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action:)
|
||||
raw = Discourse.redis.lpop(BadgeGranter.queue_key)
|
||||
parsed = JSON.parse(raw)
|
||||
|
||||
expect(parsed["post_ids"]).to contain_exactly(1, 2)
|
||||
end
|
||||
end
|
||||
|
||||
describe ".process_queue!" do
|
||||
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
|
||||
|
||||
it "continues processing other badges when one badge fails to backfill" do
|
||||
query = <<~SQL
|
||||
SELECT p.user_id, p.id post_id, p.updated_at granted_at
|
||||
FROM badge_posts p
|
||||
WHERE p.raw LIKE '%Give Me A Badge%'
|
||||
AND (:backfill OR p.id IN (:post_ids))
|
||||
SQL
|
||||
|
||||
broken_badge =
|
||||
Fabricate(
|
||||
:badge,
|
||||
query: "broken",
|
||||
trigger: Badge::Trigger::PostRevision,
|
||||
target_posts: true,
|
||||
)
|
||||
good_badge =
|
||||
Fabricate(:badge, query:, trigger: Badge::Trigger::PostRevision, target_posts: true)
|
||||
|
||||
create_post(user:, raw: "Give Me A Badge")
|
||||
|
||||
allow(BadgeGranter).to receive(:backfill).and_wrap_original do |method, badge, opts|
|
||||
raise BadgeGranter::GrantError, "broken query" if badge.id == broken_badge.id
|
||||
method.call(badge, opts)
|
||||
end
|
||||
|
||||
expect { BadgeGranter.process_queue! }.not_to raise_error
|
||||
expect(UserBadge.exists?(user:, badge: good_badge)).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
describe "update_badges" do
|
||||
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
|
||||
fab!(:liker) { Fabricate(:user, refresh_auto_groups: true) }
|
||||
|
||||
Reference in New Issue
Block a user