Refactor dirty tracking conditionals for different versions (#5575)

We have an number of conditions due to how dirty tracking changed around
Rails 5.1, that implement methods using one or another method call. I
might need more of this for mongo upgrades based on an initial
investigation, plus this makes the code really hard to reason about
sometimes with these many conditionals.

While I want to drop support for older versions of Rails soon, this
centralization of dirty methods that are used by devise conditionally
simplifies the usage considerably across the board, moves the version
condition to a single place, and will make it easier to refactor later
once we drop older Rails version by simply removing the `devise_*`
versions of the methods, alongside the prefix on the method calls for
the most part, since those methods follow the naming of the newer Rails
versions.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * enhancements
   * Allow resource class scopes to override the global configuration for `sign_in_after_reset_password` behaviour. [#5429](https://github.com/heartcombo/devise/pull/5429) [@mattr](https://github.com/mattr)
+  * Refactor conditional dirty tracking logic to a centralized module to simplify usage throughout the codebase. [#5575](https://github.com/heartcombo/devise/pull/5575)
 
 * bug fixes
   * Fix frozen string exception in validatable. [#5563](https://github.com/heartcombo/devise/pull/5563) [#5465](https://github.com/heartcombo/devise/pull/5465) [@mameier](https://github.com/mameier)

lib/devise.rb

@@ -13,6 +13,7 @@ module Devise
   autoload :Encryptor,          'devise/encryptor'
   autoload :FailureApp,         'devise/failure_app'
   autoload :OmniAuth,           'devise/omniauth'
+  autoload :OrmDirtyTracking,   'devise/orm_dirty_tracking'
   autoload :ParameterFilter,    'devise/parameter_filter'
   autoload :ParameterSanitizer, 'devise/parameter_sanitizer'
   autoload :TestHelpers,        'devise/test_helpers'
@@ -307,10 +308,6 @@ module Test
   mattr_accessor :sign_in_after_change_password
   @@sign_in_after_change_password = true
 
-  def self.activerecord51? # :nodoc:
-    defined?(ActiveRecord) && ActiveRecord.gem_version >= Gem::Version.new("5.1.x")
-  end
-
   # Default way to set up Devise. Run rails generate devise_install to create
   # a fresh initializer with all configuration values.
   def self.setup

lib/devise/models.rb

@@ -84,6 +84,7 @@ def devise(*modules)
       end
 
       devise_modules_hook! do
+        include Devise::OrmDirtyTracking
         include Devise::Models::Authenticatable
 
         selected_modules.each do |m|

lib/devise/models/confirmable.rb

@@ -258,44 +258,23 @@ def generate_confirmation_token!
           generate_confirmation_token && save(validate: false)
         end
 
-        if Devise.activerecord51?
-          def postpone_email_change_until_confirmation_and_regenerate_confirmation_token
-            @reconfirmation_required = true
-            self.unconfirmed_email = self.email
-            self.email = self.email_in_database
-            self.confirmation_token = nil
-            generate_confirmation_token
-          end
-        else
-          def postpone_email_change_until_confirmation_and_regenerate_confirmation_token
-            @reconfirmation_required = true
-            self.unconfirmed_email = self.email
-            self.email = self.email_was
-            self.confirmation_token = nil
-            generate_confirmation_token
-          end
+
+        def postpone_email_change_until_confirmation_and_regenerate_confirmation_token
+          @reconfirmation_required = true
+          self.unconfirmed_email = self.email
+          self.email = self.devise_email_in_database
+          self.confirmation_token = nil
+          generate_confirmation_token
         end
 
-        if Devise.activerecord51?
-          def postpone_email_change?
-            postpone = self.class.reconfirmable &&
-              will_save_change_to_email? &&
-              !@bypass_confirmation_postpone &&
-              self.email.present? &&
-              (!@skip_reconfirmation_in_callback || !self.email_in_database.nil?)
-            @bypass_confirmation_postpone = false
-            postpone
-          end
-        else
-          def postpone_email_change?
-            postpone = self.class.reconfirmable &&
-              email_changed? &&
-              !@bypass_confirmation_postpone &&
-              self.email.present? &&
-              (!@skip_reconfirmation_in_callback || !self.email_was.nil?)
-            @bypass_confirmation_postpone = false
-            postpone
-          end
+        def postpone_email_change?
+          postpone = self.class.reconfirmable &&
+            devise_will_save_change_to_email? &&
+            !@bypass_confirmation_postpone &&
+            self.email.present? &&
+            (!@skip_reconfirmation_in_callback || !self.devise_email_in_database.nil?)
+          @bypass_confirmation_postpone = false
+          postpone
         end
 
         def reconfirmation_required?

lib/devise/models/database_authenticatable.rb

@@ -177,16 +177,9 @@ def authenticatable_salt
         encrypted_password[0,29] if encrypted_password
       end
 
-      if Devise.activerecord51?
-        # Send notification to user when email changes.
-        def send_email_changed_notification
-          send_devise_notification(:email_changed, to: email_before_last_save)
-        end
-      else
-        # Send notification to user when email changes.
-        def send_email_changed_notification
-          send_devise_notification(:email_changed, to: email_was)
-        end
+      # Send notification to user when email changes.
+      def send_email_changed_notification
+        send_devise_notification(:email_changed, to: devise_email_before_last_save)
       end
 
       # Send notification to user when password changes.
@@ -205,24 +198,12 @@ def password_digest(password)
         Devise::Encryptor.digest(self.class, password)
       end
 
-      if Devise.activerecord51?
-        def send_email_changed_notification?
-          self.class.send_email_changed_notification && saved_change_to_email? && !@skip_email_changed_notification
-        end
-      else
-        def send_email_changed_notification?
-          self.class.send_email_changed_notification && email_changed? && !@skip_email_changed_notification
-        end
+      def send_email_changed_notification?
+        self.class.send_email_changed_notification && devise_saved_change_to_email? && !@skip_email_changed_notification
       end
 
-      if Devise.activerecord51?
-        def send_password_change_notification?
-          self.class.send_password_change_notification && saved_change_to_encrypted_password? && !@skip_password_change_notification
-        end
-      else
-        def send_password_change_notification?
-          self.class.send_password_change_notification && encrypted_password_changed? && !@skip_password_change_notification
-        end
+      def send_password_change_notification?
+        self.class.send_password_change_notification && devise_saved_change_to_encrypted_password? && !@skip_password_change_notification
       end
 
       module ClassMethods

lib/devise/models/recoverable.rb

@@ -99,24 +99,13 @@ def send_reset_password_instructions_notification(token)
           send_devise_notification(:reset_password_instructions, token, {})
         end
 
-        if Devise.activerecord51?
-          def clear_reset_password_token?
-            encrypted_password_changed = respond_to?(:will_save_change_to_encrypted_password?) && will_save_change_to_encrypted_password?
-            authentication_keys_changed = self.class.authentication_keys.any? do |attribute|
-              respond_to?("will_save_change_to_#{attribute}?") && send("will_save_change_to_#{attribute}?")
-            end
-
-            authentication_keys_changed || encrypted_password_changed
+        def clear_reset_password_token?
+          encrypted_password_changed = devise_respond_to_and_will_save_change_to_attribute?(:encrypted_password)
+          authentication_keys_changed = self.class.authentication_keys.any? do |attribute|
+            devise_respond_to_and_will_save_change_to_attribute?(attribute)
           end
-        else
-          def clear_reset_password_token?
-            encrypted_password_changed = respond_to?(:encrypted_password_changed?) && encrypted_password_changed?
-            authentication_keys_changed = self.class.authentication_keys.any? do |attribute|
-              respond_to?("#{attribute}_changed?") && send("#{attribute}_changed?")
-            end
 
-            authentication_keys_changed || encrypted_password_changed
-          end
+          authentication_keys_changed || encrypted_password_changed
         end
 
       module ClassMethods

lib/devise/models/validatable.rb

@@ -29,13 +29,8 @@ def self.included(base)
 
         base.class_eval do
           validates_presence_of   :email, if: :email_required?
-          if Devise.activerecord51?
-            validates_uniqueness_of :email, allow_blank: true, case_sensitive: true, if: :will_save_change_to_email?
-            validates_format_of     :email, with: email_regexp, allow_blank: true, if: :will_save_change_to_email?
-          else
-            validates_uniqueness_of :email, allow_blank: true, if: :email_changed?
-            validates_format_of     :email, with: email_regexp, allow_blank: true, if: :email_changed?
-          end
+          validates_uniqueness_of :email, allow_blank: true, case_sensitive: true, if: :devise_will_save_change_to_email?
+          validates_format_of     :email, with: email_regexp, allow_blank: true, if: :devise_will_save_change_to_email?
 
           validates_presence_of     :password, if: :password_required?
           validates_confirmation_of :password, if: :password_required?

lib/devise/orm_dirty_tracking.rb

@@ -0,0 +1,57 @@
+module Devise
+  module OrmDirtyTracking # :nodoc:
+    def self.activerecord51?
+      defined?(ActiveRecord) && ActiveRecord.gem_version >= Gem::Version.new("5.1.x")
+    end
+
+    if activerecord51?
+      def devise_email_before_last_save
+        email_before_last_save
+      end
+
+      def devise_email_in_database
+        email_in_database
+      end
+
+      def devise_saved_change_to_email?
+        saved_change_to_email?
+      end
+
+      def devise_saved_change_to_encrypted_password?
+        saved_change_to_encrypted_password?
+      end
+
+      def devise_will_save_change_to_email?
+        will_save_change_to_email?
+      end
+
+      def devise_respond_to_and_will_save_change_to_attribute?(attribute)
+        respond_to?("will_save_change_to_#{attribute}?") && send("will_save_change_to_#{attribute}?")
+      end
+    else
+      def devise_email_before_last_save
+        email_was
+      end
+
+      def devise_email_in_database
+        email_was
+      end
+
+      def devise_saved_change_to_email?
+        email_changed?
+      end
+
+      def devise_saved_change_to_encrypted_password?
+        encrypted_password_changed?
+      end
+
+      def devise_will_save_change_to_email?
+        email_changed?
+      end
+
+      def devise_respond_to_and_will_save_change_to_attribute?(attribute)
+        respond_to?("#{attribute}_changed?") && send("#{attribute}_changed?")
+      end
+    end
+  end
+end

test/rails_app/lib/shared_admin.rb

@@ -10,11 +10,7 @@ module SharedAdmin
            allow_unconfirmed_access_for: 2.weeks, reconfirmable: true
 
     validates_length_of     :reset_password_token, minimum: 3, allow_blank: true
-    if Devise::Test.rails51?
-      validates_uniqueness_of :email, allow_blank: true, if: :will_save_change_to_email?
-    else
-      validates_uniqueness_of :email, allow_blank: true, if: :email_changed?
-    end
+    validates_uniqueness_of :email, allow_blank: true, if: :devise_will_save_change_to_email?
   end
 
   def raw_confirmation_token