From bfd5aea20686559f030ca0f2538bebc1943b398b Mon Sep 17 00:00:00 2001 From: ThibG Date: Sat, 1 Aug 2020 18:20:37 +0200 Subject: [PATCH] Fix handling of Reject Follow when a matching follow relationship exists (#14479) * Add tests * Fix handling of Reject Follow when a matching follow relationship exists Regression from #12199 --- app/lib/activitypub/activity/reject.rb | 2 +- spec/lib/activitypub/activity/reject_spec.rb | 110 ++++++++++++++++--- 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index 8d771ed81..886dddb23 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -4,7 +4,7 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity def perform return reject_follow_for_relay if relay_follow? return follow_request_from_object.reject! unless follow_request_from_object.nil? - return UnfollowService.new.call(follow_from_object.target_account, @account) unless follow_from_object.nil? + return UnfollowService.new.call(follow_from_object.account, @account) unless follow_from_object.nil? case @object['type'] when 'Follow' diff --git a/spec/lib/activitypub/activity/reject_spec.rb b/spec/lib/activitypub/activity/reject_spec.rb index e7205df8d..fed4cd8cd 100644 --- a/spec/lib/activitypub/activity/reject_spec.rb +++ b/spec/lib/activitypub/activity/reject_spec.rb @@ -3,6 +3,14 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Reject do let(:sender) { Fabricate(:account) } let(:recipient) { Fabricate(:account) } + let(:object_json) do + { + id: 'bar', + type: 'Follow', + actor: ActivityPub::TagManager.instance.uri_for(recipient), + object: ActivityPub::TagManager.instance.uri_for(sender), + } + end let(:json) do { @@ -10,29 +18,105 @@ RSpec.describe ActivityPub::Activity::Reject do id: 'foo', type: 'Reject', actor: ActivityPub::TagManager.instance.uri_for(sender), - object: { - id: 'bar', - type: 'Follow', - actor: ActivityPub::TagManager.instance.uri_for(recipient), - object: ActivityPub::TagManager.instance.uri_for(sender), - }, + object: object_json, }.with_indifferent_access end describe '#perform' do subject { described_class.new(json, sender) } - before do - Fabricate(:follow_request, account: recipient, target_account: sender) - subject.perform + context 'rejecting a pending follow request by target' do + before do + Fabricate(:follow_request, account: recipient, target_account: sender) + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end end - it 'does not create a follow relationship' do - expect(recipient.following?(sender)).to be false + context 'rejecting a pending follow request by uri' do + before do + Fabricate(:follow_request, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end end - it 'removes the follow request' do - expect(recipient.requested?(sender)).to be false + context 'rejecting a pending follow request by uri only' do + let(:object_json) { 'bar' } + + before do + Fabricate(:follow_request, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'does not create a follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'removes the follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by target' do + before do + Fabricate(:follow, account: recipient, target_account: sender) + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by uri' do + before do + Fabricate(:follow, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end + end + + context 'rejecting an existing follow relationship by uri only' do + let(:object_json) { 'bar' } + + before do + Fabricate(:follow, account: recipient, target_account: sender, uri: 'bar') + subject.perform + end + + it 'removes the follow relationship' do + expect(recipient.following?(sender)).to be false + end + + it 'does not create a follow request' do + expect(recipient.requested?(sender)).to be false + end end end