Federate activity when remote account is added to a Collection (#37992)

app/models/collection_item.rb

@@ -35,6 +35,7 @@ class CollectionItem < ApplicationRecord
   validates :uri, presence: true, if: :remote?
 
   before_validation :set_position, on: :create
+  before_validation :set_activity_uri, only: :create, if: :local_item_with_remote_account?
 
   scope :ordered, -> { order(position: :asc) }
   scope :with_accounts, -> { includes(account: [:account_stat, :user]) }
@@ -55,4 +56,8 @@ class CollectionItem < ApplicationRecord
 
     self.position = self.class.where(collection_id:).maximum(:position).to_i + 1
   end
+
+  def set_activity_uri
+    self.activity_uri = [ActivityPub::TagManager.instance.uri_for(collection.account), '/feature_requests/', SecureRandom.uuid].join
+  end
 end

app/serializers/activitypub/feature_request_serializer.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class ActivityPub::FeatureRequestSerializer < ActivityPub::Serializer
+  attributes :id, :type, :instrument
+  attribute :virtual_object, key: :object
+
+  def id
+    object.activity_uri
+  end
+
+  def type
+    'FeatureRequest'
+  end
+
+  def virtual_object
+    ActivityPub::TagManager.instance.uri_for(object.account)
+  end
+
+  def instrument
+    ActivityPub::TagManager.instance.uri_for(object.collection)
+  end
+end

app/services/add_account_to_collection_service.rb

@@ -11,7 +11,10 @@ class AddAccountToCollectionService
 
     @collection_item = create_collection_item
 
-    distribute_add_activity if @account.local? && Mastodon::Feature.collections_federation_enabled?
+    if Mastodon::Feature.collections_federation_enabled?
+      distribute_add_activity if @account.local?
+      distribute_feature_request_activity if @account.remote?
+    end
 
     @collection_item
   end
@@ -26,10 +29,14 @@ class AddAccountToCollectionService
   end
 
   def distribute_add_activity
-    ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @collection.account_id)
+    ActivityPub::AccountRawDistributionWorker.perform_async(add_activity_json, @collection.account_id)
   end
 
-  def activity_json
+  def distribute_feature_request_activity
+    ActivityPub::FeatureRequestWorker.perform_async(@collection_item.id)
+  end
+
+  def add_activity_json
     ActiveModelSerializers::SerializableResource.new(@collection_item, serializer: ActivityPub::AddFeaturedItemSerializer, adapter: ActivityPub::Adapter).to_json
   end
 end

app/services/create_collection_service.rb

@@ -9,7 +9,10 @@ class CreateCollectionService
 
     @collection.save!
 
-    distribute_add_activity if Mastodon::Feature.collections_federation_enabled?
+    if Mastodon::Feature.collections_federation_enabled?
+      distribute_add_activity
+      distribute_feature_request_activities
+    end
 
     @collection
   end
@@ -20,6 +23,12 @@ class CreateCollectionService
     ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @account.id)
   end
 
+  def distribute_feature_request_activities
+    @collection.collection_items.select(&:local_item_with_remote_account?).each do |collection_item|
+      ActivityPub::FeatureRequestWorker.perform_async(collection_item.id)
+    end
+  end
+
   def build_items
     return if @accounts_to_add.empty?
 

app/workers/activitypub/feature_request_worker.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class ActivityPub::FeatureRequestWorker < ActivityPub::RawDistributionWorker
+  def perform(collection_item_id)
+    @collection_item = CollectionItem.find(collection_item_id)
+    @account = @collection_item.collection.account
+
+    distribute!
+  rescue ActiveRecord::RecordNotFound
+    true
+  end
+
+  protected
+
+  def inboxes
+    @inboxes ||= [@collection_item.account.inbox_url]
+  end
+
+  def payload
+    @payload ||= Oj.dump(serialize_payload(@collection_item, ActivityPub::FeatureRequestSerializer, signer: @account))
+  end
+end

spec/models/collection_item_spec.rb

@@ -16,14 +16,6 @@ RSpec.describe CollectionItem do
       it { is_expected.to validate_presence_of(:account) }
     end
 
-    context 'when item is local and account is remote' do
-      subject { Fabricate.build(:collection_item, account: remote_account) }
-
-      let(:remote_account) { Fabricate.build(:remote_account) }
-
-      it { is_expected.to validate_presence_of(:activity_uri) }
-    end
-
     context 'when item is not local' do
       subject { Fabricate.build(:collection_item, collection: remote_collection) }
 
@@ -58,5 +50,11 @@ RSpec.describe CollectionItem do
       expect(unrelated_item.position).to eq 1
       expect(custom_item.position).to eq 7
     end
+
+    it 'automatically sets `activity_uri` when account is remote' do
+      item = collection.collection_items.create(account: Fabricate(:remote_account))
+
+      expect(item.activity_uri).to be_present
+    end
   end
 end

spec/serializers/activitypub/feature_request_serializer_spec.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ActivityPub::FeatureRequestSerializer do
+  subject { serialized_record_json(collection_item, described_class, adapter: ActivityPub::Adapter) }
+
+  let(:tag_manager) { ActivityPub::TagManager.instance }
+  let(:target_account) { Fabricate(:remote_account) }
+  let(:collection) { Fabricate(:collection) }
+  let(:collection_item) { Fabricate(:collection_item, collection:, account: target_account) }
+
+  it 'serializes to the expected json' do
+    expect(subject).to include({
+      'id' => collection_item.activity_uri,
+      'type' => 'FeatureRequest',
+      'instrument' => tag_manager.uri_for(collection_item.collection),
+      'object' => tag_manager.uri_for(target_account),
+    })
+
+    expect(subject).to_not have_key('published')
+    expect(subject).to_not have_key('to')
+    expect(subject).to_not have_key('cc')
+    expect(subject).to_not have_key('target')
+  end
+end

spec/services/add_account_to_collection_service_spec.rb

@@ -21,10 +21,22 @@ RSpec.describe AddAccountToCollectionService do
         expect(new_item.account).to eq account
       end
 
-      it 'federates an `Add` activity', feature: :collections_federation do
-        subject.call(collection, account)
+      context 'when the account is local' do
+        it 'federates an `Add` activity', feature: :collections_federation do
+          subject.call(collection, account)
 
-        expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job
+          expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job
+        end
+      end
+
+      context 'when the account is remote', feature: :collections_federation do
+        let(:account) { Fabricate(:remote_account, feature_approval_policy: (0b10 << 16)) }
+
+        it 'federates a `FeatureRequest` activity' do
+          subject.call(collection, account)
+
+          expect(ActivityPub::FeatureRequestWorker).to have_enqueued_sidekiq_job
+        end
       end
     end
 

spec/services/create_collection_service_spec.rb

@@ -61,6 +61,16 @@ RSpec.describe CreateCollectionService do
             end.to raise_error(Mastodon::NotPermittedError)
           end
         end
+
+        context 'when some accounts are remote' do
+          let(:accounts) { Fabricate.times(2, :remote_account, feature_approval_policy: (0b10 << 16)) }
+
+          it 'federates `FeatureRequest` activities', feature: :collections_federation do
+            subject.call(params, author)
+
+            expect(ActivityPub::FeatureRequestWorker).to have_enqueued_sidekiq_job.exactly(2).times
+          end
+        end
       end
 
       context 'when given a tag' do

spec/workers/activitypub/feature_request_worker_spec.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ActivityPub::FeatureRequestWorker do
+  subject { described_class.new }
+
+  let(:account) { Fabricate(:account, inbox_url: 'http://example.com', domain: 'example.com') }
+  let(:collection_owner) { Fabricate(:account) }
+  let(:collection) { Fabricate(:collection, account: collection_owner) }
+  let(:collection_item) { Fabricate(:collection_item, collection:, account:) }
+
+  describe '#perform' do
+    it 'sends the expected `FeatureRequest` activity' do
+      subject.perform(collection_item.id)
+
+      expect(ActivityPub::DeliveryWorker)
+        .to have_enqueued_sidekiq_job(expected_json, collection_owner.id, 'http://example.com', {})
+    end
+
+    def expected_json
+      match_json_values(
+        id: a_string_matching(/^http/),
+        type: 'FeatureRequest',
+        object: ActivityPub::TagManager.instance.uri_for(account),
+        instrument: ActivityPub::TagManager.instance.uri_for(collection_item.collection)
+      )
+    end
+  end
+end