Announcement reactions query spec improvement and refactor (mastodon#…

…28768)

app/models/announcement.rb

@@ -75,22 +75,41 @@ def emojis
   end
 
   def reactions(account = nil)
-    records = begin
-      scope = announcement_reactions.group(:announcement_id, :name, :custom_emoji_id).order(Arel.sql('MIN(created_at) ASC'))
-
-      if account.nil?
-        scope.select('name, custom_emoji_id, count(*) as count, false as me')
-      else
-        scope.select("name, custom_emoji_id, count(*) as count, exists(select 1 from announcement_reactions r where r.account_id = #{account.id} and r.announcement_id = announcement_reactions.announcement_id and r.name = announcement_reactions.name) as me")
+    grouped_ordered_announcement_reactions.select(
+      [:name, :custom_emoji_id, 'COUNT(*) as count'].tap do |values|
+        values << value_for_reaction_me_column(account)
       end
-    end.to_a
-
-    ActiveRecord::Associations::Preloader.new(records: records, associations: :custom_emoji).call
-    records
+    ).to_a.tap do |records|
+      ActiveRecord::Associations::Preloader.new(records: records, associations: :custom_emoji).call
+    end
   end
 
   private
 
+  def grouped_ordered_announcement_reactions
+    announcement_reactions
+      .group(:announcement_id, :name, :custom_emoji_id)
+      .order(
+        Arel.sql('MIN(created_at)').asc
+      )
+  end
+
+  def value_for_reaction_me_column(account)
+    if account.nil?
+      'FALSE AS me'
+    else
+      <<~SQL.squish
+        EXISTS(
+          SELECT 1
+          FROM announcement_reactions inner_reactions
+          WHERE inner_reactions.account_id = #{account.id}
+            AND inner_reactions.announcement_id = announcement_reactions.announcement_id
+            AND inner_reactions.name = announcement_reactions.name
+        ) AS me
+      SQL
+    end
+  end
+
   def set_published
     return unless scheduled_at.blank? || scheduled_at.past?
 

spec/models/announcement_spec.rb

@@ -115,26 +115,35 @@
 
   describe '#reactions' do
     context 'with announcement_reactions present' do
+      let(:account_reaction_emoji) { Fabricate :custom_emoji }
+      let(:other_reaction_emoji) { Fabricate :custom_emoji }
       let!(:account) { Fabricate(:account) }
       let!(:announcement) { Fabricate(:announcement) }
-      let!(:announcement_reaction) { Fabricate(:announcement_reaction, announcement: announcement, created_at: 10.days.ago) }
-      let!(:announcement_reaction_account) { Fabricate(:announcement_reaction, announcement: announcement, created_at: 5.days.ago, account: account) }
 
       before do
-        Fabricate(:announcement_reaction)
+        Fabricate(:announcement_reaction, announcement: announcement, created_at: 10.days.ago, name: other_reaction_emoji.shortcode)
+        Fabricate(:announcement_reaction, announcement: announcement, created_at: 5.days.ago, account: account, name: account_reaction_emoji.shortcode)
+        Fabricate(:announcement_reaction) # For some other announcement
       end
 
       it 'returns the announcement reactions for the announcement' do
         results = announcement.reactions
 
-        expect(results.first.name).to eq(announcement_reaction.name)
-        expect(results.last.name).to eq(announcement_reaction_account.name)
+        expect(results).to have_attributes(
+          size: eq(2),
+          first: have_attributes(name: other_reaction_emoji.shortcode, me: false),
+          last: have_attributes(name: account_reaction_emoji.shortcode, me: false)
+        )
       end
 
-      it 'returns the announcement reactions for the announcement limited to account' do
+      it 'returns the announcement reactions for the announcement with `me` set correctly' do
         results = announcement.reactions(account)
 
-        expect(results.first.name).to eq(announcement_reaction.name)
+        expect(results).to have_attributes(
+          size: eq(2),
+          first: have_attributes(name: other_reaction_emoji.shortcode, me: false),
+          last: have_attributes(name: account_reaction_emoji.shortcode, me: true)
+        )
       end
     end
   end