From 365028751078547050e3c0f64cb537aff376afe0 Mon Sep 17 00:00:00 2001
From: Armando Fox <fox@cs.berkeley.edu>
Date: Thu, 28 Dec 2023 18:30:28 -0800
Subject: [PATCH] Customer#record_login! now has to be explicitly called from
 the relevant 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.

---
 app/controllers/application_controller.rb  | 5 -----
 app/controllers/customers_controller.rb    | 5 +++++
 app/controllers/sessions_controller.rb     | 3 ++-
 app/models/customer.rb                     | 4 ++++
 features/customers/merge_customers.feature | 2 +-
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1823d541e..68d024817 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -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")
diff --git a/app/controllers/customers_controller.rb b/app/controllers/customers_controller.rb
index 0fa069d0c..b5a72925d 100644
--- a/app/controllers/customers_controller.rb
+++ b/app/controllers/customers_controller.rb
@@ -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
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index c1222b5d0..36fa5012e 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -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
@@ -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
diff --git a/app/models/customer.rb b/app/models/customer.rb
index bc92c7925..eaf1c4fd4 100644
--- a/app/models/customer.rb
+++ b/app/models/customer.rb
@@ -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
diff --git a/features/customers/merge_customers.feature b/features/customers/merge_customers.feature
index 6a34fc976..56c504b02 100644
--- a/features/customers/merge_customers.feature
+++ b/features/customers/merge_customers.feature
@@ -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