diff --git a/app/controllers/admin/follow_recommendations_controller.rb b/app/controllers/admin/follow_recommendations_controller.rb index b060cfbe949..01719db2215 100644 --- a/app/controllers/admin/follow_recommendations_controller.rb +++ b/app/controllers/admin/follow_recommendations_controller.rb @@ -7,14 +7,14 @@ module Admin def show authorize :follow_recommendation, :show? - @form = Form::AccountBatch.new + @form = Form::FollowRecommendationBatch.new @accounts = filtered_follow_recommendations.page(params[:page]) end def update authorize :follow_recommendation, :show? - @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button)) + @form = Form::FollowRecommendationBatch.new(form_follow_recommendation_batch_params.merge(current_account: current_account, action: action_from_button)) @form.save rescue ActionController::ParameterMissing # Do nothing @@ -36,9 +36,9 @@ module Admin @follow_recommendation_filter ||= FollowRecommendationFilter.new(filter_params) end - def form_account_batch_params + def form_follow_recommendation_batch_params params - .expect(form_account_batch: [:action, account_ids: []]) + .expect(form_follow_recommendation_batch: [:action, account_ids: []]) end def filter_params diff --git a/app/controllers/relationships_controller.rb b/app/controllers/relationships_controller.rb index 7e793fc7342..b65162c25b2 100644 --- a/app/controllers/relationships_controller.rb +++ b/app/controllers/relationships_controller.rb @@ -10,11 +10,11 @@ class RelationshipsController < ApplicationController helper_method :following_relationship?, :followed_by_relationship?, :mutual_relationship? def show - @form = Form::AccountBatch.new + @form = Form::RelationshipBatch.new end def update - @form = Form::AccountBatch.new(form_account_batch_params.merge(current_account: current_account, action: action_from_button)) + @form = Form::RelationshipBatch.new(form_relationship_batch_params.merge(current_account: current_account, action: action_from_button)) @form.save rescue ActionController::ParameterMissing # Do nothing @@ -34,8 +34,8 @@ class RelationshipsController < ApplicationController @relationships = AccountRelationshipsPresenter.new(@accounts, current_user.account_id) end - def form_account_batch_params - params.expect(form_account_batch: [:action, account_ids: []]) + def form_relationship_batch_params + params.expect(form_relationship_batch: [:action, account_ids: []]) end def following_relationship? diff --git a/app/models/form/account_batch.rb b/app/models/form/account_batch.rb index f3109ad62a7..72d025a578b 100644 --- a/app/models/form/account_batch.rb +++ b/app/models/form/account_batch.rb @@ -9,22 +9,10 @@ class Form::AccountBatch < Form::BaseBatch def save case action - when 'follow' - follow! - when 'unfollow' - unfollow! - when 'remove_from_followers' - remove_from_followers! - when 'remove_domains_from_followers' - remove_domains_from_followers! when 'approve' approve! when 'reject' reject! - when 'suppress_follow_recommendation' - suppress_follow_recommendation! - when 'unsuppress_follow_recommendation' - unsuppress_follow_recommendation! when 'suspend' suspend! end @@ -32,36 +20,6 @@ class Form::AccountBatch < Form::BaseBatch private - def follow! - error = nil - - accounts.each do |target_account| - FollowService.new.call(current_account, target_account) - rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound => e - error ||= e - end - - raise error if error.present? - end - - def unfollow! - accounts.each do |target_account| - UnfollowService.new.call(current_account, target_account) - end - end - - def remove_from_followers! - RemoveFromFollowersService.new.call(current_account, account_ids) - end - - def remove_domains_from_followers! - RemoveDomainsFromFollowersService.new.call(current_account, account_domains) - end - - def account_domains - accounts.group(:domain).pluck(:domain).compact - end - def accounts if select_all_matching? query @@ -92,20 +50,6 @@ class Form::AccountBatch < Form::BaseBatch end end - def suppress_follow_recommendation! - authorize(:follow_recommendation, :suppress?) - - accounts.find_each do |account| - FollowRecommendationSuppression.create(account: account) - end - end - - def unsuppress_follow_recommendation! - authorize(:follow_recommendation, :unsuppress?) - - FollowRecommendationSuppression.where(account_id: account_ids).destroy_all - end - def reject_account(account) authorize(account.user, :reject?) log_action(:reject, account.user) diff --git a/app/models/form/follow_recommendation_batch.rb b/app/models/form/follow_recommendation_batch.rb new file mode 100644 index 00000000000..45fec0734b2 --- /dev/null +++ b/app/models/form/follow_recommendation_batch.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class Form::FollowRecommendationBatch < Form::BaseBatch + attr_accessor :account_ids + + def save + case action + when 'suppress_follow_recommendation' + suppress! + when 'unsuppress_follow_recommendation' + unsuppress! + end + end + + def persisted? + true + end + + private + + def suppress! + authorize(:follow_recommendation, :suppress?) + + accounts.find_each do |account| + FollowRecommendationSuppression.create(account: account) + end + end + + def unsuppress! + authorize(:follow_recommendation, :unsuppress?) + + FollowRecommendationSuppression.where(account_id: account_ids).destroy_all + end + + def accounts + Account.where(id: account_ids) + end +end diff --git a/app/models/form/relationship_batch.rb b/app/models/form/relationship_batch.rb new file mode 100644 index 00000000000..1d34a12288b --- /dev/null +++ b/app/models/form/relationship_batch.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class Form::RelationshipBatch < Form::BaseBatch + attr_accessor :account_ids + + def save + case action + when 'follow' + follow! + when 'unfollow' + unfollow! + when 'remove_from_followers' + remove_from_followers! + when 'remove_domains_from_followers' + remove_domains_from_followers! + end + end + + def persisted? + true + end + + private + + def follow! + error = nil + + accounts.each do |target_account| + FollowService.new.call(current_account, target_account) + rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound => e + error ||= e + end + + raise error if error.present? + end + + def unfollow! + accounts.each do |target_account| + UnfollowService.new.call(current_account, target_account) + end + end + + def remove_from_followers! + RemoveFromFollowersService.new.call(current_account, account_ids) + end + + def remove_domains_from_followers! + RemoveDomainsFromFollowersService.new.call(current_account, account_domains) + end + + def account_domains + accounts.group(:domain).pluck(:domain).compact + end + + def accounts + Account.where(id: account_ids) + end +end diff --git a/app/views/admin/follow_recommendations/show.html.haml b/app/views/admin/follow_recommendations/show.html.haml index 6c2627743b4..73d943614d5 100644 --- a/app/views/admin/follow_recommendations/show.html.haml +++ b/app/views/admin/follow_recommendations/show.html.haml @@ -21,7 +21,7 @@ %li= filter_link_to t('admin.accounts.moderation.active'), status: nil %li= filter_link_to t('admin.follow_recommendations.suppressed'), status: 'suppressed' -= form_with model: @form, url: admin_follow_recommendations_path, method: :patch do |f| += form_with model: @form, url: admin_follow_recommendations_path do |f| - RelationshipFilter::KEYS.each do |key| = hidden_field_tag key, params[key] if params[key].present? diff --git a/app/views/relationships/show.html.haml b/app/views/relationships/show.html.haml index 7528e7386b1..ae9fcb69f5f 100644 --- a/app/views/relationships/show.html.haml +++ b/app/views/relationships/show.html.haml @@ -31,7 +31,7 @@ %li= filter_link_to t('relationships.most_recent'), order: nil %li= filter_link_to t('relationships.last_active'), order: 'active' -= form_with model: @form, url: relationships_path, method: :patch do |f| += form_with model: @form, url: relationships_path do |f| = hidden_field_tag :page, params[:page] || 1 - RelationshipFilter::KEYS.each do |key| diff --git a/spec/controllers/relationships_controller_spec.rb b/spec/controllers/relationships_controller_spec.rb index 633d72fbba3..55b5be35fe9 100644 --- a/spec/controllers/relationships_controller_spec.rb +++ b/spec/controllers/relationships_controller_spec.rb @@ -53,7 +53,7 @@ RSpec.describe RelationshipsController do end context 'when select parameter is provided' do - subject { patch :update, params: { form_account_batch: { account_ids: [alice.id] }, remove_domains_from_followers: '' } } + subject { patch :update, params: { form_relationship_batch: { account_ids: [alice.id] }, remove_domains_from_followers: '' } } it 'soft-blocks followers from selected domains' do alice.follow!(user.account) diff --git a/spec/fabricators/follow_recommendation_suppression_fabricator.rb b/spec/fabricators/follow_recommendation_suppression_fabricator.rb new file mode 100644 index 00000000000..f545f9f3807 --- /dev/null +++ b/spec/fabricators/follow_recommendation_suppression_fabricator.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Fabricator(:follow_recommendation_suppression) do + account { Fabricate.build(:account) } +end diff --git a/spec/models/form/account_batch_spec.rb b/spec/models/form/account_batch_spec.rb index 8db9a2a2545..f5303f0ed3b 100644 --- a/spec/models/form/account_batch_spec.rb +++ b/spec/models/form/account_batch_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Form::AccountBatch do let(:account) { Fabricate(:admin_user).account } let(:account_ids) { [] } let(:query) { Account.none } + let(:select_all_matching) { false } before do account_batch.assign_attributes( @@ -77,5 +78,37 @@ RSpec.describe Form::AccountBatch do [target_account.reload, target_account2.reload].map(&:suspended?) end end + + context 'when action is "approve"' do + let(:action) { 'approve' } + let(:account_ids) { [unapproved_user_account.id] } + let(:unapproved_user_account) { Fabricate :account } + + before { unapproved_user_account.user.update!(approved: false) } + + it 'approves the user of the supplied accounts' do + expect { subject } + .to change { unapproved_user_account.reload.user.approved? }.to(true) + end + end + + context 'when action is "reject"' do + let(:action) { 'reject' } + let(:account_ids) { [reject_user_account.id] } + let(:reject_user_account) { Fabricate :account } + + before { reject_user_account.user.update!(approved: false) } + + it 'rejects the user of the supplied accounts' do + expect { subject } + .to change { reject_user_account.reload.suspended? }.to(true) + end + end + + context 'when action is "unknown"' do + let(:action) { 'unknown' } + + it { is_expected.to be_nil } + end end end diff --git a/spec/models/form/follow_recommendation_batch_spec.rb b/spec/models/form/follow_recommendation_batch_spec.rb new file mode 100644 index 00000000000..2988236fb12 --- /dev/null +++ b/spec/models/form/follow_recommendation_batch_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Form::FollowRecommendationBatch do + describe '#persisted?' do + it { is_expected.to be_persisted } + end + + describe '#save' do + subject { described_class.new(action:, account_ids:, current_account: admin).save } + + let(:account_ids) { [account.id] } + let(:account) { Fabricate :account } + let(:admin) { Fabricate :account, user: Fabricate(:admin_user) } + + context 'when action is suppress_follow_recommendation' do + let(:action) { 'suppress_follow_recommendation' } + + it 'adds a suppression for the accounts' do + expect { subject } + .to change(FollowRecommendationSuppression, :count).by(1) + .and change { account.reload.follow_recommendation_suppression }.from(be_nil).to(be_present) + end + end + + context 'when action is unsuppress_follow_recommendation' do + let(:action) { 'unsuppress_follow_recommendation' } + + before { Fabricate :follow_recommendation_suppression, account: } + + it 'removes a suppression for the accounts' do + expect { subject } + .to change(FollowRecommendationSuppression, :count).by(-1) + .and change { account.reload.follow_recommendation_suppression }.from(be_present).to(be_nil) + end + end + + context 'when action is unknown' do + let(:action) { 'unknown' } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/models/form/relationship_batch_spec.rb b/spec/models/form/relationship_batch_spec.rb new file mode 100644 index 00000000000..882703f1501 --- /dev/null +++ b/spec/models/form/relationship_batch_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Form::RelationshipBatch do + describe '#persisted?' do + it { is_expected.to be_persisted } + end + + describe '#save' do + subject { described_class.new(action:, account_ids:, current_account:).save } + + let(:account_ids) { [account.id] } + let(:account) { Fabricate :account } + let(:current_account) { Fabricate :account } + + context 'when action is follow' do + let(:action) { 'follow' } + let(:account_ids) { [account.id] } + + it 'adds a follow for the accounts' do + expect { subject } + .to change(Follow, :count).by(1) + .and change { current_account.reload.active_relationships.exists?(target_account: account) }.from(false).to(true) + end + + context 'when account cannot be followed' do + let(:account) { Fabricate :account, domain: 'test.example' } + + it 'does not save follows and re-raises error' do + expect { subject } + .to raise_error(Mastodon::NotPermittedError) + .and not_change(Follow, :count) + end + end + end + + context 'when action is unfollow' do + let(:action) { 'unfollow' } + + before { Fabricate :follow, account: current_account, target_account: account } + + it 'removes a follow for the accounts' do + expect { subject } + .to change(Follow, :count).by(-1) + .and change { current_account.reload.active_relationships.exists?(target_account: account) }.from(true).to(false) + end + end + + context 'when action is remove_from_followers' do + let(:action) { 'remove_from_followers' } + + before { Fabricate :follow, account: account, target_account: current_account } + + it 'removes followers from the accounts' do + expect { subject } + .to change(Follow, :count).by(-1) + .and change { current_account.reload.passive_relationships.exists?(account: account) }.from(true).to(false) + end + end + + context 'when action is remove_domains_from_followers' do + let(:action) { 'remove_domains_from_followers' } + + let(:account) { Fabricate :account, domain: 'host.example' } + let(:account_other) { Fabricate :account, domain: 'host.example' } + + before do + Fabricate :follow, account: account, target_account: current_account + Fabricate :follow, account: account_other, target_account: current_account + end + + it 'removes all followers from domains of the accounts' do + expect { subject } + .to change(Follow, :count).by(-2) + .and change { current_account.reload.passive_relationships.exists?(account: account) }.from(true).to(false) + .and change { current_account.reload.passive_relationships.exists?(account: account_other) }.from(true).to(false) + end + end + + context 'when action is unknown' do + let(:action) { 'unknown' } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/requests/admin/follow_recommendations_spec.rb b/spec/requests/admin/follow_recommendations_spec.rb index 146c26448cd..0d76fe4336c 100644 --- a/spec/requests/admin/follow_recommendations_spec.rb +++ b/spec/requests/admin/follow_recommendations_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Admin Follow Recommendations' do before { sign_in Fabricate(:admin_user) } it 'gracefully handles invalid nested params' do - put admin_follow_recommendations_path(form_account_batch: 'invalid') + put admin_follow_recommendations_path(form_follow_recommendation_batch: 'invalid') expect(response) .to redirect_to(admin_follow_recommendations_path) diff --git a/spec/requests/relationships_spec.rb b/spec/requests/relationships_spec.rb index ee6b321c46a..3d5d76ed8ca 100644 --- a/spec/requests/relationships_spec.rb +++ b/spec/requests/relationships_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Relationships' do before { sign_in Fabricate(:user) } it 'gracefully handles invalid nested params' do - put relationships_path(form_account_batch: 'invalid') + put relationships_path(form_relationship_batch: 'invalid') expect(response) .to redirect_to(relationships_path)