From af1b9ede9d94e716f57aa25dfc24bade5735a46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 10 Feb 2026 16:31:32 +0100 Subject: [PATCH] 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 --- app/services/badge_granter.rb | 8 +++-- spec/services/badge_granter_spec.rb | 45 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb index a36cd15ab9b..b82adf9ed37 100644 --- a/app/services/badge_granter.rb +++ b/app/services/badge_granter.rb @@ -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 diff --git a/spec/services/badge_granter_spec.rb b/spec/services/badge_granter_spec.rb index 50bca1a4fe7..2fed08a00ce 100644 --- a/spec/services/badge_granter_spec.rb +++ b/spec/services/badge_granter_spec.rb @@ -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) }