From ec265b7d8a976cbae0cc78c74ae5374d303fbbcd Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 2 May 2023 12:23:35 -0400 Subject: [PATCH] Fix Rails/ActiveRecordCallbacksOrder cop (#24689) --- .rubocop_todo.yml | 13 ---------- app/models/account.rb | 2 +- app/models/account_conversation.rb | 3 +-- app/models/announcement_reaction.rb | 3 +-- app/models/block.rb | 2 +- app/models/media_attachment.rb | 6 ++--- app/models/session_activation.rb | 2 +- app/models/status.rb | 38 +++++++++++++++-------------- 8 files changed, 28 insertions(+), 41 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 44361fd6f0..b9111fcd5b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1114,19 +1114,6 @@ RSpec/VerifiedDoubles: - 'spec/workers/feed_insert_worker_spec.rb' - 'spec/workers/regeneration_worker_spec.rb' -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/ActiveRecordCallbacksOrder: - Exclude: - - 'app/models/account.rb' - - 'app/models/account_conversation.rb' - - 'app/models/announcement_reaction.rb' - - 'app/models/block.rb' - - 'app/models/media_attachment.rb' - - 'app/models/session_activation.rb' - - 'app/models/status.rb' - # This cop supports unsafe autocorrection (--autocorrect-all). Rails/ApplicationController: Exclude: diff --git a/app/models/account.rb b/app/models/account.rb index 7f0c2a7d29..32e9898160 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -439,9 +439,9 @@ class Account < ApplicationRecord @emojis ||= CustomEmoji.from_text(emojifiable_text, domain) end - before_create :generate_keys before_validation :prepare_contents, if: :local? before_validation :prepare_username, on: :create + before_create :generate_keys before_destroy :clean_feed_manager def ensure_keys! diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb index b3ddc04c10..be07879219 100644 --- a/app/models/account_conversation.rb +++ b/app/models/account_conversation.rb @@ -17,14 +17,13 @@ class AccountConversation < ApplicationRecord include Redisable + before_validation :set_last_status after_commit :push_to_streaming_api belongs_to :account belongs_to :conversation belongs_to :last_status, class_name: 'Status' - before_validation :set_last_status - def participant_account_ids=(arr) self[:participant_account_ids] = arr.sort end diff --git a/app/models/announcement_reaction.rb b/app/models/announcement_reaction.rb index d22771034f..9881892c4b 100644 --- a/app/models/announcement_reaction.rb +++ b/app/models/announcement_reaction.rb @@ -14,6 +14,7 @@ # class AnnouncementReaction < ApplicationRecord + before_validation :set_custom_emoji after_commit :queue_publish belongs_to :account @@ -23,8 +24,6 @@ class AnnouncementReaction < ApplicationRecord validates :name, presence: true validates_with ReactionValidator - before_validation :set_custom_emoji - private def set_custom_emoji diff --git a/app/models/block.rb b/app/models/block.rb index b42c1569b9..11156ebab3 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -25,8 +25,8 @@ class Block < ApplicationRecord false # Force uri_for to use uri attribute end - after_commit :remove_blocking_cache before_validation :set_uri, only: :create + after_commit :remove_blocking_cache private diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index e51e13b956..ee8967a9bb 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -271,12 +271,12 @@ class MediaAttachment < ApplicationRecord delay_processing? && attachment_name == :file end - after_commit :enqueue_processing, on: :create - after_commit :reset_parent_cache, on: :update - before_create :set_unknown_type before_create :set_processing + after_commit :enqueue_processing, on: :create + after_commit :reset_parent_cache, on: :update + after_post_process :set_meta class << self diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index 10c3a6c250..7f5f0d9a9a 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -36,8 +36,8 @@ class SessionActivation < ApplicationRecord detection.platform.id end - before_create :assign_access_token before_save :assign_user_agent + before_create :assign_access_token class << self def active?(id) diff --git a/app/models/status.rb b/app/models/status.rb index 990d0fc29e..f4093f85dc 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -30,8 +30,6 @@ # class Status < ApplicationRecord - before_destroy :unlink_from_conversations! - include Discard::Model include Paginable include Cacheable @@ -114,6 +112,26 @@ class Status < ApplicationRecord after_create_commit :trigger_create_webhooks after_update_commit :trigger_update_webhooks + after_create_commit :increment_counter_caches + after_destroy_commit :decrement_counter_caches + + after_create_commit :store_uri, if: :local? + after_create_commit :update_statistics, if: :local? + + before_validation :prepare_contents, if: :local? + before_validation :set_reblog + before_validation :set_visibility + before_validation :set_conversation + before_validation :set_local + + around_create Mastodon::Snowflake::Callbacks + + after_create :set_poll_id + + # The `prepend: true` option below ensures this runs before + # the `dependent: destroy` callbacks remove relevant records + before_destroy :unlink_from_conversations!, prepend: true + cache_associated :application, :media_attachments, :conversation, @@ -311,22 +329,6 @@ class Status < ApplicationRecord attributes['trendable'].nil? && account.requires_review_notification? end - after_create_commit :increment_counter_caches - after_destroy_commit :decrement_counter_caches - - after_create_commit :store_uri, if: :local? - after_create_commit :update_statistics, if: :local? - - before_validation :prepare_contents, if: :local? - before_validation :set_reblog - before_validation :set_visibility - before_validation :set_conversation - before_validation :set_local - - around_create Mastodon::Snowflake::Callbacks - - after_create :set_poll_id - class << self def selectable_visibilities visibilities.keys - %w(direct limited)