From 5f7083178a0a59d53b9414d3abe7a119e616c4fd Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 22 Aug 2019 21:55:56 +0200 Subject: [PATCH] Add soft delete for statuses for instant deletes through API (#11623) * Add soft delete for statuses to allow them to appear instant * Allow reporting soft-deleted statuses and show them in the admin UI * Change index for getting an account's statuses --- Gemfile | 1 + Gemfile.lock | 3 +++ app/controllers/api/v1/reports_controller.rb | 2 +- .../api/v1/statuses/reblogs_controller.rb | 3 ++- app/controllers/api/v1/statuses_controller.rb | 1 + app/models/form/status_batch.rb | 1 + app/models/report.rb | 2 +- app/models/status.rb | 6 +++++- app/views/admin/reports/_status.html.haml | 5 ++++- app/workers/removal_worker.rb | 2 +- config/locales/en.yml | 1 + .../20190819134503_add_deleted_at_to_statuses.rb | 5 +++++ db/migrate/20190820003045_update_statuses_index.rb | 13 +++++++++++++ db/schema.rb | 5 +++-- 14 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20190819134503_add_deleted_at_to_statuses.rb create mode 100644 db/migrate/20190820003045_update_statuses_index.rb diff --git a/Gemfile b/Gemfile index 250a28a3adf2e6..86dab965a7ccf4 100644 --- a/Gemfile +++ b/Gemfile @@ -43,6 +43,7 @@ gem 'omniauth-cas', '~> 1.1' gem 'omniauth-saml', '~> 1.10' gem 'omniauth', '~> 1.9' +gem 'discard', '~> 1.1' gem 'doorkeeper', '~> 5.1' gem 'fast_blank', '~> 1.0' gem 'fastimage' diff --git a/Gemfile.lock b/Gemfile.lock index 1da6d73a6830d8..b896909a4976a2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -204,6 +204,8 @@ GEM devise (>= 4.0.0) rpam2 (~> 4.0) diff-lcs (1.3) + discard (1.1.0) + activerecord (>= 4.2, < 7) docile (1.3.2) domain_name (0.5.20180417) unf (>= 0.0.5, < 1.0.0) @@ -692,6 +694,7 @@ DEPENDENCIES devise (~> 4.6) devise-two-factor (~> 3.1) devise_pam_authenticatable2 (~> 9.2) + discard (~> 1.1) doorkeeper (~> 5.1) dotenv-rails (~> 2.7) fabrication (~> 2.20) diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb index e182a9c6cf6245..1b0b4b05b2e72d 100644 --- a/app/controllers/api/v1/reports_controller.rb +++ b/app/controllers/api/v1/reports_controller.rb @@ -21,7 +21,7 @@ def create private def reported_status_ids - reported_account.statuses.find(status_ids).pluck(:id) + reported_account.statuses.with_discarded.find(status_ids).pluck(:id) end def status_ids diff --git a/app/controllers/api/v1/statuses/reblogs_controller.rb b/app/controllers/api/v1/statuses/reblogs_controller.rb index ed4f551003d670..42381a37fdbe8a 100644 --- a/app/controllers/api/v1/statuses/reblogs_controller.rb +++ b/app/controllers/api/v1/statuses/reblogs_controller.rb @@ -18,6 +18,7 @@ def destroy @reblogs_map = { @status.id => false } authorize status_for_destroy, :unreblog? + status_for_destroy.discard RemovalWorker.perform_async(status_for_destroy.id) render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, reblogs_map: @reblogs_map) @@ -30,7 +31,7 @@ def status_for_reblog end def status_for_destroy - current_user.account.statuses.where(reblog_of_id: params[:status_id]).first! + @status_for_destroy ||= current_user.account.statuses.where(reblog_of_id: params[:status_id]).first! end def reblog_params diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 39ca564821f0d2..bba3c0651beedb 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -53,6 +53,7 @@ def destroy @status = Status.where(account_id: current_user.account).find(params[:id]) authorize @status, :destroy? + @status.discard RemovalWorker.perform_async(@status.id, redraft: true) render json: @status, serializer: REST::StatusSerializer, source_requested: true diff --git a/app/models/form/status_batch.rb b/app/models/form/status_batch.rb index 831d8b7c52ffda..e09cc2594e4835 100644 --- a/app/models/form/status_batch.rb +++ b/app/models/form/status_batch.rb @@ -34,6 +34,7 @@ def change_sensitive(sensitive) def delete_statuses Status.where(id: status_ids).reorder(nil).find_each do |status| + status.discard RemovalWorker.perform_async(status.id, redraft: false) Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true) log_action :destroy, status diff --git a/app/models/report.rb b/app/models/report.rb index 5192ceef799cce..1e707ff1c4894f 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -43,7 +43,7 @@ def object_type end def statuses - Status.where(id: status_ids).includes(:account, :media_attachments, :mentions) + Status.with_discarded.where(id: status_ids).includes(:account, :media_attachments, :mentions) end def media_attachments diff --git a/app/models/status.rb b/app/models/status.rb index a47d31a8684d2d..071c5daa86fbf0 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -22,15 +22,19 @@ # application_id :bigint(8) # in_reply_to_account_id :bigint(8) # poll_id :bigint(8) +# deleted_at :datetime # class Status < ApplicationRecord before_destroy :unlink_from_conversations + include Discard::Model include Paginable include Cacheable include StatusThreadingConcern + self.discard_column = :deleted_at + # If `override_timestamps` is set at creation time, Snowflake ID creation # will be based on current time instead of `created_at` attr_accessor :override_timestamps @@ -72,7 +76,7 @@ class Status < ApplicationRecord accepts_nested_attributes_for :poll - default_scope { recent } + default_scope { recent.kept } scope :recent, -> { reorder(id: :desc) } scope :remote, -> { where(local: false).where.not(uri: nil) } diff --git a/app/views/admin/reports/_status.html.haml b/app/views/admin/reports/_status.html.haml index 2b0a53e46c372b..2febf3268e2d00 100644 --- a/app/views/admin/reports/_status.html.haml +++ b/app/views/admin/reports/_status.html.haml @@ -16,11 +16,14 @@ - video = status.proper.media_attachments.first = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), sensitive: !current_account&.user&.show_all_media? && status.proper.sensitive? || current_account&.user&.hide_all_media?, width: 610, height: 343, inline: true, alt: video.description - else - = react_component :media_gallery, height: 343, sensitive: !current_account&.user&.show_all_media? && status.sensitive? || current_account&.user&.hide_all_media?, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, media: status.proper.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } + = react_component :media_gallery, height: 343, sensitive: !current_account&.user&.show_all_media? && status.proper.sensitive? || current_account&.user&.hide_all_media?, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, media: status.proper.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } .detailed-status__meta = link_to ActivityPub::TagManager.instance.url_for(status), class: 'detailed-status__datetime', target: stream_link_target, rel: 'noopener' do %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at) + - if status.discarded? + · + %span.negative-hint= t('admin.statuses.deleted') · - if status.reblog? = fa_icon('retweet fw') diff --git a/app/workers/removal_worker.rb b/app/workers/removal_worker.rb index 14423a4fb569bb..2a1eaa89b061d3 100644 --- a/app/workers/removal_worker.rb +++ b/app/workers/removal_worker.rb @@ -4,7 +4,7 @@ class RemovalWorker include Sidekiq::Worker def perform(status_id, options = {}) - RemoveStatusService.new.call(Status.find(status_id), **options.symbolize_keys) + RemoveStatusService.new.call(Status.with_discarded.find(status_id), **options.symbolize_keys) rescue ActiveRecord::RecordNotFound true end diff --git a/config/locales/en.yml b/config/locales/en.yml index 32074b0c6dec6a..86743b379ffded 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -500,6 +500,7 @@ en: delete: Delete nsfw_off: Mark as not sensitive nsfw_on: Mark as sensitive + deleted: Deleted failed_to_execute: Failed to execute media: title: Media diff --git a/db/migrate/20190819134503_add_deleted_at_to_statuses.rb b/db/migrate/20190819134503_add_deleted_at_to_statuses.rb new file mode 100644 index 00000000000000..5af109097e8b10 --- /dev/null +++ b/db/migrate/20190819134503_add_deleted_at_to_statuses.rb @@ -0,0 +1,5 @@ +class AddDeletedAtToStatuses < ActiveRecord::Migration[5.2] + def change + add_column :statuses, :deleted_at, :datetime + end +end diff --git a/db/migrate/20190820003045_update_statuses_index.rb b/db/migrate/20190820003045_update_statuses_index.rb new file mode 100644 index 00000000000000..5c2ea1f6a24164 --- /dev/null +++ b/db/migrate/20190820003045_update_statuses_index.rb @@ -0,0 +1,13 @@ +class UpdateStatusesIndex < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + safety_assured { add_index :statuses, [:account_id, :id, :visibility, :updated_at], where: 'deleted_at IS NULL', order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20190820 } + remove_index :statuses, name: :index_statuses_20180106 + end + + def down + safety_assured { add_index :statuses, [:account_id, :id, :visibility, :updated_at], order: { id: :desc }, algorithm: :concurrently, name: :index_statuses_20180106 } + remove_index :statuses, name: :index_statuses_20190820 + end +end diff --git a/db/schema.rb b/db/schema.rb index 1dc8121ccb54bf..eb48f672d85e76 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_08_15_225426) do +ActiveRecord::Schema.define(version: 2019_08_20_003045) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -645,7 +645,8 @@ t.bigint "application_id" t.bigint "in_reply_to_account_id" t.bigint "poll_id" - t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20180106", order: { id: :desc } + t.datetime "deleted_at" + t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20190820", order: { id: :desc }, where: "(deleted_at IS NULL)" t.index ["in_reply_to_account_id"], name: "index_statuses_on_in_reply_to_account_id" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id" t.index ["reblog_of_id", "account_id"], name: "index_statuses_on_reblog_of_id_and_account_id"