Merge fb4193469c into 20c105c194

lib/mastodon/cli/statuses.rb

@@ -35,6 +35,32 @@ module Mastodon::CLI
       vacuum_and_analyze_conversations
     end
 
+    # Preserve thread parents when there is some local interaction in the tree of replies beneath to avoid breaking threads,
+    # specifically the interactions that imply that the user intends to view the status again,
+    # or another account viewing the interaction would need to see thread parents as context:
+    # - local replies
+    # - local bookmarks
+    # - local reblogs
+    # - local quotes
+    KEEP_THREAD_PARENTS_WITH_LOCAL_INTERACTIONS = <<~SQL.squish
+      AND NOT EXISTS (
+        with RECURSIVE thread_cte as
+        (
+          SELECT id, in_reply_to_id, uri, local from statuses parent WHERE id = statuses.id
+          UNION ALL
+          SELECT child.id, child.in_reply_to_id, child.uri, child.local
+          FROM statuses child
+          JOIN thread_cte ON (child.in_reply_to_id = thread_cte.id)
+        )
+        SELECT 1 FROM thread_cte WHERE (
+          (thread_cte.uri IS NULL OR thread_cte.local)
+          OR EXISTS (SELECT 1 FROM bookmarks WHERE thread_cte.id = bookmarks.status_id AND bookmarks.account_id IN (SELECT accounts.id FROM accounts WHERE domain IS NULL))
+          OR EXISTS (SELECT 1 FROM statuses AS statuses1 WHERE thread_cte.id = statuses1.reblog_of_id AND (statuses1.uri IS NULL OR statuses1.local))
+          OR EXISTS (SELECT 1 FROM quotes JOIN statuses statuses1 ON quotes.status_id = statuses1.id WHERE quotes.quoted_status_id = thread_cte.id AND (statuses1.uri IS NULL OR statuses1.local))
+        )
+      )
+    SQL
+
     private
 
     def remove_statuses
@@ -52,10 +78,14 @@ module Mastodon::CLI
         # Skip accounts followed by local accounts
         clean_followed_sql = 'AND NOT EXISTS (SELECT 1 FROM follows WHERE statuses.account_id = follows.target_account_id)' unless options[:clean_followed]
 
+        # keep direct replies to local statuses
+        keep_direct_replies_sql = 'AND NOT EXISTS (SELECT 1 FROM statuses AS statuses1 WHERE statuses.in_reply_to_id = statuses1.id AND (statuses1.uri IS NULL OR statuses1.local))'
+
         ActiveRecord::Base.connection.exec_insert(<<~SQL.squish, 'SQL', [max_id])
           INSERT INTO statuses_to_be_deleted (id)
           SELECT statuses.id FROM statuses WHERE deleted_at IS NULL AND NOT local AND uri IS NOT NULL AND (id < $1)
-          AND NOT EXISTS (SELECT 1 FROM statuses AS statuses1 WHERE statuses.id = statuses1.in_reply_to_id)
+          #{keep_direct_replies_sql}
+          #{KEEP_THREAD_PARENTS_WITH_LOCAL_INTERACTIONS}
           AND NOT EXISTS (SELECT 1 FROM statuses AS statuses1 WHERE statuses1.id = statuses.reblog_of_id AND (statuses1.uri IS NULL OR statuses1.local))
           AND NOT EXISTS (SELECT 1 FROM statuses AS statuses1 WHERE statuses.id = statuses1.reblog_of_id AND (statuses1.uri IS NULL OR statuses1.local OR statuses1.id >= $1))
           AND NOT EXISTS (SELECT 1 FROM status_pins WHERE statuses.id = status_id)

spec/lib/mastodon/cli/statuses_spec.rb

@@ -31,4 +31,62 @@ RSpec.describe Mastodon::CLI::Statuses do
       end
     end
   end
+
+  describe '#KEEP_THREAD_PARENTS_WITH_LOCAL_INTERACTIONS', :use_transactional_tests do
+    subject do
+      query = <<~SQL.squish
+        SELECT statuses.id FROM statuses WHERE deleted_at IS NULL AND NOT local AND uri IS NOT NULL
+        #{described_class::KEEP_THREAD_PARENTS_WITH_LOCAL_INTERACTIONS}
+      SQL
+
+      ActiveRecord::Base.connection.exec_query(query).to_ary.pluck('id')
+    end
+
+    let!(:acct_local) { Fabricate(:account) }
+    let!(:acct_remote) { Fabricate(:account, domain: 'example.com') }
+    let!(:acct_target) { Fabricate(:account, domain: 'example.com') }
+
+    let!(:root) { Fabricate(:status, account: acct_remote) }
+    let!(:reply_remote) { Fabricate(:status, account: acct_remote, in_reply_to_id: root.id) }
+    let!(:reply_target) { Fabricate(:status, account: acct_target, in_reply_to_id: reply_remote.id) }
+    let!(:reply_leaf) { Fabricate(:status, account: acct_remote, in_reply_to_id: reply_target.id) }
+    let!(:reply_unrelated) { Fabricate(:status, account: acct_remote, in_reply_to_id: root.id) }
+    let!(:unrelated) { Fabricate(:status, account: acct_remote) }
+
+    shared_examples 'preserves thread parents' do
+      it 'excludes statuses with downthread interactions from pruning' do
+        expect(subject).to contain_exactly(reply_unrelated.id, unrelated.id, reply_leaf.id)
+      end
+    end
+
+    context 'without local interaction' do
+      it 'prunes the entire thread' do
+        expect(subject).to contain_exactly(root.id, reply_remote.id, reply_target.id, reply_leaf.id, reply_unrelated.id, unrelated.id)
+      end
+    end
+
+    context 'with a local reply' do
+      let!(:acct_target) { acct_local }
+
+      it_behaves_like 'preserves thread parents'
+    end
+
+    context 'with a local bookmark' do
+      before { Fabricate(:bookmark, account: acct_local, status: reply_target) }
+
+      it_behaves_like 'preserves thread parents'
+    end
+
+    context 'with a local reblog' do
+      before { Fabricate(:status, account: acct_local, reblog_of_id: reply_target.id) }
+
+      it_behaves_like 'preserves thread parents'
+    end
+
+    context 'with a local quote' do
+      before { Fabricate(:quote, status: Fabricate(:status, account: acct_local), quoted_status: reply_target, state: :accepted) }
+
+      it_behaves_like 'preserves thread parents'
+    end
+  end
 end