Refactoring Badges::AwardThumbsUp (#20360)

* Optimized Badges::AwardThumbsUp

* Reorganized Badges::AwardThumbsUp spec

app/services/badges/award_thumbs_up.rb

@@ -24,9 +24,9 @@ module Badges
 
           badge_id = badge_ids[threshold]
           next unless badge_id
-          next unless (user = User.find_by(id: user_id))
 
-          user.badge_achievements.create(
+          BadgeAchievement.create(
+            user_id: user_id,
             badge_id: badge_id,
             rewarding_context_message_markdown: generate_message(threshold: threshold),
           )
@@ -43,8 +43,8 @@ module Badges
     end
 
     def self.fetch_badge_ids
-      THUMBS_UP_BADGES.transform_values do |title|
-        Badge.find_by(title: title)&.id
+      Badge.where(title: THUMBS_UP_BADGES.values).each_with_object({}) do |badge, hash|
+        hash[THUMBS_UP_BADGES.key(badge.title)] = badge[:id]
       end
     end
 

spec/services/badges/award_thumbs_up_spec.rb

@@ -2,27 +2,39 @@ require "rails_helper"
 
 RSpec.describe Badges::AwardThumbsUp, type: :service do
   let(:thumbs_up_tiers) { described_class::THUMBS_UP_BADGES.keys }
+  let(:user) { create(:user) }
 
   it "does nothing if there are no thumbsup badges" do
-    user = create(:user)
     allow(described_class).to receive(:get_user_thumbsup_counts).and_return(user.id => thumbs_up_tiers.max)
 
     expect { described_class.call }.not_to change { user.badges.count }
   end
 
-  it "awards the correct badge to the correct user" do
-    all_badges = described_class::THUMBS_UP_BADGES.map do |_, badge_title|
-      create(:badge, title: badge_title)
+  context "when there are thumbsup badges" do
+    let!(:all_badges) do
+      described_class::THUMBS_UP_BADGES.map do |_, badge_title|
+        create(:badge, title: badge_title)
+      end
     end
-    ghost_user = create(:user)
-    user = create(:user)
-    prolific_user = create(:user)
+    let(:min_badge) { Badge.find_by(title: described_class::THUMBS_UP_BADGES[thumbs_up_tiers.min]) }
+    let(:ghost_user) { create(:user) }
+    let(:prolific_user) { create(:user) }
 
-    allow(described_class).to receive(:get_user_thumbsup_counts)
-      .and_return(user.id => thumbs_up_tiers.min, prolific_user.id => thumbs_up_tiers.max)
-    described_class.call
-    expect(ghost_user.badges.count).to eq(0)
-    expect(user.badges).to eq([Badge.find_by(title: described_class::THUMBS_UP_BADGES[thumbs_up_tiers.min])])
-    expect(prolific_user.badges).to eq(all_badges)
+    it "awards the correct badge to the correct user" do
+      allow(described_class).to receive(:get_user_thumbsup_counts)
+        .and_return(user.id => thumbs_up_tiers.min, prolific_user.id => thumbs_up_tiers.max)
+      described_class.call
+      expect(ghost_user.badges.count).to eq(0)
+      expect(user.badges).to eq([min_badge])
+      expect(prolific_user.badges).to eq(all_badges)
+    end
+
+    it "doesn't award badges second time" do
+      create(:badge_achievement, user: user, badge: min_badge)
+      allow(described_class).to receive(:get_user_thumbsup_counts).and_return(user.id => thumbs_up_tiers.min + 1)
+      expect do
+        described_class.call
+      end.not_to change(user.badge_achievements, :count)
+    end
   end
 end