From 4c214646b3d1ecd38ce819ac80e94c14c353ef71 Mon Sep 17 00:00:00 2001
From: sneakers-the-rat <sneakers-the-rat@protonmail.com>
Date: Tue, 30 Jan 2024 03:55:16 -0800
Subject: [PATCH] Filter boosts also filters for local or remote posts as
 appropriate (non-final commit to run tests)

---
 app/models/public_feed.rb       | 33 +++++++++++++++++++++++++--------
 spec/models/public_feed_spec.rb | 21 ++++++++++++++-------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/app/models/public_feed.rb b/app/models/public_feed.rb
index 060a9ac24d480d..97d4b99dcfd7c4 100644
--- a/app/models/public_feed.rb
+++ b/app/models/public_feed.rb
@@ -25,13 +25,14 @@ def get(limit, max_id = nil, since_id = nil, min_id = nil)
     scope.merge!(without_local_only_scope) unless allow_local_only?
     scope.merge!(without_replies_scope) unless with_replies?
     scope.merge!(without_reblogs_scope) unless with_reblogs?
-    scope.merge!(without_duplicate_reblogs) if with_reblogs?
     scope.merge!(local_only_scope) if local_only?
     scope.merge!(remote_only_scope) if remote_only?
     scope.merge!(account_filters_scope) if account?
     scope.merge!(media_only_scope) if media_only?
     scope.merge!(language_scope) if account&.chosen_languages.present?
 
+    scope.merge!(without_duplicate_reblogs) if with_reblogs?
+
     scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
   end
 
@@ -91,15 +92,31 @@ def without_reblogs_scope
     Status.without_reblogs
   end
 
+  # Inner query used in `without_duplicate_reblogs_scope`
+  # selects only the most recent boost for a given post
+  # Also applies scopes which catch the most common cases
+  # for missing a boost, but doesn't duplicate the whole query.
+  # This is not DRY because we want to avoid fork-of-a-fork merge conflict hell
+  def max_boost_id_scope
+    # use Arel for correlated subquery
+    s_table = Status.arel_table
+    s_alias = s_table.alias
+
+    inner_select = Status.select(s_alias[:id].maximum)
+                         .from(s_alias)
+                         .where(s_alias[:reblog_of_id]
+                                  .eq(s_table[:reblog_of_id]))
+    inner_select.merge!(local_only_scope.from(s_alias)) if local_only?
+    inner_select.merge!(remote_only_scope.from(s_alias)) if remote_only?
+    inner_select.merge!(account_filters_scope.from(s_alias)) if account?
+    inner_select.unscope!(:order)
+
+    Status.where('"statuses"."id" = (:maxid)', maxid: inner_select)
+  end
+
   def without_duplicate_reblogs
     Status.where(statuses: { reblog_of_id: nil })
-          .or(Status.where(<<~SQL.squish))
-            "statuses"."id" = (
-              SELECT MAX(id)
-              FROM statuses s2
-              WHERE s2.reblog_of_id = statuses.reblog_of_id
-            )
-          SQL
+          .or(max_boost_id_scope)
   end
 
   def media_only_scope
diff --git a/spec/models/public_feed_spec.rb b/spec/models/public_feed_spec.rb
index ff9078b4891f84..6ed75d1bcf3c91 100644
--- a/spec/models/public_feed_spec.rb
+++ b/spec/models/public_feed_spec.rb
@@ -35,6 +35,11 @@
     context 'with with_reblogs option' do
       subject { described_class.new(nil, with_reblogs: true).get(20).map(&:id) }
 
+      let!(:poster)  { Fabricate(:account, domain: nil) }
+      let!(:booster) { Fabricate(:account, domain: nil) }
+      let!(:second_booster) { Fabricate(:account, domain: nil) }
+      let!(:remote_booster) { Fabricate(:account, domain: 'example.com') }
+
       it 'does include boosts' do
         status = Fabricate(:status)
         boost = Fabricate(:status, reblog_of_id: status.id)
@@ -44,10 +49,6 @@
       end
 
       it 'only includes the most recent boost' do
-        poster = Fabricate(:account, domain: nil)
-        booster = Fabricate(:account, domain: nil)
-        second_booster = Fabricate(:account, domain: nil)
-
         status = Fabricate(:status, account: poster)
         boost = Fabricate(:status, reblog_of_id: status.id, account: poster)
         second_boost = Fabricate(:status, reblog_of_id: status.id, account: booster)
@@ -60,9 +61,6 @@
       end
 
       it 'filters duplicate boosts across pagination' do
-        poster = Fabricate(:account, domain: nil)
-        booster = Fabricate(:account, domain: nil)
-
         status = Fabricate(:status, account: poster)
         # sleep for 2ms to make sure the other posts come in a greater snowflake ID
         sleep(0.002)
@@ -87,6 +85,15 @@
         expect(subject).to include(second_boost.id)
         expect(second_page).to_not include(boost.id)
       end
+
+      it 'shows the most recent local boost when there is a more recent remote boost' do
+        status = Fabricate(:status, account: poster)
+        local_boost = Fabricate(:status, reblog_of_id: status.id, local: true, account: booster)
+        remote_boost = Fabricate(:status, reblog_of_id: status.id, id: local_boost.id + 1, local: false, uri: 'https://example.com/boosturl', account: remote_booster)
+
+        expect(subject).to include(local_boost.id)
+        expect(subject).to_not include(remote_boost.id)
+      end
     end
 
     it 'filters out silenced accounts' do