Handle adding already approved collection items (#38213)

app/models/collection_item.rb

@@ -41,6 +41,7 @@ class CollectionItem < ApplicationRecord
   scope :with_accounts, -> { includes(account: [:account_stat, :user]) }
   scope :not_blocked_by, ->(account) { where.not(accounts: { id: account.blocking }) }
   scope :local, -> { joins(:collection).merge(Collection.local) }
+  scope :accepted_partial, ->(account) { joins(:account).merge(Account.local).accepted.where(uri: nil, account_id: account.id) }
 
   def revoke!
     update!(state: :revoked)

app/services/activitypub/process_featured_item_service.rb

@@ -12,13 +12,21 @@ class ActivityPub::ProcessFeaturedItemService
     with_redis_lock("collection_item:#{item_json['id']}") do
       return if collection.collection_items.exists?(uri: item_json['id'])
 
-      @collection_item = collection.collection_items.create!(
-        uri: item_json['id'],
-        object_uri: item_json['featuredObject'],
-        approval_uri: item_json['featureAuthorization']
-      )
+      local_account = ActivityPub::TagManager.instance.uris_to_local_accounts([item_json['featuredObject']]).first
 
-      verify_authorization!
+      if local_account.present?
+        # This is a local account that has authorized this item already
+        @collection_item = collection.collection_items.accepted_partial(local_account).first
+        @collection_item&.update!(uri: item_json['id'])
+      else
+        @collection_item = collection.collection_items.create!(
+          uri: item_json['id'],
+          object_uri: item_json['featuredObject'],
+          approval_uri: item_json['featureAuthorization']
+        )
+
+        verify_authorization!
+      end
 
       @collection_item
     end

spec/services/activitypub/process_featured_item_service_spec.rb

@@ -3,17 +3,21 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::ProcessFeaturedItemService do
+  include RoutingHelper
+
   subject { described_class.new }
 
   let(:collection) { Fabricate(:remote_collection, uri: 'https://other.example.com/collection/1') }
+  let(:featured_object_uri) { 'https://example.com/actor/1' }
+  let(:feature_authorization_uri) { 'https://example.com/auth/1' }
   let(:featured_item_json) do
     {
       '@context' => 'https://www.w3.org/ns/activitystreams',
       'id' => 'https://other.example.com/featured_item/1',
       'type' => 'FeaturedItem',
-      'featuredObject' => 'https://example.com/actor/1',
+      'featuredObject' => featured_object_uri,
       'featuredObjectType' => 'Person',
-      'featureAuthorization' => 'https://example.com/auth/1',
+      'featureAuthorization' => feature_authorization_uri,
     }
   end
   let(:stubbed_service) do
@@ -50,6 +54,19 @@ RSpec.describe ActivityPub::ProcessFeaturedItemService do
       expect(new_item.object_uri).to eq 'https://example.com/actor/1'
       expect(new_item.approval_uri).to eq 'https://example.com/auth/1'
     end
+
+    context 'when an item exists for a local featured account' do
+      let!(:collection_item) do
+        Fabricate(:collection_item, collection:, state: :accepted)
+      end
+      let(:featured_object_uri) { ActivityPub::TagManager.instance.uri_for(collection_item.account) }
+      let(:feature_authorization_uri) { ap_account_feature_authorization_url(collection_item.account_id, collection_item) }
+
+      it 'updates the URI of the existing record' do
+        expect { subject.call(collection, object) }.to_not change(collection.collection_items, :count)
+        expect(collection_item.reload.uri).to eq 'https://other.example.com/featured_item/1'
+      end
+    end
   end
 
   context 'when only the id of the collection item is given' do