Merge pull request heartcombo#3570 from nviennot/no_more_bang

Removes the bang in confirm! and reset_password!

CHANGELOG.md

@@ -13,6 +13,10 @@
   * Controllers inheriting from any Devise core controller will now use appropriate translations.
     The i18n scope can be overridden in `translation_scope`.
 
+* deprecations
+  * `confirm!` has been deprecated in favor of `confirm`.
+  * `reset_password!` has been deprecated in favor of `reset_password`.
+
 ### 3.4.1 - 2014-10-29
 
 * enhancements

lib/devise/models/confirmable.rb

@@ -24,7 +24,7 @@ module Models
     #
     # == Examples
     #
-    #   User.find(1).confirm!      # returns true unless it's already confirmed
+    #   User.find(1).confirm       # returns true unless it's already confirmed
     #   User.find(1).confirmed?    # true/false
     #   User.find(1).send_confirmation_instructions # manually send instructions
     #
@@ -56,7 +56,7 @@ def self.required_fields(klass)
       # Confirm a user by setting it's confirmed_at to actual time. If the user
       # is already confirmed, add an error to email field. If the user is invalid
       # add errors
-      def confirm!(args={})
+      def confirm(args={})
         pending_any_confirmation do
           if confirmation_period_expired?
             self.errors.add(:email, :confirmation_period_expired,
@@ -82,6 +82,11 @@ def confirm!(args={})
         end
       end
 
+      def confirm!(args={})
+        ActiveSupport::Deprecation.warn "confirm! is deprecated in favor of confirm"
+        confirm(args)
+      end
+
       # Verifies whether a user is confirmed or not
       def confirmed?
         !!confirmed_at
@@ -284,7 +289,7 @@ def confirm_by_token(confirmation_token)
           confirmation_token = Devise.token_generator.digest(self, :confirmation_token, confirmation_token)
 
           confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_token)
-          confirmable.confirm! if confirmable.persisted?
+          confirmable.confirm if confirmable.persisted?
           confirmable.confirmation_token = original_token
           confirmable
         end

lib/devise/models/recoverable.rb

@@ -14,7 +14,7 @@ module Models
     # == Examples
     #
     #   # resets the user password and save the record, true if valid passwords are given, otherwise false
-    #   User.find(1).reset_password!('password123', 'password123')
+    #   User.find(1).reset_password('password123', 'password123')
     #
     #   # only resets the user password, without saving the record
     #   user = User.find(1)
@@ -32,7 +32,7 @@ def self.required_fields(klass)
 
       # Update password saving the record and clearing token. Returns true if
       # the passwords are valid and the record was saved, false otherwise.
-      def reset_password!(new_password, new_password_confirmation)
+      def reset_password(new_password, new_password_confirmation)
         self.password = new_password
         self.password_confirmation = new_password_confirmation
 
@@ -44,6 +44,11 @@ def reset_password!(new_password, new_password_confirmation)
         save
       end
 
+      def reset_password!(new_password, new_password_confirmation)
+        ActiveSupport::Deprecation.warn "reset_password! is deprecated in favor of reset_password"
+        reset_password(new_password, new_password_confirmation)
+      end
+
       # Resets reset password token and send reset password instructions by email.
       # Returns the token sent in the e-mail.
       def send_reset_password_instructions
@@ -142,7 +147,7 @@ def reset_password_by_token(attributes={})
 
           if recoverable.persisted?
             if recoverable.reset_password_period_valid?
-              recoverable.reset_password!(attributes[:password], attributes[:password_confirmation])
+              recoverable.reset_password(attributes[:password], attributes[:password_confirmation])
             else
               recoverable.errors.add(:reset_password_token, :expired)
             end

test/controllers/custom_registrations_controller_test.rb

@@ -8,7 +8,7 @@ class CustomRegistrationsControllerTest < ActionController::TestCase
   setup do
     request.env["devise.mapping"] = Devise.mappings[:user]
     @password = 'password'
-    @user = create_user(password: @password, password_confirmation: @password).tap(&:confirm!)
+    @user = create_user(password: @password, password_confirmation: @password).tap(&:confirm)
   end
 
   test "yield resource to block on create success" do

test/controllers/passwords_controller_test.rb

@@ -6,7 +6,7 @@ class PasswordsControllerTest < ActionController::TestCase
 
   setup do
     request.env["devise.mapping"] = Devise.mappings[:user]
-    @user = create_user.tap(&:confirm!)
+    @user = create_user.tap(&:confirm)
     @raw  = @user.send_reset_password_instructions
   end
 

test/controllers/sessions_controller_test.rb

@@ -36,7 +36,7 @@ class SessionsControllerTest < ActionController::TestCase
     request.session["user_return_to"] = 'foo.bar'
 
     user = create_user
-    user.confirm!
+    user.confirm
     post :create, user: {
       email: user.email,
       password: user.password
@@ -50,7 +50,7 @@ class SessionsControllerTest < ActionController::TestCase
     request.session["user_return_to"] = 'foo.bar'
 
     user = create_user
-    user.confirm!
+    user.confirm
     post :create, format: 'json', user: {
       email: user.email,
       password: user.password
@@ -72,7 +72,7 @@ class SessionsControllerTest < ActionController::TestCase
   test "#destroy doesn't set the flash if the requested format is not navigational" do
     request.env["devise.mapping"] = Devise.mappings[:user]
     user = create_user
-    user.confirm!
+    user.confirm
     post :create, format: 'json', user: {
       email: user.email,
       password: user.password

test/models/confirmable_test.rb

@@ -23,24 +23,24 @@ def setup
   test 'should confirm a user by updating confirmed at' do
     user = create_user
     assert_nil user.confirmed_at
-    assert user.confirm!
+    assert user.confirm
     assert_not_nil user.confirmed_at
   end
 
   test 'should verify whether a user is confirmed or not' do
     assert_not new_user.confirmed?
     user = create_user
     assert_not user.confirmed?
-    user.confirm!
+    user.confirm
     assert user.confirmed?
   end
 
   test 'should not confirm a user already confirmed' do
     user = create_user
-    assert user.confirm!
+    assert user.confirm
     assert_blank user.errors[:email]
 
-    assert_not user.confirm!
+    assert_not user.confirm
     assert_equal "was already confirmed, please try signing in", user.errors[:email].join
   end
 
@@ -169,7 +169,7 @@ def setup
   test 'should not reset confirmation status or token when updating email' do
     user = create_user
     original_token = user.confirmation_token
-    user.confirm!
+    user.confirm
     user.email = '[email protected]'
     user.save!
 
@@ -180,7 +180,7 @@ def setup
 
   test 'should not be able to send instructions if the user is already confirmed' do
     user = create_user
-    user.confirm!
+    user.confirm
     assert_not user.resend_confirmation_instructions
     assert user.confirmed?
     assert_equal 'was already confirmed, please try signing in', user.errors[:email].join
@@ -215,7 +215,7 @@ def setup
     assert_not user.confirmed?
     assert_not user.active_for_authentication?
 
-    user.confirm!
+    user.confirm
     assert user.confirmed?
     assert user.active_for_authentication?
   end
@@ -305,38 +305,38 @@ def confirm_user_by_token_with_confirmation_sent_at(confirmation_sent_at)
       self.username = self.username.to_s + 'updated'
     end
     old = user.username
-    assert user.confirm!
+    assert user.confirm
     assert_not_equal user.username, old
   end
 
   test 'should not call after_confirmation if not confirmed' do
     user = create_user
-    assert user.confirm!
+    assert user.confirm
     user.define_singleton_method :after_confirmation do
       self.username = self.username.to_s + 'updated'
     end
     old = user.username
-    assert_not user.confirm!
+    assert_not user.confirm
     assert_equal user.username, old
   end
 
   test 'should always perform validations upon confirm when ensure valid true' do
     admin = create_admin
     admin.stubs(:valid?).returns(false)
-    assert_not admin.confirm!(ensure_valid: true)
+    assert_not admin.confirm(ensure_valid: true)
   end
 end
 
 class ReconfirmableTest < ActiveSupport::TestCase
   test 'should not worry about validations on confirm even with reconfirmable' do
     admin = create_admin
     admin.reset_password_token = "a"
-    assert admin.confirm!
+    assert admin.confirm
   end
 
   test 'should generate confirmation token after changing email' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     residual_token = admin.confirmation_token
     assert admin.update_attributes(email: '[email protected]')
     assert_not_equal residual_token, admin.confirmation_token
@@ -345,7 +345,7 @@ class ReconfirmableTest < ActiveSupport::TestCase
   test 'should not regenerate confirmation token or require reconfirmation if skipping reconfirmation after changing email' do
     admin = create_admin
     original_token = admin.confirmation_token
-    assert admin.confirm!
+    assert admin.confirm
     admin.skip_reconfirmation!
     assert admin.update_attributes(email: '[email protected]')
     assert admin.confirmed?
@@ -364,7 +364,7 @@ class ReconfirmableTest < ActiveSupport::TestCase
 
   test 'should regenerate confirmation token after changing email' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert admin.update_attributes(email: '[email protected]')
     token = admin.confirmation_token
     assert admin.update_attributes(email: '[email protected]')
@@ -373,7 +373,7 @@ class ReconfirmableTest < ActiveSupport::TestCase
 
   test 'should send confirmation instructions by email after changing email' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert_email_sent "[email protected]" do
       assert admin.update_attributes(email: '[email protected]')
     end
@@ -382,15 +382,15 @@ class ReconfirmableTest < ActiveSupport::TestCase
 
   test 'should not send confirmation by email after changing password' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert_email_not_sent do
       assert admin.update_attributes(password: 'newpass', password_confirmation: 'newpass')
     end
   end
 
   test 'should not send confirmation by email after changing to a blank email' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert_email_not_sent do
       admin.email = ''
       admin.save(validate: false)
@@ -399,23 +399,23 @@ class ReconfirmableTest < ActiveSupport::TestCase
 
   test 'should stay confirmed when email is changed' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert admin.update_attributes(email: '[email protected]')
     assert admin.confirmed?
   end
 
   test 'should update email only when it is confirmed' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert admin.update_attributes(email: '[email protected]')
     assert_not_equal '[email protected]', admin.email
-    assert admin.confirm!
+    assert admin.confirm
     assert_equal '[email protected]', admin.email
   end
 
   test 'should not allow admin to get past confirmation email by resubmitting their new address' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert admin.update_attributes(email: '[email protected]')
     assert_not_equal '[email protected]', admin.email
     assert admin.update_attributes(email: '[email protected]')
@@ -424,7 +424,7 @@ class ReconfirmableTest < ActiveSupport::TestCase
 
   test 'should find a admin by send confirmation instructions with unconfirmed_email' do
     admin = create_admin
-    assert admin.confirm!
+    assert admin.confirm
     assert admin.update_attributes(email: '[email protected]')
     confirmation_admin = Admin.send_confirmation_instructions(email: admin.unconfirmed_email)
     assert_equal confirmation_admin, admin

test/models/lockable_test.rb

@@ -7,7 +7,7 @@ def setup
 
   test "should respect maximum attempts configuration" do
     user = create_user
-    user.confirm!
+    user.confirm
     swap Devise, maximum_attempts: 2 do
       2.times { user.valid_for_authentication?{ false } }
       assert user.reload.access_locked?
@@ -16,7 +16,7 @@ def setup
 
   test "should increment failed_attempts on successfull validation if the user is already locked" do
     user = create_user
-    user.confirm!
+    user.confirm
 
     swap Devise, maximum_attempts: 2 do
       2.times { user.valid_for_authentication?{ false } }
@@ -29,7 +29,7 @@ def setup
 
   test "should not touch failed_attempts if lock_strategy is none" do
     user = create_user
-    user.confirm!
+    user.confirm
     swap Devise, lock_strategy: :none, maximum_attempts: 2 do
       3.times { user.valid_for_authentication?{ false } }
       assert !user.access_locked?
@@ -53,7 +53,7 @@ def setup
 
   test "active_for_authentication? should be the opposite of locked?" do
     user = create_user
-    user.confirm!
+    user.confirm
     assert user.active_for_authentication?
     user.lock_access!
     assert_not user.active_for_authentication?
@@ -230,7 +230,7 @@ def setup
   test 'should unlock account if lock has expired and increase attempts on failure' do
     swap Devise, unlock_in: 1.minute do
       user = create_user
-      user.confirm!
+      user.confirm
 
       user.failed_attempts = 2
       user.locked_at = 2.minutes.ago
@@ -243,7 +243,7 @@ def setup
   test 'should unlock account if lock has expired on success' do
     swap Devise, unlock_in: 1.minute do
       user = create_user
-      user.confirm!
+      user.confirm
 
       user.failed_attempts = 2
       user.locked_at = 2.minutes.ago