Fix serialization of replies when some of them are URIs (#13957)

* Fix serialization of replies when some of them are URIs

Fixes #13956

* Add test

2 files changed, 28 insertions, 0 deletions

diff --git a/app/serializers/activitypub/collection_serializer.rb b/app/serializers/activitypub/collection_serializer.rb
index 00c7b786a..ea7af5433 100644
--- a/app/serializers/activitypub/collection_serializer.rb
+++ b/app/serializers/activitypub/collection_serializer.rb
@@ -1,6 +1,15 @@
 # frozen_string_literal: true
 
 class ActivityPub::CollectionSerializer < ActivityPub::Serializer
+  class StringSerializer < ActiveModel::Serializer
+    # Despite the name, it does not return a hash, but the same can be said of
+    # the ActiveModel::Serializer::CollectionSerializer class which handles
+    # arrays.
+    def serializable_hash(*_args)
+      object
+    end
+  end
+
   def self.serializer_for(model, options)
     case model.class.name
     when 'Status'
@@ -9,6 +18,8 @@ class ActivityPub::CollectionSerializer < ActivityPub::Serializer
       ActivityPub::DeviceSerializer
     when 'ActivityPub::CollectionPresenter'
       ActivityPub::CollectionSerializer
+    when 'String'
+      StringSerializer
     else
       super
     end
diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb
index a5ed14180..d956e1b35 100644
--- a/spec/controllers/activitypub/replies_controller_spec.rb
+++ b/spec/controllers/activitypub/replies_controller_spec.rb
@@ -4,6 +4,7 @@ require 'rails_helper'
 
 RSpec.describe ActivityPub::RepliesController, type: :controller do
   let(:status) { Fabricate(:status, visibility: parent_visibility) }
+  let(:remote_reply_id) { nil }
   let(:remote_account) { nil }
 
   before do
@@ -14,6 +15,8 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do
     Fabricate(:status, thread: status, visibility: :private)
     Fabricate(:status, account: status.account, thread: status, visibility: :public)
     Fabricate(:status, account: status.account, thread: status, visibility: :private)
+
+    Fabricate(:status, account: remote_account, thread: status, visibility: :public, uri: remote_reply_id) if remote_reply_id
   end
 
   describe 'GET #index' do
@@ -110,6 +113,20 @@ RSpec.describe ActivityPub::RepliesController, type: :controller do
               expect(json[:first][:items].size).to eq 2
               expect(json[:first][:items].all? { |item| item[:to].include?(ActivityPub::TagManager::COLLECTIONS[:public]) || item[:cc].include?(ActivityPub::TagManager::COLLECTIONS[:public]) }).to be true
             end
+
+            context 'with remote responses' do
+              let(:remote_reply_id) { 'foo' }
+
+              it 'returned items are all inlined local toots or are ids' do
+                json = body_as_json
+
+                expect(json[:first]).to be_a Hash
+                expect(json[:first][:items]).to be_an Array
+                expect(json[:first][:items].size).to eq 3
+                expect(json[:first][:items].all? { |item| item.is_a?(Hash) ? ActivityPub::TagManager.instance.local_uri?(item[:id]) : item.is_a?(String) }).to be true
+                expect(json[:first][:items]).to include remote_reply_id
+              end
+            end
           end
         end