From 8bb7615cb113bf7b73eaef9d9335588b3d15621c Mon Sep 17 00:00:00 2001 From: songtianlun Date: Wed, 8 Jan 2025 10:14:36 +0800 Subject: [PATCH] fix: correct user activation and password reset logic - Change `user.send(:activate)` to `user.activate` for clarity. - Fix typo in email parameter from `emial` to `email` in password reset. - Update render calls to include status codes for better error handling. - Modify password reset email method to accept a user parameter. - Update tests to reflect changes in password reset functionality. These changes improve the clarity of the user activation process and ensure that the password reset functionality works correctly with proper error handling and user feedback. --- .../account_activations_controller.rb | 2 +- app/controllers/password_resets_controller.rb | 4 +-- app/controllers/users_controller.rb | 4 +-- app/mailers/user_mailer.rb | 7 ++-- app/models/user.rb | 32 ++++++++++++------- app/views/password_resets/new.html.erb | 2 +- app/views/user_mailer/password_reset.html.erb | 11 +++++-- app/views/user_mailer/password_reset.text.erb | 8 +++-- test/mailers/previews/user_mailer_preview.rb | 4 ++- test/mailers/user_mailer_test.rb | 11 ++++--- 10 files changed, 55 insertions(+), 30 deletions(-) diff --git a/app/controllers/account_activations_controller.rb b/app/controllers/account_activations_controller.rb index b0ded3d..6f47b89 100644 --- a/app/controllers/account_activations_controller.rb +++ b/app/controllers/account_activations_controller.rb @@ -3,7 +3,7 @@ class AccountActivationsController < ApplicationController def edit user = User.find_by(email: params[:email]) if user && !user.activated? && user.authenticated?(:activation, params[:id]) - user.send(:activate) + user.activate log_in user flash[:success] = "Account activated!" redirect_to user diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index 7dc3455..0acd046 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -3,7 +3,7 @@ class PasswordResetsController < ApplicationController end def create - @user = User.find_by(emial: params[:password_reset][:email].downcase) + @user = User.find_by(email: params[:password_reset][:email].downcase) if @user @user.create_reset_digest @user.send_password_reset_email @@ -11,7 +11,7 @@ class PasswordResetsController < ApplicationController redirect_to root_url else flash.now[:danger] = "Email not found" - render 'new' + render 'new', status: :unprocessable_entity end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index edc7eee..4415b7c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -28,11 +28,11 @@ class UsersController < ApplicationController # flash[:success] = "Welcome to the Sample App!" # redirect_to @user # redirect_to user_url(@user) - @user.send(:send_activation_email) + @user.send_activation_email flash[:info] = "Please check your email to activate your account." redirect_to root_url else - render "new" + render "new", status: :unprocessable_entity end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index e0ff16a..cce1ee0 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -14,9 +14,8 @@ class UserMailer < ApplicationMailer # # en.user_mailer.password_reset.subject # - def password_reset - @greeting = "Hi" - - mail to: "to@example.org" + def password_reset(user) + @user = user + mail to: user.email, subject: "Password reset" end end diff --git a/app/models/user.rb b/app/models/user.rb index 25343b9..1875cf8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - attr_accessor :remember_token, :activation_token + attr_accessor :remember_token, :activation_token, :reset_token # before_save { self.email = email.downcase } # before_save { email.downcase! } before_save :downcase_email @@ -57,6 +57,26 @@ class User < ApplicationRecord update_attribute(:remember_digest, nil) end + def activate + # update_attribute(:activated, true) + # update_attribute(:activated_at, Time.zone.now) + update_columns(activated: true, activated_at: Time.zone.now) + end + + def send_activation_email + UserMailer.account_activation(self).deliver_now + end + + def create_reset_digest + self.reset_token = User.new_token + update_attribute(:reset_digest, User.digest(reset_token)) + update_attribute(:reset_sent_at, Time.zone.now) + end + + def send_password_reset_email + UserMailer.password_reset(self).deliver_now + end + private def downcase_email @@ -68,14 +88,4 @@ class User < ApplicationRecord self.activation_token = User.new_token self.activation_digest = User.digest(activation_token) end - - def activate - # update_attribute(:activated, true) - # update_attribute(:activated_at, Time.zone.now) - update_columns(activated: true, activated_at: Time.zone.now) - end - - def send_activation_email - UserMailer.account_activation(self).deliver_now - end end diff --git a/app/views/password_resets/new.html.erb b/app/views/password_resets/new.html.erb index f9debf8..350bcd0 100644 --- a/app/views/password_resets/new.html.erb +++ b/app/views/password_resets/new.html.erb @@ -3,7 +3,7 @@
- <%= form_with(url: password_resets_path, scope: password_reset, + <%= form_with(url: password_resets_path, scope: :password_reset, local: true) do |f| %> <%= f.label :email %> <%= f.email_field :email, class: "form-control" %> diff --git a/app/views/user_mailer/password_reset.html.erb b/app/views/user_mailer/password_reset.html.erb index 6dec984..0daa991 100644 --- a/app/views/user_mailer/password_reset.html.erb +++ b/app/views/user_mailer/password_reset.html.erb @@ -1,5 +1,12 @@ -

User#password_reset

+

Password reset

+ +

To reset your password click the link below:

+ +<%= link_to "Reset password", edit_password_reset_url(@user.reset_token, + email: @user.email) %> + +

This link will expire in two hours.

- <%= @greeting %>, find me in app/views/user_mailer/password_reset.html.erb + If you did not request your password to be reset, please ignore this email and your password will stay as it is.

diff --git a/app/views/user_mailer/password_reset.text.erb b/app/views/user_mailer/password_reset.text.erb index 5ba80cc..d080702 100644 --- a/app/views/user_mailer/password_reset.text.erb +++ b/app/views/user_mailer/password_reset.text.erb @@ -1,3 +1,7 @@ -User#password_reset +To reset your password click the link below: -<%= @greeting %>, find me in app/views/user_mailer/password_reset.text.erb +<%= edit_password_reset_url(@user.reset_token, email: @user.email) %> + +This link will expire in two hours. + +If you did not request your password to be reset, please ignore this email and your password will stay as it is. diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb index 9c70a0d..c5c3d24 100644 --- a/test/mailers/previews/user_mailer_preview.rb +++ b/test/mailers/previews/user_mailer_preview.rb @@ -9,6 +9,8 @@ class UserMailerPreview < ActionMailer::Preview # Preview this email at http://localhost:3000/rails/mailers/user_mailer/password_reset def password_reset - UserMailer.password_reset + user = User.first + user.reset_token = User.new_token + UserMailer.password_reset(user) end end diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb index 8adf3eb..92584f9 100644 --- a/test/mailers/user_mailer_test.rb +++ b/test/mailers/user_mailer_test.rb @@ -14,10 +14,13 @@ class UserMailerTest < ActionMailer::TestCase end test "password_reset" do - mail = UserMailer.password_reset - assert_equal "Password reset", mail.subject - assert_equal [ "to@example.org" ], mail.to + user = users(:michael) + user.reset_token = User.new_token + mail = UserMailer.password_reset(user) + assert_equal "Password reset", mail.subject + assert_equal [ user.email ], mail.to assert_equal [ "noreply@mail.frytea.com" ], mail.from - assert_match "Hi", mail.body.encoded + assert_match user.reset_token, mail.body.encoded + assert_match CGI.escape(user.email), mail.body.encoded end end