Merge remote-tracking branch 'parent/main' into upstream-20231225

spec/services/account_statuses_cleanup_service_spec.rb

@@ -39,6 +39,13 @@ describe AccountStatusesCleanupService, type: :service do
         it 'actually deletes the statuses' do
           subject.call(account_policy, 10)
           expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])).to be_nil
+          expect { recent_status.reload }.to_not raise_error
+        end
+
+        it 'preserves recent and unrelated statuses' do
+          subject.call(account_policy, 10)
+          expect { unrelated_status.reload }.to_not raise_error
+          expect { recent_status.reload }.to_not raise_error
         end
       end
 

spec/services/activitypub/fetch_featured_collection_service_spec.rb

@@ -87,6 +87,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do
         'https://example.com/account/pinned/unknown-inlined',
         'https://example.com/account/pinned/unknown-reachable'
       )
+      expect(actor.pinned_statuses).to_not include(known_status)
     end
   end
 

spec/services/activitypub/fetch_remote_status_service_spec.rb

@@ -8,7 +8,6 @@ RSpec.describe ActivityPub::FetchRemoteStatusService, type: :service do
   subject { described_class.new }
 
   let!(:sender) { Fabricate(:account, domain: 'foo.bar', uri: 'https://foo.bar') }
-  let!(:recipient) { Fabricate(:account) }
 
   let(:existing_status) { nil }
 

spec/services/activitypub/process_account_service_spec.rb

@@ -283,7 +283,7 @@ RSpec.describe ActivityPub::ProcessAccountService, type: :service do
   end
 
   context 'when account is not suspended' do
-    subject { described_class.new.call('alice', 'example.com', payload) }
+    subject { described_class.new.call(account.username, account.domain, payload) }
 
     let!(:account) { Fabricate(:account, username: 'alice', domain: 'example.com') }
 

spec/services/activitypub/process_collection_service_spec.rb

@@ -242,7 +242,8 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do
         it 'does not process forged payload' do
           allow(ActivityPub::Activity).to receive(:factory)
 
-          subject.call(json, forwarder)
+          expect { subject.call(json, forwarder) }
+            .to_not change(actor.reload.statuses, :count)
 
           expect(ActivityPub::Activity).to_not have_received(:factory).with(
             hash_including(

spec/services/batched_remove_status_service_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe BatchedRemoveStatusService, type: :service do
   let!(:jeff)   { Fabricate(:account) }
   let!(:hank)   { Fabricate(:account, username: 'hank', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') }
 
-  let(:status_alice_hello) { PostStatusService.new.call(alice, text: 'Hello @bob@example.com') }
+  let(:status_alice_hello) { PostStatusService.new.call(alice, text: "Hello @#{bob.pretty_acct}") }
   let(:status_alice_other) { PostStatusService.new.call(alice, text: 'Another status') }
 
   before do

spec/services/block_domain_service_spec.rb

@@ -10,7 +10,10 @@ RSpec.describe BlockDomainService, type: :service do
   let!(:bad_status_with_attachment) { Fabricate(:status, account: bad_account, text: 'Hahaha') }
   let!(:bad_attachment) { Fabricate(:media_attachment, account: bad_account, status: bad_status_with_attachment, file: attachment_fixture('attachment.jpg')) }
   let!(:already_banned_account) { Fabricate(:account, username: 'badguy', domain: 'evil.org', suspended: true, silenced: true) }
-  let!(:bad_friend) { Fabricate(:friend_domain, domain: 'evil.org', inbox_url: 'https://evil.org/inbox', active_state: :accepted, passive_state: :accepted) }
+
+  before do
+    Fabricate(:friend_domain, domain: 'evil.org', inbox_url: 'https://evil.org/inbox', active_state: :accepted, passive_state: :accepted)
+  end
 
   describe 'for a suspension' do
     before do
@@ -25,19 +28,19 @@ RSpec.describe BlockDomainService, type: :service do
     end
 
     it 'removes remote accounts from that domain' do
-      expect(Account.find_remote('badguy666', 'evil.org').suspended?).to be true
+      expect(bad_account.reload.suspended?).to be true
     end
 
     it 'records suspension date appropriately' do
-      expect(Account.find_remote('badguy666', 'evil.org').suspended_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at
+      expect(bad_account.reload.suspended_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at
     end
 
     it 'keeps already-banned accounts banned' do
-      expect(Account.find_remote('badguy', 'evil.org').suspended?).to be true
+      expect(already_banned_account.reload.suspended?).to be true
     end
 
     it 'does not overwrite suspension date of already-banned accounts' do
-      expect(Account.find_remote('badguy', 'evil.org').suspended_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at
+      expect(already_banned_account.reload.suspended_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at
     end
 
     it 'removes the remote accounts\'s statuses and media attachments' do
@@ -72,19 +75,19 @@ RSpec.describe BlockDomainService, type: :service do
     end
 
     it 'silences remote accounts from that domain' do
-      expect(Account.find_remote('badguy666', 'evil.org').silenced?).to be true
+      expect(bad_account.reload.silenced?).to be true
     end
 
     it 'records suspension date appropriately' do
-      expect(Account.find_remote('badguy666', 'evil.org').silenced_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at
+      expect(bad_account.reload.silenced_at).to eq DomainBlock.find_by(domain: 'evil.org').created_at
     end
 
     it 'keeps already-banned accounts banned' do
-      expect(Account.find_remote('badguy', 'evil.org').silenced?).to be true
+      expect(already_banned_account.reload.silenced?).to be true
     end
 
     it 'does not overwrite suspension date of already-banned accounts' do
-      expect(Account.find_remote('badguy', 'evil.org').silenced_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at
+      expect(already_banned_account.reload.silenced_at).to_not eq DomainBlock.find_by(domain: 'evil.org').created_at
     end
 
     it 'leaves the domains status and attachments, but clears media' do

spec/services/bulk_import_service_spec.rb

@@ -271,14 +271,15 @@ RSpec.describe BulkImportService do
       let(:import_type) { 'domain_blocking' }
       let(:overwrite)   { false }
 
-      let!(:rows) do
+      let(:rows) do
         [
           { 'domain' => 'blocked.com' },
           { 'domain' => 'to_block.com' },
-        ].map { |data| import.rows.create!(data: data) }
+        ]
       end
 
       before do
+        rows.each { |data| import.rows.create!(data: data) }
         account.block_domain!('alreadyblocked.com')
         account.block_domain!('blocked.com')
       end
@@ -298,14 +299,15 @@ RSpec.describe BulkImportService do
       let(:import_type) { 'domain_blocking' }
       let(:overwrite)   { true }
 
-      let!(:rows) do
+      let(:rows) do
         [
           { 'domain' => 'blocked.com' },
           { 'domain' => 'to_block.com' },
-        ].map { |data| import.rows.create!(data: data) }
+        ]
       end
 
       before do
+        rows.each { |data| import.rows.create!(data: data) }
         account.block_domain!('alreadyblocked.com')
         account.block_domain!('blocked.com')
       end

spec/services/delete_account_service_spec.rb

@@ -48,30 +48,14 @@ RSpec.describe DeleteAccountService, type: :service do
     let!(:account_note) { Fabricate(:account_note, account: account) }
 
     it 'deletes associated owned and target records and target notifications' do
-      expect { subject }
-        .to delete_associated_owned_records
-        .and delete_associated_target_records
-        .and delete_associated_target_notifications
+      subject
+
+      expect_deletion_of_associated_owned_records
+      expect_deletion_of_associated_target_records
+      expect_deletion_of_associated_target_notifications
     end
 
-    def delete_associated_owned_records
-      change do
-        [
-          account.statuses,
-          account.media_attachments,
-          account.notifications,
-          account.favourites,
-          account.emoji_reactions,
-          account.bookmarks,
-          account.active_relationships,
-          account.passive_relationships,
-          account.polls,
-          account.account_notes,
-        ].map(&:count)
-      end.from([3, 1, 1, 1, 1, 1, 2, 2, 1, 1]).to([0, 0, 0, 0, 0, 0, 0, 0, 0, 0])
-    end
-
-    it 'deletes associated owned record groups' do
+    it 'deletes associated owned record groups' do # rubocop:disable RSpec/MultipleExpectations
       expect { subject }.to change {
         [
           account.owned_lists,
@@ -80,62 +64,81 @@ RSpec.describe DeleteAccountService, type: :service do
           account.bookmark_categories,
         ].map(&:count)
       }.from([1, 1, 1, 1]).to([0, 0, 0, 0])
+      expect { list_target_account.reload }.to_not raise_error
       expect { bookmark_category_status.status.reload }.to_not raise_error
       expect { antenna_account.account.reload }.to_not raise_error
       expect { circle_account.account.reload }.to_not raise_error
+      expect { list.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { list_account.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect { antenna_account.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect { circle_account.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect { circle_status.reload }.to raise_error(ActiveRecord::RecordNotFound)
       expect { bookmark_category_status.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
-    def delete_associated_target_records
-      change(account_pins_for_account, :count).from(1).to(0)
+    def expect_deletion_of_associated_owned_records
+      expect { status.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { status_with_mention.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { mention.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { media_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { favourite.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { emoji_reaction.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { bookmark.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { active_relationship.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { passive_relationship.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { poll.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { poll_vote.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { account_note.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    def expect_deletion_of_associated_target_records
+      expect { endorsement.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
     def account_pins_for_account
       AccountPin.where(target_account: account)
     end
 
-    def delete_associated_target_notifications
-      change do
-        %w(
-          poll favourite emoji_reaction status mention follow
-        ).map { |type| Notification.where(type: type).count }
-      end.from([1, 1, 1, 1, 1, 1]).to([0, 0, 0, 0, 0, 0])
+    def expect_deletion_of_associated_target_notifications
+      expect { favourite_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { follow_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { mention_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { poll_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { status_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { emoji_reaction_notification.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 
   describe '#call on local account' do
     before do
-      stub_request(:post, 'https://alice.com/inbox').to_return(status: 201)
-      stub_request(:post, 'https://bob.com/inbox').to_return(status: 201)
+      stub_request(:post, remote_alice.inbox_url).to_return(status: 201)
+      stub_request(:post, remote_bob.inbox_url).to_return(status: 201)
     end
 
     let!(:remote_alice) { Fabricate(:account, inbox_url: 'https://alice.com/inbox', domain: 'alice.com', protocol: :activitypub) }
     let!(:remote_bob) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', domain: 'bob.com', protocol: :activitypub) }
 
     include_examples 'common behavior' do
-      let!(:account) { Fabricate(:account) }
-      let!(:local_follower) { Fabricate(:account) }
+      let(:account) { Fabricate(:account) }
+      let(:local_follower) { Fabricate(:account) }
 
       it 'sends a delete actor activity to all known inboxes' do
         subject
-        expect(a_request(:post, 'https://alice.com/inbox')).to have_been_made.once
-        expect(a_request(:post, 'https://bob.com/inbox')).to have_been_made.once
+        expect(a_request(:post, remote_alice.inbox_url)).to have_been_made.once
+        expect(a_request(:post, remote_bob.inbox_url)).to have_been_made.once
       end
     end
   end
 
   describe '#call on remote account' do
     before do
-      stub_request(:post, 'https://alice.com/inbox').to_return(status: 201)
-      stub_request(:post, 'https://bob.com/inbox').to_return(status: 201)
+      stub_request(:post, account.inbox_url).to_return(status: 201)
     end
 
     include_examples 'common behavior' do
-      let!(:account) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub, domain: 'bob.com') }
-      let!(:local_follower) { Fabricate(:account) }
+      let(:account) { Fabricate(:account, inbox_url: 'https://bob.com/inbox', protocol: :activitypub, domain: 'bob.com') }
+      let(:local_follower) { Fabricate(:account) }
 
       it 'sends expected activities to followed and follower inboxes' do
         subject

spec/services/import_service_spec.rb

@@ -190,7 +190,7 @@ RSpec.describe ImportService, type: :service do
 
     # Make sure to not actually go to the remote server
     before do
-      stub_request(:post, 'https://թութ.հայ/inbox').to_return(status: 200)
+      stub_request(:post, nare.inbox_url).to_return(status: 200)
     end
 
     it 'follows the listed account' do

spec/services/notify_service_spec.rb

@@ -67,9 +67,10 @@ RSpec.describe NotifyService, type: :service do
 
       context 'when the message chain is initiated by recipient, but is not direct message' do
         let(:reply_to) { Fabricate(:status, account: recipient) }
-        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
         let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
 
+        before { Fabricate(:mention, account: sender, status: reply_to) }
+
         it 'does not notify' do
           expect { subject }.to_not change(Notification, :count)
         end
@@ -77,10 +78,11 @@ RSpec.describe NotifyService, type: :service do
 
       context 'when the message chain is initiated by recipient, but without a mention to the sender, even if the sender sends multiple messages in a row' do
         let(:reply_to) { Fabricate(:status, account: recipient) }
-        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
         let(:dummy_reply) { Fabricate(:status, account: sender, visibility: :direct, thread: reply_to) }
         let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: dummy_reply)) }
 
+        before { Fabricate(:mention, account: sender, status: reply_to) }
+
         it 'does not notify' do
           expect { subject }.to_not change(Notification, :count)
         end
@@ -88,9 +90,10 @@ RSpec.describe NotifyService, type: :service do
 
       context 'when the message chain is initiated by the recipient with a mention to the sender' do
         let(:reply_to) { Fabricate(:status, account: recipient, visibility: :direct) }
-        let!(:mention) { Fabricate(:mention, account: sender, status: reply_to) }
         let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) }
 
+        before { Fabricate(:mention, account: sender, status: reply_to) }
+
         it 'does notify' do
           expect { subject }.to change(Notification, :count)
         end

spec/services/remove_status_service_spec.rb

@@ -12,17 +12,17 @@ RSpec.describe RemoveStatusService, type: :service do
   let!(:bill)   { Fabricate(:account, username: 'bill', protocol: :activitypub, domain: 'example2.com', shared_inbox_url: 'http://example2.com/inbox', inbox_url: 'http://example2.com/bill/inbox') }
 
   before do
-    stub_request(:post, 'http://example.com/inbox').to_return(status: 200)
-    stub_request(:post, 'http://example.com/hank/inbox').to_return(status: 200)
-    stub_request(:post, 'http://example2.com/inbox').to_return(status: 200)
-    stub_request(:post, 'http://example2.com/bill/inbox').to_return(status: 200)
+    stub_request(:post, hank.inbox_url).to_return(status: 200)
+    stub_request(:post, hank.shared_inbox_url).to_return(status: 200)
+    stub_request(:post, bill.inbox_url).to_return(status: 200)
+    stub_request(:post, bill.shared_inbox_url).to_return(status: 200)
 
     jeff.follow!(alice)
     hank.follow!(alice)
   end
 
   context 'when removed status is not a reblog' do
-    let!(:status) { PostStatusService.new.call(alice, text: 'Hello @bob@example.com ThisIsASecret') }
+    let!(:status) { PostStatusService.new.call(alice, text: "Hello @#{bob.pretty_acct} ThisIsASecret") }
 
     before do
       FavouriteService.new.call(jeff, status)
@@ -41,7 +41,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Delete activity to followers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example.com/inbox').with(
+      expect(a_request(:post, hank.shared_inbox_url).with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
@@ -55,7 +55,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Delete activity to rebloggers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example2.com/inbox').with(
+      expect(a_request(:post, bill.shared_inbox_url).with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
@@ -83,7 +83,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Delete activity to followers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example.com/inbox').with(
+      expect(a_request(:post, hank.shared_inbox_url).with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
@@ -106,7 +106,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Delete activity to conversation' do
       subject.call(status)
-      expect(a_request(:post, 'http://example2.com/bill/inbox').with(
+      expect(a_request(:post, bill.inbox_url).with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
@@ -120,8 +120,8 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'do not send Delete activity to followers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example.com/hank/inbox')).to_not have_been_made
-      expect(a_request(:post, 'http://example.com/inbox')).to_not have_been_made
+      expect(a_request(:post, hank.inbox_url)).to_not have_been_made
+      expect(a_request(:post, hank.shared_inbox_url)).to_not have_been_made
     end
   end
 
@@ -131,7 +131,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Undo activity to followers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example.com/inbox').with(
+      expect(a_request(:post, hank.shared_inbox_url).with(
                body: hash_including({
                  'type' => 'Undo',
                  'object' => hash_including({
@@ -149,7 +149,7 @@ RSpec.describe RemoveStatusService, type: :service do
 
     it 'sends Undo activity to followers' do
       subject.call(status)
-      expect(a_request(:post, 'http://example.com/inbox').with(
+      expect(a_request(:post, hank.shared_inbox_url).with(
                body: hash_including({
                  'type' => 'Undo',
                  'object' => hash_including({

spec/services/report_service_spec.rb

@@ -156,9 +156,8 @@ RSpec.describe ReportService, type: :service do
       -> {  described_class.new.call(source_account, target_account) }
     end
 
-    let!(:other_report) { Fabricate(:report, target_account: target_account) }
-
     before do
+      Fabricate(:report, target_account: target_account)
       ActionMailer::Base.deliveries.clear
       source_account.user.settings['notification_emails.report'] = true
       source_account.user.save

spec/services/resolve_account_service_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe ResolveAccountService, type: :service do
       let!(:remote_account) { Fabricate(:account, username: 'foo', domain: 'ap.example.com', protocol: 'activitypub') }
 
       context 'when domain is banned' do
-        let!(:domain_block) { Fabricate(:domain_block, domain: 'ap.example.com', severity: :suspend) }
+        before { Fabricate(:domain_block, domain: 'ap.example.com', severity: :suspend) }
 
         it 'does not return an account' do
           expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to be_nil
@@ -218,6 +218,7 @@ RSpec.describe ResolveAccountService, type: :service do
       expect(account.domain).to eq 'ap.example.com'
       expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox'
       expect(account.uri).to eq 'https://ap.example.com/users/foo'
+      expect(status.reload.account).to eq(account)
     end
   end
 

spec/services/suspend_account_service_spec.rb

@@ -46,9 +46,9 @@ RSpec.describe SuspendAccountService, type: :service do
       let!(:account)         { Fabricate(:account) }
       let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub, domain: 'alice.com') }
       let!(:remote_reporter) { Fabricate(:account, uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub, domain: 'bob.com') }
-      let!(:report)          { Fabricate(:report, account: remote_reporter, target_account: account) }
 
       before do
+        Fabricate(:report, account: remote_reporter, target_account: account)
         remote_follower.follow!(account)
       end
 

spec/services/unallow_domain_service_spec.rb

@@ -27,6 +27,7 @@ RSpec.describe UnallowDomainService, type: :service do
       end
 
       it 'removes remote accounts from that domain' do
+        expect { already_banned_account.reload }.to raise_error(ActiveRecord::RecordNotFound)
         expect(Account.where(domain: 'evil.org').exists?).to be false
       end
 

spec/services/unsuspend_account_service_spec.rb

@@ -39,9 +39,9 @@ RSpec.describe UnsuspendAccountService, type: :service do
       let!(:account)         { Fabricate(:account) }
       let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub, domain: 'alice.com') }
       let!(:remote_reporter) { Fabricate(:account, uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub, domain: 'bob.com') }
-      let!(:report)          { Fabricate(:report, account: remote_reporter, target_account: account) }
 
       before do
+        Fabricate(:report, account: remote_reporter, target_account: account)
         remote_follower.follow!(account)
       end