Sort self-replies to the top of descendants (#9320)

Fix #6463

diff --git a/app/models/concerns/status_threading_concern.rb b/app/models/concerns/status_threading_concern.rb
index fa441469c11c80766bea1baf62313106e8ae2f01..b9c800c2aabdbb3a262dd9839ee6c6ede2f7a2a4 100644 (file)
--- 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 e5736a307909970a88918729b35e7a09c228fc9a..94c2d5fc2e84e95007f2e54acbabe76f2e316b92 100644 (file)
--- 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