From 6f490b4bfed5fba9bd543a4c99b5694f37cd1f99 Mon Sep 17 00:00:00 2001
From: unarist <m.unarist@gmail.com>
Date: Mon, 16 Oct 2017 22:58:23 +0900
Subject: [PATCH] Fix un-reblogged status being at wrong position in the home
timeline (#5418)
We've changed un-reblogging behavior when we implement Snowflake, to insert un-reblogged status at the position reblogging status existed.
However, our API expects home timeline is ordered by status ids, and max_id/since_id filters by zset score. Due to this, un-reblogged status appears as a last item of result set, and timeline expansion may skips many statuses.
So this reverts that change...reblogged status inserted at corresponding position to its id.
---
app/lib/feed_manager.rb | 9 +++++----
spec/lib/feed_manager_spec.rb | 9 ++++++---
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb
index 89aeaadcd..dfd11a23b 100644
--- a/app/lib/feed_manager.rb
+++ b/app/lib/feed_manager.rb
@@ -198,10 +198,11 @@ class FeedManager
# 2. Remove the reblogged status from the `:reblogs` zset.
redis.zrem(reblog_key, status.reblog_of_id)
- # 3. Add the reblogged status to the feed using the reblogging
- # status' ID as its score, and the reblogged status' ID as its
- # value.
- redis.zadd(timeline_key, status.id, status.reblog_of_id)
+ # 3. Add the reblogged status to the feed.
+ # Note that we can't use old score in here
+ # and it must be an ID of corresponding status
+ # because we need to filter timeline by status ID.
+ redis.zadd(timeline_key, status.reblog_of_id, status.reblog_of_id)
# 4. Remove the reblogging status from the feed (as normal)
end
diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb
index 923894ccb..643f1f003 100644
--- a/spec/lib/feed_manager_spec.rb
+++ b/spec/lib/feed_manager_spec.rb
@@ -233,19 +233,22 @@ RSpec.describe FeedManager do
describe '#unpush' do
it 'leaves a reblogged status when deleting the reblog' do
account = Fabricate(:account)
- reblogged = Fabricate(:status)
+ reblogged = Fabricate(:status, id: Mastodon::Snowflake.id_at(2.day.ago.utc))
+ other_status = Fabricate(:status, id: Mastodon::Snowflake.id_at(1.day.ago.utc))
status = Fabricate(:status, reblog: reblogged)
+ FeedManager.instance.push('type', account, other_status)
FeedManager.instance.push('type', account, status)
# The reblogging status should show up under normal conditions.
- expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [status.id.to_s]
+ expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [other_status.id.to_s, status.id.to_s]
FeedManager.instance.unpush('type', account, status)
# Because we couldn't tell if the status showed up any other way,
# we had to stick the reblogged status in by itself.
- expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [reblogged.id.to_s]
+ # And it must be ordered by status ids.
+ expect(Redis.current.zrange("feed:type:#{account.id}", 0, -1)).to eq [reblogged.id.to_s, other_status.id.to_s]
end
it 'sends push updates' do
--
GitLab