From adf8a3601de7869572bdd47857a65830124aaa0f Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Wed, 10 Dec 2025 17:59:21 +0100 Subject: [PATCH] Add service to add item to a collection (#37192) --- app/models/account.rb | 4 +++ app/models/collection_item.rb | 10 ++++++ app/policies/account_policy.rb | 4 +++ .../add_account_to_collection_service.rb | 23 ++++++++++++ config/locales/en.yml | 2 ++ spec/models/account_spec.rb | 33 +++++++++++++++++ spec/models/collection_item_spec.rb | 19 ++++++++++ spec/policies/account_policy_spec.rb | 32 +++++++++++++++++ .../add_account_to_collection_service_spec.rb | 35 +++++++++++++++++++ 9 files changed, 162 insertions(+) create mode 100644 app/services/add_account_to_collection_service.rb create mode 100644 spec/services/add_account_to_collection_service_spec.rb diff --git a/app/models/account.rb b/app/models/account.rb index 5f4caf9eaa..562d33508d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -455,6 +455,10 @@ class Account < ApplicationRecord save! end + def featureable? + local? && discoverable? + end + private def prepare_contents diff --git a/app/models/collection_item.rb b/app/models/collection_item.rb index 093005fd3e..5c624165e6 100644 --- a/app/models/collection_item.rb +++ b/app/models/collection_item.rb @@ -32,6 +32,8 @@ class CollectionItem < ApplicationRecord validates :account, presence: true, if: :accepted? validates :object_uri, presence: true, if: -> { account.nil? } + before_validation :set_position, on: :create + scope :ordered, -> { order(position: :asc) } scope :with_accounts, -> { includes(account: [:account_stat, :user]) } scope :not_blocked_by, ->(account) { where.not(accounts: { id: account.blocking }) } @@ -39,4 +41,12 @@ class CollectionItem < ApplicationRecord def local_item_with_remote_account? local? && account&.remote? end + + private + + def set_position + return if position_changed? + + self.position = self.class.where(collection_id:).maximum(:position).to_i + 1 + end end diff --git a/app/policies/account_policy.rb b/app/policies/account_policy.rb index a744af81de..50fa9b4d5c 100644 --- a/app/policies/account_policy.rb +++ b/app/policies/account_policy.rb @@ -64,4 +64,8 @@ class AccountPolicy < ApplicationPolicy def review? role.can?(:manage_taxonomies) end + + def feature? + record.featureable? && !current_account.blocking?(record) && !record.blocking?(current_account) + end end diff --git a/app/services/add_account_to_collection_service.rb b/app/services/add_account_to_collection_service.rb new file mode 100644 index 0000000000..4477384dd0 --- /dev/null +++ b/app/services/add_account_to_collection_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddAccountToCollectionService + def call(collection, account) + raise ArgumentError unless collection.local? + + @collection = collection + @account = account + + raise Mastodon::NotPermittedError, I18n.t('accounts.errors.cannot_be_added_to_collections') unless AccountPolicy.new(@collection.account, @account).feature? + + create_collection_item + end + + private + + def create_collection_item + @collection.collection_items.create!( + account: @account, + state: :accepted + ) + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index f2b6de3a77..abce180d57 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -7,6 +7,8 @@ en: hosted_on: Mastodon hosted on %{domain} title: About accounts: + errors: + cannot_be_added_to_collections: This account cannot be added to collections. followers: one: Follower other: Followers diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index be400fecd4..89a2f78c2e 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -781,4 +781,37 @@ RSpec.describe Account do expect(subject.reload.followers_count).to eq 15 end end + + describe '#featureable?' do + subject { Fabricate.build(:account, domain: (local ? nil : 'example.com'), discoverable:) } + + context 'when account is local' do + let(:local) { true } + + context 'when account is discoverable' do + let(:discoverable) { true } + + it 'returns `true`' do + expect(subject.featureable?).to be true + end + end + + context 'when account is not discoverable' do + let(:discoverable) { false } + + it 'returns `false`' do + expect(subject.featureable?).to be false + end + end + end + + context 'when account is remote' do + let(:local) { false } + let(:discoverable) { true } + + it 'returns `false`' do + expect(subject.featureable?).to be false + end + end + end end diff --git a/spec/models/collection_item_spec.rb b/spec/models/collection_item_spec.rb index 39464b7a34..3592c75e66 100644 --- a/spec/models/collection_item_spec.rb +++ b/spec/models/collection_item_spec.rb @@ -38,4 +38,23 @@ RSpec.describe CollectionItem do it { is_expected.to validate_presence_of(:object_uri) } end end + + describe 'Creation' do + let(:collection) { Fabricate(:collection) } + let(:other_collection) { Fabricate(:collection) } + let(:account) { Fabricate(:account) } + let(:other_account) { Fabricate(:account) } + + it 'automatically sets the `position` if absent' do + first_item = collection.collection_items.create(account:) + second_item = collection.collection_items.create(account: other_account) + unrelated_item = other_collection.collection_items.create(account:) + custom_item = other_collection.collection_items.create(account: other_account, position: 7) + + expect(first_item.position).to eq 1 + expect(second_item.position).to eq 2 + expect(unrelated_item.position).to eq 1 + expect(custom_item.position).to eq 7 + end + end end diff --git a/spec/policies/account_policy_spec.rb b/spec/policies/account_policy_spec.rb index 8b2edb15b0..f877bded25 100644 --- a/spec/policies/account_policy_spec.rb +++ b/spec/policies/account_policy_spec.rb @@ -156,4 +156,36 @@ RSpec.describe AccountPolicy do end end end + + permissions :feature? do + context 'when account is featureable?' do + it 'permits' do + expect(subject).to permit(alice, john) + end + end + + context 'when account is not featureable' do + before { allow(alice).to receive(:featureable?).and_return(false) } + + it 'denies' do + expect(subject).to_not permit(john, alice) + end + end + + context 'when account is blocked' do + before { alice.block!(john) } + + it 'denies' do + expect(subject).to_not permit(alice, john) + end + end + + context 'when account is blocking' do + before { john.block!(alice) } + + it 'denies' do + expect(subject).to_not permit(alice, john) + end + end + end end diff --git a/spec/services/add_account_to_collection_service_spec.rb b/spec/services/add_account_to_collection_service_spec.rb new file mode 100644 index 0000000000..98d88d0b8f --- /dev/null +++ b/spec/services/add_account_to_collection_service_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe AddAccountToCollectionService do + subject { described_class.new } + + let(:collection) { Fabricate.create(:collection) } + + describe '#call' do + context 'when given a featurable account' do + let(:account) { Fabricate(:account) } + + it 'creates a new CollectionItem in the `accepted` state' do + expect do + subject.call(collection, account) + end.to change(collection.collection_items, :count).by(1) + + new_item = collection.collection_items.last + expect(new_item.state).to eq 'accepted' + expect(new_item.account).to eq account + end + end + + context 'when given an account that is not featureable' do + let(:account) { Fabricate(:account, discoverable: false) } + + it 'raises an error' do + expect do + subject.call(collection, account) + end.to raise_error(Mastodon::NotPermittedError) + end + end + end +end