Preserve original callback execution order

This cop registers an offense when a callback appears after another
callback that will be executed later in the record's lifecycle.

The auto-correct logic currently always moves the offending callback to
before the first callback in the class. This does eventually produce the
correct result, but it's not very efficient, and can reorder callbacks
of the same type, which changes the order in which they're executed.

By moving the offending callback above the preceding one, the sort
becomes stable and the original callback execution order is preserved.

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)
 

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
 

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