From bb0efe16e62636895079b3a968e444cd6e80b467 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 30 Nov 2023 08:30:35 -0500 Subject: [PATCH] Remove `default_scope` from `MediaAttachment` class (#28043) --- app/controllers/accounts_controller.rb | 2 +- app/lib/account_statuses_filter.rb | 2 +- app/lib/vacuum/media_attachments_vacuum.rb | 4 ++-- app/models/admin/status_filter.rb | 2 +- app/models/media_attachment.rb | 9 ++++----- app/services/backup_service.rb | 2 +- app/services/clear_domain_media_service.rb | 2 +- app/services/delete_account_service.rb | 2 +- app/services/suspend_account_service.rb | 2 +- app/services/unsuspend_account_service.rb | 2 +- .../activitypub/process_status_update_service_spec.rb | 2 +- 11 files changed, 15 insertions(+), 16 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 850bf881f..4e475fe78 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -50,7 +50,7 @@ class AccountsController < ApplicationController end def only_media_scope - Status.joins(:media_attachments).merge(@account.media_attachments.reorder(nil)).group(:id) + Status.joins(:media_attachments).merge(@account.media_attachments).group(:id) end def no_replies_scope diff --git a/app/lib/account_statuses_filter.rb b/app/lib/account_statuses_filter.rb index b34ebb477..d1365de58 100644 --- a/app/lib/account_statuses_filter.rb +++ b/app/lib/account_statuses_filter.rb @@ -70,7 +70,7 @@ class AccountStatusesFilter end def only_media_scope - Status.joins(:media_attachments).merge(account.media_attachments.reorder(nil)).group(Status.arel_table[:id]) + Status.joins(:media_attachments).merge(account.media_attachments).group(Status.arel_table[:id]) end def no_replies_scope diff --git a/app/lib/vacuum/media_attachments_vacuum.rb b/app/lib/vacuum/media_attachments_vacuum.rb index 7b21c84bb..ab7ea4092 100644 --- a/app/lib/vacuum/media_attachments_vacuum.rb +++ b/app/lib/vacuum/media_attachments_vacuum.rb @@ -27,11 +27,11 @@ class Vacuum::MediaAttachmentsVacuum end def media_attachments_past_retention_period - MediaAttachment.unscoped.remote.cached.where(MediaAttachment.arel_table[:created_at].lt(@retention_period.ago)).where(MediaAttachment.arel_table[:updated_at].lt(@retention_period.ago)) + MediaAttachment.remote.cached.where(MediaAttachment.arel_table[:created_at].lt(@retention_period.ago)).where(MediaAttachment.arel_table[:updated_at].lt(@retention_period.ago)) end def orphaned_media_attachments - MediaAttachment.unscoped.unattached.where(MediaAttachment.arel_table[:created_at].lt(TTL.ago)) + MediaAttachment.unattached.where(MediaAttachment.arel_table[:created_at].lt(TTL.ago)) end def retention_period? diff --git a/app/models/admin/status_filter.rb b/app/models/admin/status_filter.rb index 645c2e620..4708785e7 100644 --- a/app/models/admin/status_filter.rb +++ b/app/models/admin/status_filter.rb @@ -32,7 +32,7 @@ class Admin::StatusFilter def scope_for(key, _value) case key.to_s when 'media' - Status.joins(:media_attachments).merge(@account.media_attachments.reorder(nil)).group(:id).reorder('statuses.id desc') + Status.joins(:media_attachments).merge(@account.media_attachments).group(:id).reorder('statuses.id desc') else raise Mastodon::InvalidParameterError, "Unknown filter: #{key}" end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index b567003fb..1f40e5725 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -205,12 +205,11 @@ class MediaAttachment < ApplicationRecord validates :thumbnail, absence: true, if: -> { local? && !audio_or_video? } scope :attached, -> { where.not(status_id: nil).or(where.not(scheduled_status_id: nil)) } - scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) } - scope :local, -> { where(remote_url: '') } - scope :remote, -> { where.not(remote_url: '') } scope :cached, -> { remote.where.not(file_file_name: nil) } - - default_scope { order(id: :asc) } + scope :local, -> { where(remote_url: '') } + scope :ordered, -> { order(id: :asc) } + scope :remote, -> { where.not(remote_url: '') } + scope :unattached, -> { where(status_id: nil, scheduled_status_id: nil) } attr_accessor :skip_download diff --git a/app/services/backup_service.rb b/app/services/backup_service.rb index 3ef0366c3..886bab1eb 100644 --- a/app/services/backup_service.rb +++ b/app/services/backup_service.rb @@ -72,7 +72,7 @@ class BackupService < BaseService end def dump_media_attachments!(zipfile) - MediaAttachment.attached.where(account: account).reorder(nil).find_in_batches do |media_attachments| + MediaAttachment.attached.where(account: account).find_in_batches do |media_attachments| media_attachments.each do |m| path = m.file&.path next unless path diff --git a/app/services/clear_domain_media_service.rb b/app/services/clear_domain_media_service.rb index 7bf2d62fb..d3ad43e70 100644 --- a/app/services/clear_domain_media_service.rb +++ b/app/services/clear_domain_media_service.rb @@ -43,7 +43,7 @@ class ClearDomainMediaService < BaseService end def media_from_blocked_domain - MediaAttachment.joins(:account).merge(blocked_domain_accounts).reorder(nil) + MediaAttachment.joins(:account).merge(blocked_domain_accounts) end def emojis_from_blocked_domains diff --git a/app/services/delete_account_service.rb b/app/services/delete_account_service.rb index 190a72e5c..7c7cb97df 100644 --- a/app/services/delete_account_service.rb +++ b/app/services/delete_account_service.rb @@ -165,7 +165,7 @@ class DeleteAccountService < BaseService end def purge_media_attachments! - @account.media_attachments.reorder(nil).find_each do |media_attachment| + @account.media_attachments.find_each do |media_attachment| next if keep_account_record? && reported_status_ids.include?(media_attachment.status_id) media_attachment.destroy diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index e79c2d3d8..8d5446f1a 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -65,7 +65,7 @@ class SuspendAccountService < BaseService def privatize_media_attachments! attachment_names = MediaAttachment.attachment_definitions.keys - @account.media_attachments.reorder(nil).find_each do |media_attachment| + @account.media_attachments.find_each do |media_attachment| attachment_names.each do |attachment_name| attachment = media_attachment.public_send(attachment_name) styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index 93cd04a94..652dd6a84 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -61,7 +61,7 @@ class UnsuspendAccountService < BaseService def publish_media_attachments! attachment_names = MediaAttachment.attachment_definitions.keys - @account.media_attachments.reorder(nil).find_each do |media_attachment| + @account.media_attachments.find_each do |media_attachment| attachment_names.each do |attachment_name| attachment = media_attachment.public_send(attachment_name) styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 9d91f31cc..53cbaf4cc 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -384,7 +384,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService, type: :service do end it 'updates the existing media attachment in-place' do - media_attachment = status.media_attachments.reload.first + media_attachment = status.media_attachments.ordered.reload.first expect(media_attachment).to_not be_nil expect(media_attachment.remote_url).to eq 'https://example.com/foo.png'