Filter direct statuses in Status.as_home_timeline (#3842)

The classes using Status.as_home_timeline, namely Feed and
PrecomputeFeedService are expected to filter direct statuses as
FanOutWriteService does, but their filtering were incomplete or missing.

This commit solves the problem by filtering direct statuses in
as_home_timeline as the other similar methods such as as_public_timeline
does.

diff --git a/app/models/status.rb b/app/models/status.rb
index 24d3db2bf4c5ccf1b2cd24eb41ad2653ab693565..544d2e005d532cf36dcbb50745d4e2dbd79da640 100644 (file)
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -131,7 +131,15 @@ class Status < ApplicationRecord
     end
 
     def as_home_timeline(account)
-      where(account: [account] + account.following)
+      # 'references' is a workaround for the following issue:
+      # Inconsistent results with #or in ActiveRecord::Relation with respect to documentation Issue #24055 rails/rails
+      # https://github.com/rails/rails/issues/24055
+      references(:mentions)
+        .where.not(visibility: :direct)
+        .or(where(mentions: { account: account }))
+        .where(follows: { account_id: account })
+        .or(references(:mentions, :follows).where(account: account))
+        .left_outer_joins(account: :followers).left_outer_joins(:mentions).group(:id)
     end
 
     def as_public_timeline(account = nil, local_only = false)

diff --git a/app/services/precompute_feed_service.rb b/app/services/precompute_feed_service.rb
index 626ec2f6c2e4370f2f00b79fd56822880cf7b79b..83765bb05704b0a1183fef532b609a570851c2bb 100644 (file)
--- a/app/services/precompute_feed_service.rb
+++ b/app/services/precompute_feed_service.rb
@@ -23,11 +23,7 @@ class PrecomputeFeedService < BaseService
   end
 
   def process_status(status)
-    add_status_to_feed(status) unless skip_status?(status)
-  end
-
-  def skip_status?(status)
-    status.direct_visibility? || status_filtered?(status)
+    add_status_to_feed(status) unless status_filtered?(status)
   end
 
   def add_status_to_feed(status)

diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb
index dd52a5d4389d9a2aaeaf6fd42b32d507573c4b77..a3ced1abc48a1479649500bd7a6660b2ae8b14a1 100644 (file)
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@@ -181,15 +181,18 @@ RSpec.describe Status, type: :model do
   end
 
   describe '.as_home_timeline' do
+    let(:account) { Fabricate(:account) }
+    let(:followed) { Fabricate(:account) }
+    let(:not_followed) { Fabricate(:account) }
+
     before do
-      account = Fabricate(:account)
-      followed = Fabricate(:account)
-      not_followed = Fabricate(:account)
       Fabricate(:follow, account: account, target_account: followed)
 
-      @self_status = Fabricate(:status, account: account)
-      @followed_status = Fabricate(:status, account: followed)
-      @not_followed_status = Fabricate(:status, account: not_followed)
+      @self_status = Fabricate(:status, account: account, visibility: :public)
+      @self_direct_status = Fabricate(:status, account: account, visibility: :direct)
+      @followed_status = Fabricate(:status, account: followed, visibility: :public)
+      @followed_direct_status = Fabricate(:status, account: followed, visibility: :direct)
+      @not_followed_status = Fabricate(:status, account: not_followed, visibility: :public)
 
       @results = Status.as_home_timeline(account)
     end
@@ -198,10 +201,23 @@ RSpec.describe Status, type: :model do
       expect(@results).to include(@self_status)
     end
 
+    it 'includes direct statuses from self' do
+      expect(@results).to include(@self_direct_status)
+    end
+
     it 'includes statuses from followed' do
       expect(@results).to include(@followed_status)
     end
 
+    it 'includes direct statuses mentioning recipient from followed' do
+      Fabricate(:mention, account: account, status: @followed_direct_status)
+      expect(@results).to include(@followed_direct_status)
+    end
+
+    it 'does not include direct statuses not mentioning recipient from followed' do
+      expect(@results).not_to include(@followed_direct_status)
+    end
+
     it 'does not include statuses from non-followed' do
       expect(@results).not_to include(@not_followed_status)
     end