From bab5a18232a163b0c3c6a245f7f95d50d7022b36 Mon Sep 17 00:00:00 2001 From: "Akihiko Odaki (@fn_aki@pawoo.net)" Date: Wed, 21 Jun 2017 03:41:23 +0900 Subject: [PATCH] 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. --- app/models/status.rb | 10 ++++++++- app/services/precompute_feed_service.rb | 6 +----- spec/models/status_spec.rb | 28 +++++++++++++++++++------ 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 24d3db2bf4c..544d2e005d5 100644 --- 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 626ec2f6c2e..83765bb0570 100644 --- 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 dd52a5d4389..a3ced1abc48 100644 --- 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