From 0eaf6d7693ba1f5568c9b857a306e01250f2f714 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Nov 2018 20:48:50 +0100 Subject: [PATCH] Sort self-replies to the top of descendants (#9320) Fix #6463 --- .../concerns/status_threading_concern.rb | 26 +++++++++++++++++-- .../concerns/status_threading_concern_spec.rb | 10 +++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/status_threading_concern.rb b/app/models/concerns/status_threading_concern.rb index fa441469c..b9c800c2a 100644 --- a/app/models/concerns/status_threading_concern.rb +++ b/app/models/concerns/status_threading_concern.rb @@ -8,7 +8,7 @@ module StatusThreadingConcern end def descendants(limit, account = nil, max_child_id = nil, since_child_id = nil, depth = nil) - find_statuses_from_tree_path(descendant_ids(limit, max_child_id, since_child_id, depth), account) + find_statuses_from_tree_path(descendant_ids(limit, max_child_id, since_child_id, depth), account, promote: true) end private @@ -76,7 +76,7 @@ module StatusThreadingConcern descendants_with_self - [self] end - def find_statuses_from_tree_path(ids, account) + def find_statuses_from_tree_path(ids, account, promote: false) statuses = statuses_with_accounts(ids).to_a account_ids = statuses.map(&:account_id).uniq domains = statuses.map(&:account_domain).compact.uniq @@ -86,6 +86,28 @@ module StatusThreadingConcern # Order ancestors/descendants by tree path statuses.sort_by! { |status| ids.index(status.id) } + + # Bring self-replies to the top + if promote + promote_by!(statuses) { |status| status.in_reply_to_account_id == status.account_id } + else + statuses + end + end + + def promote_by!(arr) + insert_at = arr.find_index { |item| !yield(item) } + + return arr if insert_at.nil? + + arr.each_with_index do |item, index| + next if index <= insert_at || !yield(item) + + arr.insert(insert_at, arr.delete_at(index)) + insert_at += 1 + end + + arr end def relations_map_for_account(account, account_ids, domains) diff --git a/spec/models/concerns/status_threading_concern_spec.rb b/spec/models/concerns/status_threading_concern_spec.rb index e5736a307..94c2d5fc2 100644 --- a/spec/models/concerns/status_threading_concern_spec.rb +++ b/spec/models/concerns/status_threading_concern_spec.rb @@ -118,5 +118,15 @@ describe StatusThreadingConcern do viewer.block_domain!('example.com') expect(status.descendants(4, viewer)).to_not include(reply2) end + + it 'promotes self-replies to the top while leaving the rest in order' do + a = Fabricate(:status, account: alice) + d = Fabricate(:status, account: jeff, thread: a) + e = Fabricate(:status, account: bob, thread: d) + c = Fabricate(:status, account: alice, thread: a) + f = Fabricate(:status, account: bob, thread: c) + + expect(a.descendants(20)).to eq [c, d, e, f] + end end end