Add support for editing for published statuses (mastodon#16697)

* Add support for editing for published statuses

* Fix references to stripped-out code

* Various fixes and improvements

* Further fixes and improvements

* Fix updates being potentially sent to unauthorized recipients

* Various fixes and improvements

* Fix wrong words in test

* Fix notifying accounts that were tagged but were not in the audience

* Fix mistake

app/controllers/api/v1/statuses/histories_controller.rb

@@ -7,7 +7,7 @@ class Api::V1::Statuses::HistoriesController < Api::BaseController
   before_action :set_status
 
   def show
-    render json: @status.edits.includes(:account, status: [:account]), each_serializer: REST::StatusEditSerializer
+    render json: @status.edits, each_serializer: REST::StatusEditSerializer
   end
 
   private

app/javascript/mastodon/components/status.js

@@ -56,7 +56,7 @@ const messages = defineMessages({
   public_short: { id: 'privacy.public.short', defaultMessage: 'Public' },
   unlisted_short: { id: 'privacy.unlisted.short', defaultMessage: 'Unlisted' },
   private_short: { id: 'privacy.private.short', defaultMessage: 'Followers-only' },
-  direct_short: { id: 'privacy.direct.short', defaultMessage: 'Mentioned people only' },
+  direct_short: { id: 'privacy.direct.short', defaultMessage: 'Direct' },
   edited: { id: 'status.edited', defaultMessage: 'Edited {date}' },
 });
 

app/javascript/mastodon/features/status/components/detailed_status.js

@@ -6,7 +6,7 @@ import DisplayName from '../../../components/display_name';
 import StatusContent from '../../../components/status_content';
 import MediaGallery from '../../../components/media_gallery';
 import { Link } from 'react-router-dom';
-import { injectIntl, defineMessages, FormattedDate } from 'react-intl';
+import { injectIntl, defineMessages, FormattedDate, FormattedMessage } from 'react-intl';
 import Card from './card';
 import ImmutablePureComponent from 'react-immutable-pure-component';
 import Video from '../../video';
@@ -243,7 +243,7 @@ class DetailedStatus extends ImmutablePureComponent {
       edited = (
         <React.Fragment>
           <React.Fragment> · </React.Fragment>
-          <EditedTimestamp statusId={status.get('id')} timestamp={status.get('edited_at')} />
+          <FormattedMessage id='status.edited' defaultMessage='Edited {date}' values={{ date: intl.formatDate(status.get('edited_at'), { hour12: false, month: 'short', day: '2-digit', hour: '2-digit', minute: '2-digit' }) }} />
         </React.Fragment>
       );
     }

app/lib/activitypub/activity.rb

@@ -86,49 +86,6 @@ def converted_object_type?
     equals_or_includes_any?(@object['type'], CONVERTED_TYPES)
   end
 
-  def distribute(status)
-    crawl_links(status)
-
-    notify_about_reblog(status) if reblog_of_local_account?(status) && !reblog_by_following_group_account?(status)
-    notify_about_mentions(status)
-
-    # Only continue if the status is supposed to have arrived in real-time.
-    # Note that if @options[:override_timestamps] isn't set, the status
-    # may have a lower snowflake id than other existing statuses, potentially
-    # "hiding" it from paginated API calls
-    return unless @options[:override_timestamps] || status.within_realtime_window?
-
-    distribute_to_followers(status)
-  end
-
-  def reblog_of_local_account?(status)
-    status.reblog? && status.reblog.account.local?
-  end
-
-  def reblog_by_following_group_account?(status)
-    status.reblog? && status.account.group? && status.reblog.account.following?(status.account)
-  end
-
-  def notify_about_reblog(status)
-    NotifyService.new.call(status.reblog.account, :reblog, status)
-  end
-
-  def notify_about_mentions(status)
-    status.active_mentions.includes(:account).each do |mention|
-      next unless mention.account.local? && audience_includes?(mention.account)
-      NotifyService.new.call(mention.account, :mention, mention)
-    end
-  end
-
-  def crawl_links(status)
-    # Spread out crawling randomly to avoid DDoSing the link
-    LinkCrawlWorker.perform_in(rand(1..59).seconds, status.id)
-  end
-
-  def distribute_to_followers(status)
-    ::DistributionWorker.perform_async(status.id)
-  end
-
   def delete_arrived_first?(uri)
     redis.exists?("delete_upon_arrival:#{@account.id}:#{uri}")
   end

app/lib/activitypub/activity/announce.rb

@@ -36,7 +36,7 @@ def perform
 
   def distribute
     # Notify the author of the original status if that status is local
-    LocalNotificationWorker.perform_async(@status.reblog.account_id, @status.id, 'Status', 'reblog') if reblog_of_local_account?(@status) && !reblog_by_following_group_account?(@status)
+    NotifyService.new.call(@status.reblog.account, :reblog, @status) if reblog_of_local_account?(@status) && !reblog_by_following_group_account?(@status)
 
     # Distribute into home and list feeds
     ::DistributionWorker.perform_async(@status.id) if @options[:override_timestamps] || @status.within_realtime_window?

app/lib/activitypub/activity/create.rb

@@ -96,7 +96,7 @@ def distribute
     LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id)
 
     # Distribute into home and list feeds and notify mentioned accounts
-    ::DistributionWorker.perform_async(@status.id, { 'silenced_account_ids' => @silenced_account_ids }) if @options[:override_timestamps] || @status.within_realtime_window?
+    ::DistributionWorker.perform_async(@status.id, silenced_account_ids: @silenced_account_ids) if @options[:override_timestamps] || @status.within_realtime_window?
   end
 
   def find_existing_status
@@ -114,10 +114,10 @@ def process_status_params
         url: @status_parser.url || @status_parser.uri,
         account: @account,
         text: converted_object_type? ? converted_text : (@status_parser.text || ''),
-        language: @status_parser.language,
+        language: @status_parser.language || detected_language,
         spoiler_text: converted_object_type? ? '' : (@status_parser.spoiler_text || ''),
         created_at: @status_parser.created_at,
-        edited_at: @status_parser.edited_at && @status_parser.edited_at != @status_parser.created_at ? @status_parser.edited_at : nil,
+        edited_at: @status_parser.edited_at,
         override_timestamps: @options[:override_timestamps],
         reply: @status_parser.reply,
         sensitive: @account.sensitized? || @status_parser.sensitive || false,
@@ -177,6 +177,10 @@ def delivered_to_account
     @delivered_to_account ||= Account.find(@options[:delivered_to_account_id])
   end
 
+  def delivered_to_account
+    @delivered_to_account ||= Account.find(@options[:delivered_to_account_id])
+  end
+
   def attach_tags(status)
     @tags.each do |tag|
       status.tags << tag
@@ -369,7 +373,11 @@ def in_reply_to_uri
   end
 
   def converted_text
-    linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n"))
+    Formatter.instance.linkify([@status_parser.title.presence, @status_parser.spoiler_text.presence, @status_parser.url || @status_parser.uri].compact.join("\n\n"))
+  end
+
+  def detected_language
+    LanguageDetector.instance.detect(@status_parser.text, @account) if supported_object_type?
   end
 
   def unsupported_media_type?(mime_type)

app/lib/activitypub/activity/update.rb

@@ -8,8 +8,6 @@ def perform
       update_account
     elsif equals_or_includes_any?(@object['type'], %w(Note Question))
       update_status
-    elsif converted_object_type?
-      Status.find_by(uri: object_uri, account_id: @account.id)
     end
   end
 
@@ -22,12 +20,12 @@ def update_account
   end
 
   def update_status
-    return reject_payload! if invalid_origin?(object_uri)
+    return reject_payload! if invalid_origin?(@object['id'])
 
-    @status = Status.find_by(uri: object_uri, account_id: @account.id)
+    status = Status.find_by(uri: object_uri, account_id: @account.id)
 
-    return if @status.nil?
+    return if status.nil?
 
-    ActivityPub::ProcessStatusUpdateService.new.call(@status, @object)
+    ActivityPub::ProcessStatusUpdateService.new.call(status, @object)
   end
 end

app/lib/activitypub/parser/media_attachment_parser.rb

@@ -27,9 +27,7 @@ def thumbnail_remote_url
   end
 
   def description
-    str = @json['summary'].presence || @json['name'].presence
-    str = str.strip[0...MediaAttachment::MAX_DESCRIPTION_LENGTH] if str.present?
-    str
+    @json['summary'].presence || @json['name'].presence
   end
 
   def focus

app/lib/feed_manager.rb

@@ -59,7 +59,7 @@ def push_to_home(account, status, update: false)
     return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?)
 
     trim(:home, account.id)
-    PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", { 'update' => update }) if push_update_required?("timeline:#{account.id}")
+    PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", update: update) if push_update_required?("timeline:#{account.id}")
     true
   end
 
@@ -84,7 +84,7 @@ def push_to_list(list, status, update: false)
     return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?)
 
     trim(:list, list.id)
-    PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", { 'update' => update }) if push_update_required?("timeline:list:#{list.id}")
+    PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", update: update) if push_update_required?("timeline:list:#{list.id}")
     true
   end
 

app/models/status.rb

@@ -3,29 +3,27 @@
 #
 # Table name: statuses
 #
-#  id                           :bigint(8)        not null, primary key
-#  uri                          :string
-#  text                         :text             default(""), not null
-#  created_at                   :datetime         not null
-#  updated_at                   :datetime         not null
-#  in_reply_to_id               :bigint(8)
-#  reblog_of_id                 :bigint(8)
-#  url                          :string
-#  sensitive                    :boolean          default(FALSE), not null
-#  visibility                   :integer          default("public"), not null
-#  spoiler_text                 :text             default(""), not null
-#  reply                        :boolean          default(FALSE), not null
-#  language                     :string
-#  conversation_id              :bigint(8)
-#  local                        :boolean
-#  account_id                   :bigint(8)        not null
-#  application_id               :bigint(8)
-#  in_reply_to_account_id       :bigint(8)
-#  poll_id                      :bigint(8)
-#  deleted_at                   :datetime
-#  edited_at                    :datetime
-#  trendable                    :boolean
-#  ordered_media_attachment_ids :bigint(8)        is an Array
+#  id                     :bigint(8)        not null, primary key
+#  uri                    :string
+#  text                   :text             default(""), not null
+#  created_at             :datetime         not null
+#  updated_at             :datetime         not null
+#  in_reply_to_id         :bigint(8)
+#  reblog_of_id           :bigint(8)
+#  url                    :string
+#  sensitive              :boolean          default(FALSE), not null
+#  visibility             :integer          default("public"), not null
+#  spoiler_text           :text             default(""), not null
+#  reply                  :boolean          default(FALSE), not null
+#  language               :string
+#  conversation_id        :bigint(8)
+#  local                  :boolean
+#  account_id             :bigint(8)        not null
+#  application_id         :bigint(8)
+#  in_reply_to_account_id :bigint(8)
+#  poll_id                :bigint(8)
+#  deleted_at             :datetime
+#  edited_at              :datetime
 #
 
 class Status < ApplicationRecord
@@ -60,6 +58,8 @@ class Status < ApplicationRecord
   belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true
   belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true
 
+  has_many :edits, class_name: 'StatusEdit', inverse_of: :status, dependent: :destroy
+
   has_many :favourites, inverse_of: :status, dependent: :destroy
   has_many :bookmarks, inverse_of: :status, dependent: :destroy
   has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
@@ -222,6 +222,10 @@ def distributable?
     public_visibility? || unlisted_visibility?
   end
 
+  def edited?
+    edited_at.present?
+  end
+
   alias sign? distributable?
 
   def with_media?

app/models/status_edit.rb

@@ -3,60 +3,21 @@
 #
 # Table name: status_edits
 #
-#  id                           :bigint(8)        not null, primary key
-#  status_id                    :bigint(8)        not null
-#  account_id                   :bigint(8)
-#  text                         :text             default(""), not null
-#  spoiler_text                 :text             default(""), 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
+#  id                        :bigint(8)        not null, primary key
+#  status_id                 :bigint(8)        not null
+#  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
 #
 
 class StatusEdit < ApplicationRecord
-  include RateLimitable
-
-  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,
-             :not_processed?, :needs_redownload?, :local?,
-             :file, :thumbnail, :thumbnail_remote_url,
-             :shortcode, to: :media_attachment
-  end
-
-  rate_limit by: :account, family: :statuses
-
   belongs_to :status
   belongs_to :account, optional: true
 
   default_scope { order(id: :asc) }
 
   delegate :local?, to: :status
-
-  def emojis
-    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/rest/status_edit_serializer.rb

@@ -1,22 +1,6 @@
 # frozen_string_literal: true
 
 class REST::StatusEditSerializer < ActiveModel::Serializer
-  include FormattingHelper
-
-  has_one :account, serializer: REST::AccountSerializer
-
-  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
-    status_content_format(object)
-  end
-
-  def poll
-    { options: object.poll_options.map { |title| { title: title } } }
-  end
+  attributes :text, :spoiler_text, :media_attachments_changed,
+             :created_at
 end