From d41b43ed4fff8ccbee5eac62064c5def14f88973 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 8 Jul 2024 09:41:50 +0200 Subject: [PATCH] Limit attachments to `MEDIA_ATTACHMENTS_LIMIT` when returning posts through the API (#30932) --- app/models/status.rb | 2 +- app/models/status_edit.rb | 14 ++++++++------ spec/models/status_spec.rb | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index baa65780055..73f00526735 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -288,7 +288,7 @@ class Status < ApplicationRecord else map = media_attachments.index_by(&:id) ordered_media_attachment_ids.filter_map { |media_attachment_id| map[media_attachment_id] } - end + end.take(MEDIA_ATTACHMENTS_LIMIT) end def replies_count diff --git a/app/models/status_edit.rb b/app/models/status_edit.rb index 089c42fb994..6e25a6f3bbd 100644 --- a/app/models/status_edit.rb +++ b/app/models/status_edit.rb @@ -53,12 +53,14 @@ class StatusEdit < ApplicationRecord def ordered_media_attachments return @ordered_media_attachments if defined?(@ordered_media_attachments) - @ordered_media_attachments = if ordered_media_attachment_ids.nil? - [] - else - map = status.media_attachments.index_by(&:id) - ordered_media_attachment_ids.map.with_index { |media_attachment_id, index| PreservedMediaAttachment.new(media_attachment: map[media_attachment_id], description: media_descriptions[index]) } - end + @ordered_media_attachments = begin + if ordered_media_attachment_ids.nil? + [] + else + map = status.media_attachments.index_by(&:id) + ordered_media_attachment_ids.map.with_index { |media_attachment_id, index| PreservedMediaAttachment.new(media_attachment: map[media_attachment_id], description: media_descriptions[index]) } + end + end.take(Status::MEDIA_ATTACHMENTS_LIMIT) end def proper diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 271cf8690ec..df67c365eb3 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -247,6 +247,41 @@ RSpec.describe Status do end end + describe '#ordered_media_attachments' do + let(:status) { Fabricate(:status) } + + let(:first_attachment) { Fabricate(:media_attachment) } + let(:second_attachment) { Fabricate(:media_attachment) } + let(:last_attachment) { Fabricate(:media_attachment) } + let(:extra_attachment) { Fabricate(:media_attachment) } + + before do + stub_const('Status::MEDIA_ATTACHMENTS_LIMIT', 3) + + # Add attachments out of order + status.media_attachments << second_attachment + status.media_attachments << last_attachment + status.media_attachments << extra_attachment + status.media_attachments << first_attachment + end + + context 'when ordered_media_attachment_ids is not set' do + it 'returns up to MEDIA_ATTACHMENTS_LIMIT attachments' do + expect(status.ordered_media_attachments.size).to eq Status::MEDIA_ATTACHMENTS_LIMIT + end + end + + context 'when ordered_media_attachment_ids is set' do + before do + status.update!(ordered_media_attachment_ids: [first_attachment.id, second_attachment.id, last_attachment.id, extra_attachment.id]) + end + + it 'returns up to MEDIA_ATTACHMENTS_LIMIT attachments in the expected order' do + expect(status.ordered_media_attachments).to eq [first_attachment, second_attachment, last_attachment] + end + end + end + describe '.mutes_map' do subject { described_class.mutes_map([status.conversation.id], account) }