From a2f02a07758c32f0dcc6388b4f30ca5a84e762f3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 15 Jan 2024 08:46:47 -0500 Subject: [PATCH] Disable `Rails/SkipsModelValidations` cop (#28712) --- .rubocop.yml | 5 +-- .rubocop_todo.yml | 35 ------------------- .../admin/system_check/media_privacy_check.rb | 2 +- app/lib/attachment_batch.rb | 2 +- app/models/bulk_import.rb | 4 +-- app/models/follow_request.rb | 2 +- app/models/form/import.rb | 2 +- spec/services/reblog_service_spec.rb | 2 +- spec/workers/redownload_avatar_worker_spec.rb | 2 +- spec/workers/redownload_header_worker_spec.rb | 2 +- 10 files changed, 12 insertions(+), 46 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a06621d660..fcdc4e06cb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -109,9 +109,10 @@ Rails/LexicallyScopedActionFilter: Exclude: - 'app/controllers/auth/*' +# Reason: There are appropriate times to use these features +# https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsskipsmodelvalidations Rails/SkipsModelValidations: - Exclude: - - 'db/*migrate/**/*' + Enabled: false # Reason: We want to preserve the ability to migrate from arbitrary old versions, # and cannot guarantee that every installation has run every migration as they upgrade. diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 602d99c9f0..2c2366c9cf 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -80,41 +80,6 @@ Rails/RakeEnvironment: - 'lib/tasks/repo.rake' - 'lib/tasks/statistics.rake' -# Configuration parameters: ForbiddenMethods, AllowedMethods. -# ForbiddenMethods: decrement!, decrement_counter, increment!, increment_counter, insert, insert!, insert_all, insert_all!, toggle!, touch, touch_all, update_all, update_attribute, update_column, update_columns, update_counters, upsert, upsert_all -Rails/SkipsModelValidations: - Exclude: - - 'app/controllers/admin/invites_controller.rb' - - 'app/controllers/concerns/session_tracking_concern.rb' - - 'app/models/concerns/account/merging.rb' - - 'app/models/concerns/expireable.rb' - - 'app/models/status.rb' - - 'app/models/trends/links.rb' - - 'app/models/trends/preview_card_batch.rb' - - 'app/models/trends/preview_card_provider_batch.rb' - - 'app/models/trends/status_batch.rb' - - 'app/models/trends/statuses.rb' - - 'app/models/trends/tag_batch.rb' - - 'app/models/trends/tags.rb' - - 'app/models/user.rb' - - 'app/services/activitypub/process_status_update_service.rb' - - 'app/services/approve_appeal_service.rb' - - 'app/services/block_domain_service.rb' - - 'app/services/delete_account_service.rb' - - 'app/services/process_mentions_service.rb' - - 'app/services/unallow_domain_service.rb' - - 'app/services/unblock_domain_service.rb' - - 'app/services/update_status_service.rb' - - 'app/workers/activitypub/post_upgrade_worker.rb' - - 'app/workers/move_worker.rb' - - 'app/workers/scheduler/ip_cleanup_scheduler.rb' - - 'app/workers/scheduler/scheduled_statuses_scheduler.rb' - - 'lib/mastodon/cli/accounts.rb' - - 'lib/mastodon/cli/maintenance.rb' - - 'spec/lib/activitypub/activity/follow_spec.rb' - - 'spec/services/follow_service_spec.rb' - - 'spec/services/update_account_service_spec.rb' - # Configuration parameters: Include. # Include: app/models/**/*.rb Rails/UniqueValidationWithoutIndex: diff --git a/app/lib/admin/system_check/media_privacy_check.rb b/app/lib/admin/system_check/media_privacy_check.rb index 1df05b120e..2ddc8e8b07 100644 --- a/app/lib/admin/system_check/media_privacy_check.rb +++ b/app/lib/admin/system_check/media_privacy_check.rb @@ -78,7 +78,7 @@ class Admin::SystemCheck::MediaPrivacyCheck < Admin::SystemCheck::BaseCheck @media_attachment ||= begin attachment = Account.representative.media_attachments.first if attachment.present? - attachment.touch # rubocop:disable Rails/SkipsModelValidations + attachment.touch attachment else create_test_attachment! diff --git a/app/lib/attachment_batch.rb b/app/lib/attachment_batch.rb index b28f5c3d7f..32ccb0b13c 100644 --- a/app/lib/attachment_batch.rb +++ b/app/lib/attachment_batch.rb @@ -37,7 +37,7 @@ class AttachmentBatch def clear remove_files - batch.update_all(nullified_attributes) # rubocop:disable Rails/SkipsModelValidations + batch.update_all(nullified_attributes) end private diff --git a/app/models/bulk_import.rb b/app/models/bulk_import.rb index 810e471849..406fb2aba2 100644 --- a/app/models/bulk_import.rb +++ b/app/models/bulk_import.rb @@ -44,8 +44,8 @@ class BulkImport < ApplicationRecord def self.progress!(bulk_import_id, imported: false) # Use `increment_counter` so that the incrementation is done atomically in the database - BulkImport.increment_counter(:processed_items, bulk_import_id) # rubocop:disable Rails/SkipsModelValidations - BulkImport.increment_counter(:imported_items, bulk_import_id) if imported # rubocop:disable Rails/SkipsModelValidations + BulkImport.increment_counter(:processed_items, bulk_import_id) + BulkImport.increment_counter(:imported_items, bulk_import_id) if imported # Since the incrementation has been done atomically, concurrent access to `bulk_import` is now bening bulk_import = BulkImport.find(bulk_import_id) diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 3c5e8f96f0..c13cc718d8 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -33,7 +33,7 @@ class FollowRequest < ApplicationRecord def authorize! follow = account.follow!(target_account, reblogs: show_reblogs, notify: notify, languages: languages, uri: uri, bypass_limit: true) - ListAccount.where(follow_request: self).update_all(follow_request_id: nil, follow_id: follow.id) # rubocop:disable Rails/SkipsModelValidations + ListAccount.where(follow_request: self).update_all(follow_request_id: nil, follow_id: follow.id) MergeWorker.perform_async(target_account.id, account.id) if account.local? destroy! end diff --git a/app/models/form/import.rb b/app/models/form/import.rb index 712acf3706..fc83d9c58c 100644 --- a/app/models/form/import.rb +++ b/app/models/form/import.rb @@ -69,7 +69,7 @@ class Form::Import ApplicationRecord.transaction do now = Time.now.utc @bulk_import = current_account.bulk_imports.create(type: type, overwrite: overwrite || false, state: :unconfirmed, original_filename: data.original_filename, likely_mismatched: likely_mismatched?) - nb_items = BulkImportRow.insert_all(parsed_rows.map { |row| { bulk_import_id: bulk_import.id, data: row, created_at: now, updated_at: now } }).length # rubocop:disable Rails/SkipsModelValidations + nb_items = BulkImportRow.insert_all(parsed_rows.map { |row| { bulk_import_id: bulk_import.id, data: row, created_at: now, updated_at: now } }).length @bulk_import.update(total_items: nb_items) end end diff --git a/spec/services/reblog_service_spec.rb b/spec/services/reblog_service_spec.rb index 357b315af0..e5d0a2d6ce 100644 --- a/spec/services/reblog_service_spec.rb +++ b/spec/services/reblog_service_spec.rb @@ -46,7 +46,7 @@ RSpec.describe ReblogService, type: :service do Status .where(id: reblog_of_id) .where(text: 'discard-status-text') - .update_all(deleted_at: Time.now.utc) # rubocop:disable Rails/SkipsModelValidations + .update_all(deleted_at: Time.now.utc) end end end diff --git a/spec/workers/redownload_avatar_worker_spec.rb b/spec/workers/redownload_avatar_worker_spec.rb index 4ab368e12f..6ef320bc4f 100644 --- a/spec/workers/redownload_avatar_worker_spec.rb +++ b/spec/workers/redownload_avatar_worker_spec.rb @@ -41,7 +41,7 @@ describe RedownloadAvatarWorker do it 'reprocesses a remote avatar' do stub_request(:get, 'https://example.host/file').to_return request_fixture('avatar.txt') account = Fabricate(:account, avatar_remote_url: 'https://example.host/file') - account.update_column(:avatar_file_name, nil) # rubocop:disable Rails/SkipsModelValidations + account.update_column(:avatar_file_name, nil) result = worker.perform(account.id) diff --git a/spec/workers/redownload_header_worker_spec.rb b/spec/workers/redownload_header_worker_spec.rb index 3b6f497bb8..746c1a63ff 100644 --- a/spec/workers/redownload_header_worker_spec.rb +++ b/spec/workers/redownload_header_worker_spec.rb @@ -41,7 +41,7 @@ describe RedownloadHeaderWorker do it 'reprocesses a remote header' do stub_request(:get, 'https://example.host/file').to_return request_fixture('avatar.txt') account = Fabricate(:account, header_remote_url: 'https://example.host/file') - account.update_column(:header_file_name, nil) # rubocop:disable Rails/SkipsModelValidations + account.update_column(:header_file_name, nil) result = worker.perform(account.id)