Merge pull request NeuromatchAcademy#85 from NeuromatchAcademy/fix-hide-boosts-in-public-feeds

hide boosts from boost-muted accounts in public feeds

Author: sneakers-the-rat

app/models/account.rb

@@ -380,6 +380,17 @@ def excluded_from_timeline_account_ids
     Rails.cache.fetch("exclude_account_ids_for:#{id}") { block_relationships.pluck(:target_account_id) + blocked_by_relationships.pluck(:account_id) + mute_relationships.pluck(:target_account_id) }
   end
 
+  # Accounts that are followed but have reblogs hidden
+  def excluded_boosts_from_timeline_account_ids
+    Rails.cache.fetch("exclude_boosts_account_ids_for:#{id}", expires_in: 72.hours) do
+      active_relationships.where(show_reblogs: false).pluck(:target_account_id)
+    end
+  end
+
+  def invalidate_mute_boosts_cache
+    Rails.cache.delete("exclude_boosts_account_ids_for:#{id}")
+  end
+
   def excluded_from_timeline_domains
     Rails.cache.fetch("exclude_domains_for:#{id}") { domain_blocks.pluck(:domain) }
   end

app/models/concerns/account/interactions.rb

@@ -55,6 +55,7 @@ def follow!(other_account, reblogs: nil, notify: nil, languages: nil, uri: nil,
     rel.languages    = languages unless languages.nil?
 
     rel.save! if rel.changed?
+    try(:invalidate_mute_boosts_cache)
 
     rel
   end
@@ -68,6 +69,7 @@ def request_follow!(other_account, reblogs: nil, notify: nil, languages: nil, ur
     rel.languages    = languages unless languages.nil?
 
     rel.save! if rel.changed?
+    try(:invalidate_mute_boosts_cache)
 
     rel
   end
@@ -100,6 +102,9 @@ def block_domain!(other_domain)
   def unfollow!(other_account)
     follow = active_relationships.find_by(target_account: other_account)
     follow&.destroy
+    try(:invalidate_mute_boosts_cache)
+
+    follow
   end
 
   def unblock!(other_account)

app/models/status.rb

@@ -127,7 +127,10 @@ class Status < ApplicationRecord
   scope :not_replying_to_account, ->(account) { where.not(in_reply_to_account: account) }
   scope :without_reblogs, -> { where(statuses: { reblog_of_id: nil }) }
   scope :tagged_with, ->(tag_ids) { joins(:statuses_tags).where(statuses_tags: { tag_id: tag_ids }) }
-  scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) }
+  scope :not_excluded_by_account, lambda { |account|
+    where.not(account_id: account.excluded_from_timeline_account_ids)
+         .and(where(reblog_of_id: nil).or(where.not(account_id: account.excluded_boosts_from_timeline_account_ids)))
+  }
   scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).merge(Account.not_domain_blocked_by_account(account)) }
   scope :tagged_with_all, lambda { |tag_ids|
     Array(tag_ids).map(&:to_i).reduce(self) do |result, id|

spec/models/concerns/account/interactions_spec.rb

@@ -734,6 +734,10 @@
       it 'does mute reblogs' do
         expect(me.muting_reblogs?(you)).to be(true)
       end
+
+      it 'adds me to the collection of accounts that are boost muted' do
+        expect(me.excluded_boosts_from_timeline_account_ids).to include(you.id)
+      end
     end
 
     context 'with the reblogs option set to true' do
@@ -745,6 +749,20 @@
         expect(me.muting_reblogs?(you)).to be(false)
       end
     end
+
+    context 'when toggling the reblogs option' do
+      before do
+        me.follow!(you)
+      end
+
+      it 'evicts the cache of boost muted accounts' do
+        expect(me.excluded_boosts_from_timeline_account_ids).to be_empty
+        me.follow!(you, reblogs: false)
+        expect(me.excluded_boosts_from_timeline_account_ids).to include(you.id)
+        me.follow!(you, reblogs: true)
+        expect(me.excluded_boosts_from_timeline_account_ids).to be_empty
+      end
+    end
   end
 
   describe '#lists_for_local_distribution' do

spec/models/public_feed_spec.rb

@@ -664,5 +664,59 @@
         end
       end
     end
+
+    context 'with with_reblogs true' do
+      subject { described_class.new(viewer, with_reblogs: true).get(20).map(&:id) }
+
+      let!(:viewer)  { Fabricate(:account, domain: nil) }
+      let!(:booster) { Fabricate(:account, domain: nil) }
+      let!(:boosted) { Fabricate(:account, domain: nil) }
+      let!(:status) { Fabricate(:status, account: boosted) }
+      let!(:boost) { Fabricate(:status, account: booster, reblog_of_id: status.id) }
+
+      shared_examples 'shows boost' do
+        it 'shows boosts in public feeds' do
+          expect(subject).to include(boost.id)
+        end
+      end
+
+      shared_examples 'does not show boost' do
+        it 'does not show boosts in public feeds' do
+          expect(subject).to_not include(boost.id)
+        end
+      end
+
+      it_behaves_like 'shows boost'
+
+      context 'with a local_only option set' do
+        subject { described_class.new(viewer, with_reblogs: true, local: true).get(20).map(&:id) }
+
+        context 'with a remote booster' do
+          let!(:booster) { Fabricate(:account, domain: 'other.net') }
+
+          it_behaves_like 'does not show boost'
+        end
+
+        context 'with a remote status' do
+          let!(:boosted) { Fabricate(:account, domain: 'other.net') }
+
+          it_behaves_like 'shows boost'
+
+          context 'with a remote booster' do
+            let!(:booster) { Fabricate(:account, domain: 'other.net') }
+
+            it_behaves_like 'does not show boost'
+          end
+        end
+
+        context 'when hiding boosts from a followed account' do
+          before do
+            Fabricate(:follow, account: viewer, target_account: booster, show_reblogs: false)
+          end
+
+          it_behaves_like 'does not show boost'
+        end
+      end
+    end
   end
 end