Merge pull request #313 from eugeneius/preserve_callback_order

Preserve original callback execution order
rubocop · Jul 30, 2020 · 30d433f · 30d433f
2 parents ecf7b32 + 9300bbe
commit 30d433f
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,14 @@
 * [#52](https://github.com/rubocop-hq/rubocop-rails/issues/52): Add new `Rails/AfterCommitOverride` cop. ([@fatkodima][])
 * [#274](https://github.com/rubocop-hq/rubocop-rails/pull/274): Add new `Rails/WhereNot` cop. ([@fatkodima][])
 
+### Bug fixes
+
+* [#313](https://github.com/rubocop-hq/rubocop-rails/pull/313): Fix `Rails/ActiveRecordCallbacksOrder` to preserve the original callback execution order. ([@eugeneius][])
+
+### Changes
+
+* [#312](https://github.com/rubocop-hq/rubocop-rails/pull/312): Mark `Rails/MailerName` as unsafe for auto-correct. ([@eugeneius][])
+
 ## 2.7.1 (2020-07-26)
 
 ### Bug fixes
@@ -18,7 +26,6 @@
 ### Changes
 
 * [#301](https://github.com/rubocop-hq/rubocop-rails/issues/301): Set disalbed by default for `Rails/PluckId`. ([@koic][])
-* [#312](https://github.com/rubocop-hq/rubocop-rails/pull/312): Mark `Rails/MailerName` as unsafe for auto-correct. ([@eugeneius][])
 
 ## 2.7.0 (2020-07-21)
 

diff --git a/lib/rubocop/cop/rails/active_record_callbacks_order.rb b/lib/rubocop/cop/rails/active_record_callbacks_order.rb
@@ -44,9 +44,7 @@ class ActiveRecordCallbacksOrder < Cop
  after_touch
  ].freeze
 
- CALLBACKS_ORDER_MAP = Hash[
- CALLBACKS_IN_ORDER.map.with_index { |name, index| [name, index] }
- ].freeze
+ CALLBACKS_ORDER_MAP = CALLBACKS_IN_ORDER.each_with_index.to_h.freeze
 
  def on_class(class_node)
  previous_index = -1
@@ -68,7 +66,7 @@ def on_class(class_node)
 
  # Autocorrect by swapping between two nodes autocorrecting them
  def autocorrect(node)
- previous = left_siblings_of(node).find do |sibling|
+ previous = left_siblings_of(node).reverse_each.find do |sibling|
  callback?(sibling)
  end
 

diff --git a/spec/rubocop/cop/rails/active_record_callbacks_order_spec.rb b/spec/rubocop/cop/rails/active_record_callbacks_order_spec.rb
@@ -83,6 +83,25 @@ class User < ApplicationRecord
  RUBY
  end
 
+ it 'preserves the original order of callbacks of the same type' do
+ expect_offense(<<~RUBY)
+ class User < ApplicationRecord
+ after_commit :after_commit_callback
+ after_save :after_save_callback1
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `after_save` is supposed to appear before `after_commit`.
+ after_save :after_save_callback2
+ end
+ RUBY
+
+ expect_correction(<<~RUBY)
+ class User < ApplicationRecord
+ after_save :after_save_callback1
+ after_save :after_save_callback2
+ after_commit :after_commit_callback
+ end
+ RUBY
+ end
+
  it 'does not register an offense when declared callbacks are correctly ordered' do
  expect_no_offenses(<<~RUBY)
  class User < ApplicationRecord