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.
This commit is contained in:
Akihiko Odaki (@fn_aki@pawoo.net) 2017-06-21 03:41:23 +09:00 committed by Eugen Rochko
parent a20cf3b64e
commit bab5a18232
3 changed files with 32 additions and 12 deletions

View file

@ -131,7 +131,15 @@ class Status < ApplicationRecord
end end
def as_home_timeline(account) 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 end
def as_public_timeline(account = nil, local_only = false) def as_public_timeline(account = nil, local_only = false)

View file

@ -23,11 +23,7 @@ class PrecomputeFeedService < BaseService
end end
def process_status(status) def process_status(status)
add_status_to_feed(status) unless skip_status?(status) add_status_to_feed(status) unless status_filtered?(status)
end
def skip_status?(status)
status.direct_visibility? || status_filtered?(status)
end end
def add_status_to_feed(status) def add_status_to_feed(status)

View file

@ -181,15 +181,18 @@ RSpec.describe Status, type: :model do
end end
describe '.as_home_timeline' do describe '.as_home_timeline' do
let(:account) { Fabricate(:account) }
let(:followed) { Fabricate(:account) }
let(:not_followed) { Fabricate(:account) }
before do before do
account = Fabricate(:account)
followed = Fabricate(:account)
not_followed = Fabricate(:account)
Fabricate(:follow, account: account, target_account: followed) Fabricate(:follow, account: account, target_account: followed)
@self_status = Fabricate(:status, account: account) @self_status = Fabricate(:status, account: account, visibility: :public)
@followed_status = Fabricate(:status, account: followed) @self_direct_status = Fabricate(:status, account: account, visibility: :direct)
@not_followed_status = Fabricate(:status, account: not_followed) @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) @results = Status.as_home_timeline(account)
end end
@ -198,10 +201,23 @@ RSpec.describe Status, type: :model do
expect(@results).to include(@self_status) expect(@results).to include(@self_status)
end end
it 'includes direct statuses from self' do
expect(@results).to include(@self_direct_status)
end
it 'includes statuses from followed' do it 'includes statuses from followed' do
expect(@results).to include(@followed_status) expect(@results).to include(@followed_status)
end 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 it 'does not include statuses from non-followed' do
expect(@results).not_to include(@not_followed_status) expect(@results).not_to include(@not_followed_status)
end end