Change how changes to media attachments are stored for edits (#17696)

* Change how changes to media attachments are stored for edits

Fix not being able to re-order media attachments

* Fix not broadcasting updates when polls/media is changed through ActivityPub

* Various fixes and improvements

* Update app/models/report.rb

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

* Add tracking of media attachment description changes

* Change poll in status edit to have a structure closer to the real one

Co-authored-by: Claire <claire.github-309c@sitedethib.com>

app/chewy/statuses_index.rb

@@ -57,7 +57,7 @@ class StatusesIndex < Chewy::Index
     field :id, type: 'long'
     field :account_id, type: 'long'
 
-    field :text, type: 'text', value: ->(status) { [status.spoiler_text, Formatter.instance.plaintext(status)].concat(status.media_attachments.map(&:description)).concat(status.preloadable_poll ? status.preloadable_poll.options : []).join("\n\n") } do
+    field :text, type: 'text', value: ->(status) { [status.spoiler_text, Formatter.instance.plaintext(status)].concat(status.ordered_media_attachments.map(&:description)).concat(status.preloadable_poll ? status.preloadable_poll.options : []).join("\n\n") } do
       field :stemmed, type: 'text', analyzer: 'content'
     end
 

app/helpers/statuses_helper.rb

@@ -132,7 +132,7 @@ module StatusesHelper
   end
 
   def render_video_component(status, **options)
-    video = status.media_attachments.first
+    video = status.ordered_media_attachments.first
 
     meta = video.file.meta || {}
 
@@ -150,12 +150,12 @@ module StatusesHelper
     }.merge(**options)
 
     react_component :video, component_params do
-      render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
+      render partial: 'statuses/attachment_list', locals: { attachments: status.ordered_media_attachments }
     end
   end
 
   def render_audio_component(status, **options)
-    audio = status.media_attachments.first
+    audio = status.ordered_media_attachments.first
 
     meta = audio.file.meta || {}
 
@@ -170,7 +170,7 @@ module StatusesHelper
     }.merge(**options)
 
     react_component :audio, component_params do
-      render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
+      render partial: 'statuses/attachment_list', locals: { attachments: status.ordered_media_attachments }
     end
   end
 
@@ -178,11 +178,11 @@ module StatusesHelper
     component_params = {
       sensitive: sensitized?(status, current_account),
       autoplay: prefers_autoplay?,
-      media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json },
+      media: status.ordered_media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json },
     }.merge(**options)
 
     react_component :media_gallery, component_params do
-      render partial: 'statuses/attachment_list', locals: { attachments: status.media_attachments }
+      render partial: 'statuses/attachment_list', locals: { attachments: status.ordered_media_attachments }
     end
   end
 

app/lib/feed_manager.rb

@@ -448,7 +448,7 @@ class FeedManager
       Formatter.instance.plaintext(status),
       status.spoiler_text,
       status.preloadable_poll ? status.preloadable_poll.options.join("\n\n") : nil,
-      status.media_attachments.map(&:description).join("\n\n"),
+      status.ordered_media_attachments.map(&:description).join("\n\n"),
     ].compact.join("\n\n")
 
     combined_regex.match?(combined_text)

app/lib/rss/serializer.rb

@@ -11,7 +11,7 @@ class RSS::Serializer
             .pub_date(status.created_at)
             .description(status.spoiler_text.presence || Formatter.instance.format(status, inline_poll_options: true).to_str)
 
-        status.media_attachments.each do |media|
+        status.ordered_media_attachments.each do |media|
           item.enclosure(full_asset_url(media.file.url(:original, false)), media.file.content_type, media.file.size)
         end
       end

app/models/report.rb

@@ -63,8 +63,20 @@ class Report < ApplicationRecord
     Status.with_discarded.where(id: status_ids)
   end
 
-  def media_attachments
-    MediaAttachment.where(status_id: status_ids)
+  def media_attachments_count
+    statuses_to_query = []
+    count = 0
+
+    statuses.pluck(:id, :ordered_media_attachment_ids).each do |id, ordered_ids|
+      if ordered_ids.nil?
+        statuses_to_query << id
+      else
+        count += ordered_ids.size
+      end
+    end
+
+    count += MediaAttachment.where(status_id: statuses_to_query).count unless statuses_to_query.empty?
+    count
   end
 
   def rules

app/models/status.rb

@@ -25,6 +25,7 @@
 #  deleted_at                   :datetime
 #  edited_at                    :datetime
 #  trendable                    :boolean
+#  ordered_media_attachment_ids :bigint(8)        is an Array
 #
 
 class Status < ApplicationRecord
@@ -211,11 +212,14 @@ class Status < ApplicationRecord
     public_visibility? || unlisted_visibility?
   end
 
-  def snapshot!(media_attachments_changed: false, account_id: nil, at_time: nil)
+  def snapshot!(account_id: nil, at_time: nil)
     edits.create!(
       text: text,
       spoiler_text: spoiler_text,
-      media_attachments_changed: media_attachments_changed,
+      sensitive: sensitive,
+      ordered_media_attachment_ids: ordered_media_attachment_ids || media_attachments.pluck(:id),
+      media_descriptions: ordered_media_attachments.map(&:description),
+      poll_options: preloadable_poll&.options,
       account_id: account_id || self.account_id,
       created_at: at_time || edited_at
     )
@@ -228,7 +232,7 @@ class Status < ApplicationRecord
   alias sign? distributable?
 
   def with_media?
-    media_attachments.any?
+    ordered_media_attachments.any?
   end
 
   def with_preview_card?
@@ -252,6 +256,15 @@ class Status < ApplicationRecord
     @emojis = CustomEmoji.from_text(fields.join(' '), account.domain)
   end
 
+  def ordered_media_attachments
+    if ordered_media_attachment_ids.nil?
+      media_attachments
+    else
+      map = media_attachments.index_by(&:id)
+      ordered_media_attachment_ids.map { |media_attachment_id| map[media_attachment_id] }
+    end
+  end
+
   def replies_count
     status_stat&.replies_count || 0
   end

app/models/status_edit.rb

@@ -8,12 +8,24 @@
 #  account_id                   :bigint(8)
 #  text                         :text             default(""), not null
 #  spoiler_text                 :text             default(""), not null
-#  media_attachments_changed :boolean          default(FALSE), not null
 #  created_at                   :datetime         not null
 #  updated_at                   :datetime         not null
+#  ordered_media_attachment_ids :bigint(8)        is an Array
+#  media_descriptions           :text             is an Array
+#  poll_options                 :string           is an Array
+#  sensitive                    :boolean
 #
 
 class StatusEdit < ApplicationRecord
+  self.ignored_columns = %w(
+    media_attachments_changed
+  )
+
+  class PreservedMediaAttachment < ActiveModelSerializers::Model
+    attributes :media_attachment, :description
+    delegate :id, :type, :url, :preview_url, :remote_url, :preview_remote_url, :text_url, :meta, :blurhash, to: :media_attachment
+  end
+
   belongs_to :status
   belongs_to :account, optional: true
 
@@ -25,4 +37,17 @@ class StatusEdit < ApplicationRecord
     return @emojis if defined?(@emojis)
     @emojis = CustomEmoji.from_text([spoiler_text, text].join(' '), status.account.domain)
   end
+
+  def ordered_media_attachments
+    return @ordered_media_attachments if defined?(@ordered_media_attachments)
+
+    @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
+  end
 end

app/serializers/activitypub/note_serializer.rb

@@ -13,7 +13,7 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
   attribute :content_map, if: :language?
   attribute :updated, if: :edited?
 
-  has_many :media_attachments, key: :attachment
+  has_many :virtual_attachments, key: :attachment
   has_many :virtual_tags, key: :tag
 
   has_one :replies, serializer: ActivityPub::CollectionSerializer, if: :local?
@@ -106,6 +106,10 @@ class ActivityPub::NoteSerializer < ActivityPub::Serializer
     object.account.sensitized? || object.sensitive
   end
 
+  def virtual_attachments
+    object.ordered_media_attachments
+  end
+
   def virtual_tags
     object.active_mentions.to_a.sort_by(&:id) + object.tags + object.emojis
   end

app/serializers/rest/status_edit_serializer.rb

@@ -3,12 +3,18 @@
 class REST::StatusEditSerializer < ActiveModel::Serializer
   has_one :account, serializer: REST::AccountSerializer
 
-  attributes :content, :spoiler_text,
-             :media_attachments_changed, :created_at
+  attributes :content, :spoiler_text, :sensitive, :created_at
 
+  has_many :ordered_media_attachments, key: :media_attachments, serializer: REST::MediaAttachmentSerializer
   has_many :emojis, serializer: REST::CustomEmojiSerializer
 
+  attribute :poll, if: -> { object.poll_options.present? }
+
   def content
     Formatter.instance.format(object)
   end
+
+  def poll
+    { options: object.poll_options.map { |title| { title: title } } }
+  end
 end

app/serializers/rest/status_serializer.rb

@@ -19,7 +19,7 @@ class REST::StatusSerializer < ActiveModel::Serializer
   belongs_to :application, if: :show_application?
   belongs_to :account, serializer: REST::AccountSerializer
 
-  has_many :media_attachments, serializer: REST::MediaAttachmentSerializer
+  has_many :ordered_media_attachments, key: :media_attachments, serializer: REST::MediaAttachmentSerializer
   has_many :ordered_mentions, key: :mentions
   has_many :tags
   has_many :emojis, serializer: REST::CustomEmojiSerializer

app/services/activitypub/process_status_update_service.rb

@@ -76,13 +76,14 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
       end
     end
 
-    removed_media_attachments = previous_media_attachments - next_media_attachments
     added_media_attachments = next_media_attachments - previous_media_attachments
 
-    MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil)
     MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id)
 
-    @media_attachments_changed = true if removed_media_attachments.any? || added_media_attachments.any?
+    @status.ordered_media_attachment_ids = next_media_attachments.map(&:id)
+    @status.media_attachments.reload
+
+    @media_attachments_changed = true if @status.ordered_media_attachment_ids_changed?
   end
 
   def update_poll!
@@ -215,19 +216,13 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
 
     return if @status.edits.any?
 
-    @status.snapshot!(
-      media_attachments_changed: false,
-      at_time: @status.created_at
-    )
+    @status.snapshot!(at_time: @status.created_at)
   end
 
   def create_edit!
     return unless significant_changes?
 
-    @status.snapshot!(
-      media_attachments_changed: @media_attachments_changed || @poll_changed,
-      account_id: @account.id
-    )
+    @status.snapshot!(account_id: @account.id)
   end
 
   def skip_download?

app/services/fan_out_on_write_service.rb

@@ -110,7 +110,7 @@ class FanOutOnWriteService < BaseService
     Redis.current.publish('timeline:public', anonymous_payload)
     Redis.current.publish(@status.local? ? 'timeline:public:local' : 'timeline:public:remote', anonymous_payload)
 
-    if @status.media_attachments.any?
+    if @status.with_media?
       Redis.current.publish('timeline:public:media', anonymous_payload)
       Redis.current.publish(@status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', anonymous_payload)
     end

app/services/post_status_service.rb

@@ -100,7 +100,10 @@ class PostStatusService < BaseService
   end
 
   def validate_media!
-    return if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable)
+    if @options[:media_ids].blank? || !@options[:media_ids].is_a?(Enumerable)
+      @media = []
+      return
+    end
 
     raise Mastodon::ValidationError, I18n.t('media_attachments.validations.too_many') if @options[:media_ids].size > 4 || @options[:poll].present?
 
@@ -157,6 +160,7 @@ class PostStatusService < BaseService
     {
       text: @text,
       media_attachments: @media || [],
+      ordered_media_attachment_ids: (@options[:media_ids] || []).map(&:to_i) & @media.map(&:id),
       thread: @in_reply_to,
       poll_attributes: poll_attributes,
       sensitive: @sensitive,

app/services/remove_status_service.rb

@@ -40,7 +40,7 @@ class RemoveStatusService < BaseService
           remove_reblogs
           remove_from_hashtags
           remove_from_public
-          remove_from_media if @status.media_attachments.any?
+          remove_from_media if @status.with_media?
           remove_media
         end
 

app/services/update_status_service.rb

@@ -17,8 +17,6 @@ class UpdateStatusService < BaseService
     @status                    = status
     @options                   = options
     @account_id                = account_id
-    @media_attachments_changed = false
-    @poll_changed              = false
 
     Status.transaction do
       create_previous_edit!
@@ -41,14 +39,12 @@ class UpdateStatusService < BaseService
   def update_media_attachments!
     previous_media_attachments = @status.media_attachments.to_a
     next_media_attachments     = validate_media!
-    removed_media_attachments  = previous_media_attachments - next_media_attachments
     added_media_attachments    = next_media_attachments - previous_media_attachments
 
-    MediaAttachment.where(id: removed_media_attachments.map(&:id)).update_all(status_id: nil)
     MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id)
 
+    @status.ordered_media_attachment_ids = (@options[:media_ids] || []).map(&:to_i) & next_media_attachments.map(&:id)
     @status.media_attachments.reload
-    @media_attachments_changed = true if removed_media_attachments.any? || added_media_attachments.any?
   end
 
   def validate_media!
@@ -73,19 +69,18 @@ class UpdateStatusService < BaseService
 
       # If for some reasons the options were changed, it invalidates all previous
       # votes, so we need to remove them
-      @poll_changed = true if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple
+      poll_changed = true if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple
 
       poll.options     = @options[:poll][:options]
       poll.hide_totals = @options[:poll][:hide_totals] || false
       poll.multiple    = @options[:poll][:multiple] || false
       poll.expires_in  = @options[:poll][:expires_in]
-      poll.reset_votes! if @poll_changed
+      poll.reset_votes! if poll_changed
       poll.save!
 
       @status.poll_id = poll.id
     elsif previous_poll.present?
       previous_poll.destroy
-      @poll_changed = true
       @status.poll_id = nil
     end
   end
@@ -136,16 +131,10 @@ class UpdateStatusService < BaseService
 
     return if @status.edits.any?
 
-    @status.snapshot!(
-      media_attachments_changed: false,
-      at_time: @status.created_at
-    )
+    @status.snapshot!(at_time: @status.created_at)
   end
 
   def create_edit!
-    @status.snapshot!(
-      media_attachments_changed: @media_attachments_changed || @poll_changed,
-      account_id: @account_id
-    )
+    @status.snapshot!(account_id: @account_id)
   end
 end

app/views/admin/reports/_status.html.haml

@@ -11,15 +11,15 @@
             %strong> Content warning: #{Formatter.instance.format_spoiler(status.proper)}
           = Formatter.instance.format(status.proper, custom_emojify: true)
 
-    - unless status.proper.media_attachments.empty?
-      - if status.proper.media_attachments.first.video?
-        - video = status.proper.media_attachments.first
+    - unless status.proper.ordered_media_attachments.empty?
+      - if status.proper.ordered_media_attachments.first.video?
+        - video = status.proper.ordered_media_attachments.first
         = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), frameRate: video.file.meta.dig('original', 'frame_rate'), blurhash: video.blurhash, sensitive: status.proper.sensitive?, visible: false, width: 610, height: 343, inline: true, alt: video.description, media: [ActiveModelSerializers::SerializableResource.new(video, serializer: REST::MediaAttachmentSerializer)].as_json
-      - elsif status.proper.media_attachments.first.audio?
-        - audio = status.proper.media_attachments.first
+      - elsif status.proper.ordered_media_attachments.first.audio?
+        - audio = status.proper.ordered_media_attachments.first
         = react_component :audio, src: audio.file.url(:original), height: 110, alt: audio.description, duration: audio.file.meta.dig(:original, :duration)
       - else
-        = react_component :media_gallery, height: 343, sensitive: status.proper.sensitive?, visible: false, media: status.proper.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
+        = react_component :media_gallery, height: 343, sensitive: status.proper.sensitive?, visible: false, media: status.proper.ordered_media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }
 
     .detailed-status__meta
       - if status.application

app/views/admin/reports/index.html.haml

@@ -59,11 +59,11 @@
 
               %span.report-card__summary__item__content__icon{ title: t('admin.accounts.statuses') }
                 = fa_icon('comment')
-                = report.statuses.count
+                = report.status_ids.size
 
               %span.report-card__summary__item__content__icon{ title: t('admin.accounts.media_attachments') }
                 = fa_icon('camera')
-                = report.media_attachments.count
+                = report.media_attachments_count
 
               - if report.forwarded?
                 ·

app/views/admin/trends/statuses/_status.html.haml

@@ -9,7 +9,7 @@
       = link_to ActivityPub::TagManager.instance.url_for(status), target: '_blank', class: 'emojify', rel: 'noopener noreferrer' do
         = one_line_preview(status)
 
-        - status.media_attachments.each do |media_attachment|
+        - status.ordered_media_attachments.each do |media_attachment|
           %abbr{ title: media_attachment.description }
             = fa_icon 'link'
             = media_attachment.file_file_name

app/views/disputes/strikes/show.html.haml

@@ -53,7 +53,7 @@
                   = link_to short_account_status_url(@strike.target_account, status_id), class: 'emojify' do
                     = one_line_preview(status)
 
-                    - status.media_attachments.each do |media_attachment|
+                    - status.ordered_media_attachments.each do |media_attachment|
                       %abbr{ title: media_attachment.description }
                         = fa_icon 'link'
                         = media_attachment.file_file_name

app/views/notification_mailer/_status.html.haml

@@ -33,9 +33,9 @@
                               %div.auto-dir
                                 = Formatter.instance.format(status)
 
-                                - if status.media_attachments.size > 0
+                                - if status.ordered_media_attachments.size > 0
                                   %p
-                                    - status.media_attachments.each do |a|
+                                    - status.ordered_media_attachments.each do |a|
                                       - if status.local?
                                         = link_to full_asset_url(a.file.url(:original)), full_asset_url(a.file.url(:original))
                                       - else

app/views/statuses/_detailed_status.html.haml

@@ -25,10 +25,10 @@
       - if status.preloadable_poll
         = render_poll_component(status)
 
-  - if !status.media_attachments.empty?
-    - if status.media_attachments.first.video?
+  - if !status.ordered_media_attachments.empty?
+    - if status.ordered_media_attachments.first.video?
       = render_video_component(status, width: 670, height: 380, detailed: true)
-    - elsif status.media_attachments.first.audio?
+    - elsif status.ordered_media_attachments.first.audio?
       = render_audio_component(status, width: 670, height: 380)
     - else
       = render_media_gallery_component(status, height: 380, standalone: true)

app/views/statuses/_og_image.html.haml

@@ -1,6 +1,6 @@
 - if activity.is_a?(Status) && (activity.non_sensitive_with_media? || (activity.with_media? && Setting.preview_sensitive_media))
   - player_card = false
-  - activity.media_attachments.each do |media|
+  - activity.ordered_media_attachments.each do |media|
     - if media.image?
       = opengraph 'og:image', full_asset_url(media.file.url(:original))
       = opengraph 'og:image:type', media.file_content_type

app/views/statuses/_simple_status.html.haml

@@ -37,10 +37,10 @@
       - if status.preloadable_poll
         = render_poll_component(status)
 
-  - if !status.media_attachments.empty?
-    - if status.media_attachments.first.video?
+  - if !status.ordered_media_attachments.empty?
+    - if status.ordered_media_attachments.first.video?
       = render_video_component(status, width: 610, height: 343)
-    - elsif status.media_attachments.first.audio?
+    - elsif status.ordered_media_attachments.first.audio?
       = render_audio_component(status, width: 610, height: 343)
     - else
       = render_media_gallery_component(status, height: 343)

db/migrate/20220302232632_add_ordered_media_attachment_ids_to_statuses.rb

@@ -0,0 +1,5 @@
+class AddOrderedMediaAttachmentIdsToStatuses < ActiveRecord::Migration[6.1]
+  def change
+    add_column :statuses, :ordered_media_attachment_ids, :bigint, array: true
+  end
+end

db/migrate/20220303000827_add_ordered_media_attachment_ids_to_status_edits.rb

@@ -0,0 +1,8 @@
+class AddOrderedMediaAttachmentIdsToStatusEdits < ActiveRecord::Migration[6.1]
+  def change
+    add_column :status_edits, :ordered_media_attachment_ids, :bigint, array: true
+    add_column :status_edits, :media_descriptions, :text, array: true
+    add_column :status_edits, :poll_options, :string, array: true
+    add_column :status_edits, :sensitive, :boolean
+  end
+end

db/post_migrate/20220303203437_remove_media_attachments_changed_from_status_edits.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class RemoveMediaAttachmentsChangedFromStatusEdits < ActiveRecord::Migration[5.2]
+  def change
+    safety_assured { remove_column :status_edits, :media_attachments_changed, :boolean, default: false, null: false }
+  end
+end

db/schema.rb

@@ -391,7 +391,6 @@ ActiveRecord::Schema.define(version: 2022_03_07_094650) do
     t.bigint "parent_id"
     t.inet "ips", array: true
     t.datetime "last_refresh_at"
-
     t.index ["domain"], name: "index_email_domain_blocks_on_domain", unique: true
   end
 
@@ -845,9 +844,12 @@ ActiveRecord::Schema.define(version: 2022_03_07_094650) do
     t.bigint "account_id"
     t.text "text", default: "", null: false
     t.text "spoiler_text", default: "", null: false
-    t.boolean "media_attachments_changed", default: false, null: false
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
+    t.bigint "ordered_media_attachment_ids", array: true
+    t.text "media_descriptions", array: true
+    t.string "poll_options", array: true
+    t.boolean "sensitive"
     t.index ["account_id"], name: "index_status_edits_on_account_id"
     t.index ["status_id"], name: "index_status_edits_on_status_id"
   end
@@ -892,6 +894,7 @@ ActiveRecord::Schema.define(version: 2022_03_07_094650) do
     t.datetime "deleted_at"
     t.datetime "edited_at"
     t.boolean "trendable"
+    t.bigint "ordered_media_attachment_ids", array: true
     t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)"
     t.index ["deleted_at"], name: "index_statuses_on_deleted_at", where: "(deleted_at IS NOT NULL)"
     t.index ["id", "account_id"], name: "index_statuses_local_20190824", order: { id: :desc }, where: "((local OR (uri IS NULL)) AND (deleted_at IS NULL) AND (visibility = 0) AND (reblog_of_id IS NULL) AND ((NOT reply) OR (in_reply_to_account_id = account_id)))"

spec/models/report_spec.rb

@@ -11,14 +11,13 @@ describe Report do
     end
   end
 
-  describe 'media_attachments' do
-    it 'returns media attachments from statuses' do
-      status = Fabricate(:status)
-      media_attachment = Fabricate(:media_attachment, status: status)
-      _other_media_attachment = Fabricate(:media_attachment)
-      report = Fabricate(:report, status_ids: [status.id])
+  describe 'media_attachments_count' do
+    it 'returns count of media attachments in statuses' do
+      status1 = Fabricate(:status, ordered_media_attachment_ids: [1, 2])
+      status2 = Fabricate(:status, ordered_media_attachment_ids: [5])
+      report  = Fabricate(:report, status_ids: [status1.id, status2.id])
 
-      expect(report.media_attachments).to eq [media_attachment]
+      expect(report.media_attachments_count).to eq 3
     end
   end
 

spec/services/activitypub/process_status_update_service_spec.rb

@@ -124,7 +124,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       end
 
       it 'updates media attachments' do
-        media_attachment = status.media_attachments.reload.first
+        media_attachment = status.reload.ordered_media_attachments.first
 
         expect(media_attachment).to_not be_nil
         expect(media_attachment.remote_url).to eq 'https://example.com/foo.png'
@@ -135,7 +135,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       end
 
       it 'records media change in edit' do
-        expect(status.edits.reload.last.media_attachments_changed).to be true
+        expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty
       end
     end
 
@@ -173,11 +173,11 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       end
 
       it 'updates media attachments' do
-        expect(status.media_attachments.reload.map(&:remote_url)).to eq %w(https://example.com/foo.png)
+        expect(status.ordered_media_attachments.map(&:remote_url)).to eq %w(https://example.com/foo.png)
       end
 
       it 'records media change in edit' do
-        expect(status.edits.reload.last.media_attachments_changed).to be true
+        expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty
       end
     end
 
@@ -193,7 +193,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       end
 
       it 'records media change in edit' do
-        expect(status.edits.reload.last.media_attachments_changed).to be true
+        expect(status.edits.reload.last.poll_options).to be_nil
       end
     end
 
@@ -226,7 +226,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       end
 
       it 'records media change in edit' do
-        expect(status.edits.reload.last.media_attachments_changed).to be true
+        expect(status.edits.reload.last.poll_options).to eq %w(Foo Bar Baz)
       end
     end
 
@@ -239,10 +239,5 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do
       subject.call(status, json)
       expect(status.reload.edited_at.to_s).to eq '2021-09-08 22:39:25 UTC'
     end
-
-    it 'records that no media has been changed in edit' do
-      subject.call(status, json)
-      expect(status.edits.reload.last.media_attachments_changed).to be false
-    end
   end
 end

spec/services/update_status_service_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe UpdateStatusService, type: :service do
     end
 
     it 'saves edit history' do
-      expect(status.edits.pluck(:text, :media_attachments_changed)).to eq [['Foo', false], ['Bar', false]]
+      expect(status.edits.pluck(:text)).to eq %w(Foo Bar)
     end
   end
 
@@ -39,7 +39,7 @@ RSpec.describe UpdateStatusService, type: :service do
     end
 
     it 'saves edit history' do
-      expect(status.edits.pluck(:text, :spoiler_text, :media_attachments_changed)).to eq [['Foo', '', false], ['Foo', 'Bar', false]]
+      expect(status.edits.pluck(:text, :spoiler_text)).to eq [['Foo', ''], ['Foo', 'Bar']]
     end
   end
 
@@ -54,11 +54,11 @@ RSpec.describe UpdateStatusService, type: :service do
     end
 
     it 'updates media attachments' do
-      expect(status.media_attachments.to_a).to eq [attached_media_attachment]
+      expect(status.ordered_media_attachments).to eq [attached_media_attachment]
     end
 
-    it 'detaches detached media attachments' do
-      expect(detached_media_attachment.reload.status_id).to be_nil
+    it 'does not detach detached media attachments' do
+      expect(detached_media_attachment.reload.status_id).to eq status.id
     end
 
     it 'attaches attached media attachments' do
@@ -66,7 +66,7 @@ RSpec.describe UpdateStatusService, type: :service do
     end
 
     it 'saves edit history' do
-      expect(status.edits.pluck(:text, :media_attachments_changed)).to eq [['Foo', false], ['Foo', true]]
+      expect(status.edits.pluck(:ordered_media_attachment_ids)).to eq [[detached_media_attachment.id], [attached_media_attachment.id]]
     end
   end
 
@@ -95,7 +95,7 @@ RSpec.describe UpdateStatusService, type: :service do
     end
 
     it 'saves edit history' do
-      expect(status.edits.pluck(:text, :media_attachments_changed)).to eq [['Foo', false], ['Foo', true]]
+      expect(status.edits.pluck(:poll_options)).to eq [%w(Foo Bar), %w(Bar Baz Foo)]
     end
   end