Skip to content

Commit

Permalink
Filter boosts also filters for local or remote posts as appropriate (…
Browse files Browse the repository at this point in the history
…non-final commit to run tests)
  • Loading branch information
sneakers-the-rat committed Jan 30, 2024
1 parent 4ffdfa3 commit 4c21464
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 15 deletions.
33 changes: 25 additions & 8 deletions app/models/public_feed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions spec/models/public_feed_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit 4c21464

Please sign in to comment.