diff --git a/lib/mastodon/cli/statuses.rb b/lib/mastodon/cli/statuses.rb index 74b37db3ab0..f101e3d1d8c 100644 --- a/lib/mastodon/cli/statuses.rb +++ b/lib/mastodon/cli/statuses.rb @@ -35,7 +35,14 @@ module Mastodon::CLI vacuum_and_analyze_conversations end - KEEP_STATUSES_WITH_LOCAL_REPLIES = <<~SQL.squish + # 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 ( @@ -45,7 +52,12 @@ module Mastodon::CLI 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) + 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 @@ -73,7 +85,7 @@ module Mastodon::CLI 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) #{keep_direct_replies_sql} - #{KEEP_STATUSES_WITH_LOCAL_REPLIES} + #{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) diff --git a/spec/lib/mastodon/cli/statuses_spec.rb b/spec/lib/mastodon/cli/statuses_spec.rb index cb7b332345c..cb6a78397f6 100644 --- a/spec/lib/mastodon/cli/statuses_spec.rb +++ b/spec/lib/mastodon/cli/statuses_spec.rb @@ -32,31 +32,61 @@ RSpec.describe Mastodon::CLI::Statuses do end end - describe '#KEEP_STATUSES_WITH_LOCAL_REPLIES', :use_transactional_tests do - let(:action) { :remove } + 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 - context 'with threaded replies' do - let!(:acct_remote) { Fabricate(:account, domain: 'example.com') } - let!(:acct_local) { Fabricate(:account) } + ActiveRecord::Base.connection.exec_query(query).to_ary.pluck('id') + end - let!(:root) { Fabricate(:status, account: acct_remote) } - let!(:reply_remote) { Fabricate(:status, account: acct_remote, in_reply_to_id: root.id) } - let!(:reply_local) { Fabricate(:status, account: acct_local, in_reply_to_id: reply_remote.id) } - let!(:reply_leaf) { Fabricate(:status, account: acct_remote, in_reply_to_id: reply_local.id) } - let!(:reply_unrelated) { Fabricate(:status, account: acct_remote, in_reply_to_id: root.id) } - let!(:unrelated) { Fabricate(:status, account: acct_remote) } + let!(:acct_local) { Fabricate(:account) } + let!(:acct_remote) { Fabricate(:account, domain: 'example.com') } + let!(:acct_target) { Fabricate(:account, domain: 'example.com') } - it 'excludes statuses with local replies beneath them from pruning' do - subject = described_class::KEEP_STATUSES_WITH_LOCAL_REPLIES - query = <<~SQL.squish - SELECT statuses.id FROM statuses WHERE deleted_at IS NULL AND NOT local AND uri IS NOT NULL - #{subject} - SQL + 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) } - to_delete = ActiveRecord::Base.connection.exec_query(query).to_ary.pluck('id') - - expect(to_delete.sort).to eq([reply_unrelated.id, unrelated.id, reply_leaf.id].sort) + 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