From d0252442f6dfc6ca75ac36a129cef140b28e760c Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Sat, 12 Oct 2024 17:47:31 -0400
Subject: [PATCH] Add coverage for update_sign_in!

---
 app/models/user.rb       | 11 +++--------
 spec/models/user_spec.rb | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index be9ebac699..734f3d8691 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -208,16 +208,11 @@ class User < ApplicationRecord
   end
 
   def update_sign_in!(new_sign_in: false)
-    old_current = current_sign_in_at
     new_current = Time.now.utc
 
-    self.last_sign_in_at     = old_current || new_current
-    self.current_sign_in_at  = new_current
-
-    if new_sign_in
-      self.sign_in_count ||= 0
-      self.sign_in_count  += 1
-    end
+    self.last_sign_in_at = current_sign_in_at || new_current
+    self.current_sign_in_at = new_current
+    self.sign_in_count = sign_in_count.to_i.next if new_sign_in
 
     save(validate: false) unless new_record?
     prepare_returning_user!
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index e907f2418e..f39b80c942 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -177,6 +177,39 @@ RSpec.describe User do
     end
   end
 
+  describe '#update_sign_in!' do
+    context 'with an existing user' do
+      let!(:user) { Fabricate :user, last_sign_in_at: 10.days.ago, current_sign_in_at: 1.hour.ago, sign_in_count: 123 }
+
+      context 'with new sign in false' do
+        it 'updates timestamps but not counts' do
+          expect { user.update_sign_in!(new_sign_in: false) }
+            .to change(user, :last_sign_in_at)
+            .and change(user, :current_sign_in_at)
+            .and not_change(user, :sign_in_count)
+        end
+      end
+
+      context 'with new sign in true' do
+        it 'updates timestamps and counts' do
+          expect { user.update_sign_in!(new_sign_in: true) }
+            .to change(user, :last_sign_in_at)
+            .and change(user, :current_sign_in_at)
+            .and change(user, :sign_in_count).by(1)
+        end
+      end
+    end
+
+    context 'with a new user' do
+      let(:user) { Fabricate.build :user }
+
+      it 'does not persist the user' do
+        expect { user.update_sign_in! }
+          .to_not change(user, :persisted?).from(false)
+      end
+    end
+  end
+
   describe '#confirmed?' do
     it 'returns true when a confirmed_at is set' do
       user = Fabricate.build(:user, confirmed_at: Time.now.utc)