diff options
| -rw-r--r-- | app/services/follow_service.rb | 4 | ||||
| -rw-r--r-- | app/workers/move_worker.rb | 8 | ||||
| -rw-r--r-- | app/workers/unfollow_follow_worker.rb | 7 | ||||
| -rw-r--r-- | spec/workers/move_worker_spec.rb | 63 | ||||
| -rw-r--r-- | spec/workers/unfollow_follow_worker_spec.rb | 50 |
5 files changed, 126 insertions, 6 deletions
diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 1941c2e2d..dc47804c0 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -8,7 +8,7 @@ class FollowService < BaseService # @param [Account] source_account From which to follow # @param [String, Account] uri User URI to follow in the form of username@domain (or account record) # @param [true, false, nil] reblogs Whether or not to show reblogs, defaults to true - def call(source_account, target_account, reblogs: nil) + def call(source_account, target_account, reblogs: nil, bypass_locked: false) reblogs = true if reblogs.nil? target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true) @@ -30,7 +30,7 @@ class FollowService < BaseService ActivityTracker.increment('activity:interactions') - if target_account.locked? || source_account.silenced? || target_account.activitypub? + if (target_account.locked? && !bypass_locked) || source_account.silenced? || target_account.activitypub? request_follow(source_account, target_account, reblogs: reblogs) elsif target_account.local? direct_follow(source_account, target_account, reblogs: reblogs) diff --git a/app/workers/move_worker.rb b/app/workers/move_worker.rb index 83f7090ee..595730226 100644 --- a/app/workers/move_worker.rb +++ b/app/workers/move_worker.rb @@ -7,7 +7,7 @@ class MoveWorker @source_account = Account.find(source_account_id) @target_account = Account.find(target_account_id) - if @target_account.local? + if @target_account.local? && @source_account.local? rewrite_follows! else queue_follow_unfollows! @@ -21,13 +21,17 @@ class MoveWorker def rewrite_follows! @source_account.passive_relationships .where(account: Account.local) + .where.not(account: @target_account.followers.local) + .where.not(account_id: @target_account.id) .in_batches .update_all(target_account_id: @target_account.id) end def queue_follow_unfollows! + bypass_locked = @target_account.local? + @source_account.followers.local.select(:id).find_in_batches do |accounts| - UnfollowFollowWorker.push_bulk(accounts.map(&:id)) { |follower_id| [follower_id, @source_account.id, @target_account.id] } + UnfollowFollowWorker.push_bulk(accounts.map(&:id)) { |follower_id| [follower_id, @source_account.id, @target_account.id, bypass_locked] } end end end diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb index 95549e107..b6e665a41 100644 --- a/app/workers/unfollow_follow_worker.rb +++ b/app/workers/unfollow_follow_worker.rb @@ -5,12 +5,15 @@ class UnfollowFollowWorker sidekiq_options queue: 'pull' - def perform(follower_account_id, old_target_account_id, new_target_account_id) + def perform(follower_account_id, old_target_account_id, new_target_account_id, bypass_locked = false) follower_account = Account.find(follower_account_id) old_target_account = Account.find(old_target_account_id) new_target_account = Account.find(new_target_account_id) - FollowService.new.call(follower_account, new_target_account) + follow = follower_account.active_relationships.find_by(target_account: old_target_account) + reblogs = follow&.show_reblogs? + + FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, bypass_locked: bypass_locked) UnfollowService.new.call(follower_account, old_target_account, skip_unmerge: true) rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError true diff --git a/spec/workers/move_worker_spec.rb b/spec/workers/move_worker_spec.rb new file mode 100644 index 000000000..b8f4d9900 --- /dev/null +++ b/spec/workers/move_worker_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe MoveWorker do + let(:local_follower) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + let(:source_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } + let(:target_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } + + subject { described_class.new } + + before do + local_follower.follow!(source_account) + end + + context 'both accounts are distant' do + describe 'perform' do + it 'calls UnfollowFollowWorker' do + allow(UnfollowFollowWorker).to receive(:push_bulk) + subject.perform(source_account.id, target_account.id) + expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) + end + end + end + + context 'target account is local' do + let(:target_account) { Fabricate(:user, email: 'alice@example.com', account: Fabricate(:account, username: 'alice')).account } + + describe 'perform' do + it 'calls UnfollowFollowWorker' do + allow(UnfollowFollowWorker).to receive(:push_bulk) + subject.perform(source_account.id, target_account.id) + expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) + end + end + end + + context 'both target and source accounts are local' do + let(:target_account) { Fabricate(:user, email: 'alice@example.com', account: Fabricate(:account, username: 'alice')).account } + let(:source_account) { Fabricate(:user, email: 'alice_@example.com', account: Fabricate(:account, username: 'alice_')).account } + + describe 'perform' do + it 'calls makes local followers follow the target account' do + subject.perform(source_account.id, target_account.id) + expect(local_follower.following?(target_account)).to be true + end + + it 'does not fail when a local user is already following both accounts' do + double_follower = Fabricate(:user, email: 'eve@example.com', account: Fabricate(:account, username: 'eve')).account + double_follower.follow!(source_account) + double_follower.follow!(target_account) + subject.perform(source_account.id, target_account.id) + expect(local_follower.following?(target_account)).to be true + end + + it 'does not allow the moved account to follow themselves' do + source_account.follow!(target_account) + subject.perform(source_account.id, target_account.id) + expect(target_account.following?(target_account)).to be false + end + end + end +end diff --git a/spec/workers/unfollow_follow_worker_spec.rb b/spec/workers/unfollow_follow_worker_spec.rb new file mode 100644 index 000000000..5052c5616 --- /dev/null +++ b/spec/workers/unfollow_follow_worker_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe UnfollowFollowWorker do + let(:local_follower) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + let(:source_account) { Fabricate(:account) } + let(:target_account) { Fabricate(:account) } + let(:show_reblogs) { true } + + subject { described_class.new } + + before do + local_follower.follow!(source_account, reblogs: show_reblogs) + end + + context 'when show_reblogs is true' do + let(:show_reblogs) { true } + + describe 'perform' do + it 'unfollows source account and follows target account' do + subject.perform(local_follower.id, source_account.id, target_account.id) + expect(local_follower.following?(source_account)).to be false + expect(local_follower.following?(target_account)).to be true + end + + it 'preserves show_reblogs' do + subject.perform(local_follower.id, source_account.id, target_account.id) + expect(Follow.find_by(account: local_follower, target_account: target_account).show_reblogs?).to be show_reblogs + end + end + end + + context 'when show_reblogs is false' do + let(:show_reblogs) { false } + + describe 'perform' do + it 'unfollows source account and follows target account' do + subject.perform(local_follower.id, source_account.id, target_account.id) + expect(local_follower.following?(source_account)).to be false + expect(local_follower.following?(target_account)).to be true + end + + it 'preserves show_reblogs' do + subject.perform(local_follower.id, source_account.id, target_account.id) + expect(Follow.find_by(account: local_follower, target_account: target_account).show_reblogs?).to be show_reblogs + end + end + end +end |