Fix media attachment order of remote posts (mastodon#28469)

glitch-soc · Dec 22, 2023 · ea6c187 · ea6c187
1 parent 72ff0d3
commit ea6c187
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 4 deletions.
diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
@@ -110,6 +110,8 @@ def find_existing_status
   def process_status_params
     @status_parser = ActivityPub::Parser::StatusParser.new(@json, followers_collection: @account.followers_url)
 
+    attachment_ids = process_attachments.take(4).map(&:id)
+
     @params = {
       uri: @status_parser.uri,
       url: @status_parser.url || @status_parser.uri,
@@ -125,7 +127,8 @@ def process_status_params
       visibility: @status_parser.visibility,
       thread: replied_to_status,
       conversation: conversation_from_uri(@object['conversation']),
-      media_attachment_ids: process_attachments.take(4).map(&:id),
+      media_attachment_ids: attachment_ids,
+      ordered_media_attachment_ids: attachment_ids,
       poll: process_poll,
     }
   end

diff --git a/app/models/status.rb b/app/models/status.rb
@@ -284,7 +284,9 @@ def emojis
 
   def ordered_media_attachments
     if ordered_media_attachment_ids.nil?
-      media_attachments
+      # NOTE: sort Ruby-side to avoid hitting the database when the status is
+      # not persisted to database yet
+      media_attachments.sort_by(&:id)
     else
       map = media_attachments.index_by(&:id)
       ordered_media_attachment_ids.filter_map { |media_attachment_id| map[media_attachment_id] }

diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb
@@ -578,15 +578,21 @@ def activity_for_object(json)
                 mediaType: 'image/png',
                 url: 'http://example.com/attachment.png',
               },
+              {
+                type: 'Document',
+                mediaType: 'image/png',
+                url: 'http://example.com/emoji.png',
+              },
             ],
           }
         end
 
-        it 'creates status' do
+        it 'creates status with correctly-ordered media attachments' do
           status = sender.statuses.first
 
           expect(status).to_not be_nil
-          expect(status.media_attachments.map(&:remote_url)).to include('http://example.com/attachment.png')
+          expect(status.ordered_media_attachments.map(&:remote_url)).to eq ['http://example.com/attachment.png', 'http://example.com/emoji.png']
+          expect(status.ordered_media_attachment_ids).to be_present
         end
       end