Skip to content

Commit

Permalink
Customer#record_login! now has to be explicitly called from the relev…
Browse files Browse the repository at this point in the history
…ant flows that call SessionsController#create_session. Notably, when that method is called from the guest-checkout flow for a customer who has previously purchased with a given email BUT has never logged in, a login should NOT be recorded.
  • Loading branch information
armandofox committed Dec 29, 2023
1 parent cf4c1b5 commit 3650287
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 7 deletions.
5 changes: 0 additions & 5 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,6 @@ def create_session(u = nil, action = '')
# button. Uncomment if you understand the tradeoffs.
# reset_session
self.current_user = @user
# 185979216: explicitly update last_login so that if this customer has never logged
# in before, this counts as a 'Login' action and they will now see the action tabs.
# This update used to occur in SessionsController#create, but creating a session
# can also happen as the result of resetting a password.
@user.update_attribute(:last_login, Time.current)
session[:guest_checkout] = false
# 'remember me' checked?
new_cookie_flag = (params[:remember_me] == "1")
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/customers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ def reset_token
if @customer.try(:valid_reset_token?)
@customer.token_created_at = 10.minutes.ago
create_session(@customer, 'reset_token')
# 185979216: explicitly update last_login so that if this customer has never logged
# in before, this counts as a 'Login' action and they will now see the action tabs.
# This update used to occur in SessionsController#create, but creating a session
# can also happen as the result of resetting a password.
@customer.record_login!
else
redirect_to login_path, :alert => "The reset password link is invalid or has expired"
end
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def create
note_failed_signin(@email, u)
redirect_to new_session_path, :email => @email, :remember_me => @remember_me
else
u.record_login!
session.delete(:admin_disabled) # in case admin signin
end
u
Expand All @@ -38,7 +39,7 @@ def create_from_secret
note_failed_signin(@email, u)
redirect_to (u.errors.has_key?(:no_secret_question) ? login_path : new_from_secret_session_path)
else
u.update_attribute(:last_login,Time.current)
u.record_login!
end
u
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/customer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ def valid_reset_token?
token_created_at >= 10.minutes.ago
end

def record_login!
self.update_attributes!(:last_login => Time.current)
end

def has_ever_logged_in?
last_login > Time.zone.parse('2007-04-07') # sentinel date should match what's in schema.rb
end
Expand Down
2 changes: 1 addition & 1 deletion features/customers/merge_customers.feature
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ Scenario: cannot merge Admins

When I select customers "Super Administrator" and "Janey Weigandt" for merging
And I press "Auto Merge"
Then I should see "super admins cannot be merged"
Then I should see "Customers with the role of Admin cannot be merged with other customers."
And customer "Janey Weigandt" should exist
And customer "Super Administrator" should exist

0 comments on commit 3650287

Please sign in to comment.