mirror of
https://github.com/mastodon/mastodon.git
synced 2025-09-05 17:31:12 +00:00
4fa462a9f8
Related to - https://github.com/mastodon/mastodon/pull/35458 - which added a batch base class, and to - https://github.com/mastodon/mastodon/pull/35872 - which envisions a world where form objects tell the view what to do. Background here... - The `Form::AccountBatch` form object is used in a few different places, for somewhat varied purposes - The user relationships view uses it on a show page, which does a PUT to update, and uses the `follow`, `unfollow`, `remove_from_followers`, and `remove_domains_from_followers` "actions" of the form model - The admin accounts view shows a form on index view, does POST to batch action, uses the `suspend`, `approve` and `reject` actions. - The admin relationships view (for admins only) also has a form on index and does a POST back to that same spot - The admin/follow_recommendations page has a form on show and does PUT to update, using the `suppress_follow_recommendation` and `unsuppress_follow_recommendation` actions So, while these are all related in that they supply an `accounts_ids` array, the actual things that they "bulk update" are Follow (from relationships), FollowRecommendationSuppression (from follow recs) and Account (from accounts). This is a little inconsistent with other of the "batch form model" classes, which tend to update the same domain concept (ie, CustomEmojiBlock, IpBlockBatch, EmailDomainBlockBatch, etc) that they are named after (and in some cases that a "filter" class is named after). All that said, the changes here: - Pull out `Form::FollowRecommendationBatch` and `Form::RelationshipBatch` classes, which more closely align with the domain concept they are bulk updating (open to naming suggestions here) - Mark both of these as `persisted?` true so that their form views will infer correct HTTP action - Add some missing coverage to existing class, and coverage for new classes as well It would be pretty straightforward to chop this up into smaller pieces, if any of "just the coverage first", "just the boolean attribute", "one new batch class at a time", etc - were more appealing. Possible follow-up: - In the existing `Form::AccountBatch`, use attributes API for boolean `select_all_matching` value, remove query method - Convert the relationships controller spec to mix of request/system specs, round out coverage - Most (all maybe?) of the "batch actions" are doing a `POST` to a `batch` action on the controller of whatever they are related to. Might be worth looking at what the diff would look like to move these all to nested controllers where they did a `PUT to */batches#update` with restful routing, etc.
88 lines
2.9 KiB
Ruby
88 lines
2.9 KiB
Ruby
# 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
|