diff --git a/CHANGELOG.md b/CHANGELOG.md index 22b1b76358..73b834bb65 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/devise.rb b/lib/devise.rb index 1d9370cc4f..e0749eb824 100644 --- a/lib/devise.rb +++ b/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 diff --git a/lib/devise/models.rb b/lib/devise/models.rb index 4d50fa2453..1dc5753b04 100644 --- a/lib/devise/models.rb +++ b/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| diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index 7faae516b6..0f74d07578 100644 --- a/lib/devise/models/confirmable.rb +++ b/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? diff --git a/lib/devise/models/database_authenticatable.rb b/lib/devise/models/database_authenticatable.rb index 8c0e22613d..fc6ad714e6 100644 --- a/lib/devise/models/database_authenticatable.rb +++ b/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 diff --git a/lib/devise/models/recoverable.rb b/lib/devise/models/recoverable.rb index 59f3a613d6..b17c42aae6 100644 --- a/lib/devise/models/recoverable.rb +++ b/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 diff --git a/lib/devise/models/validatable.rb b/lib/devise/models/validatable.rb index 5a190a7c36..1c22fb5fec 100644 --- a/lib/devise/models/validatable.rb +++ b/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? diff --git a/lib/devise/orm_dirty_tracking.rb b/lib/devise/orm_dirty_tracking.rb new file mode 100644 index 0000000000..07391108fb --- /dev/null +++ b/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 diff --git a/test/rails_app/lib/shared_admin.rb b/test/rails_app/lib/shared_admin.rb index 3e6362a78d..374666ff52 100644 --- a/test/rails_app/lib/shared_admin.rb +++ b/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