From ab9c62e8c7d71296f426b8c1a8c5c892bc570523 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Mon, 25 Nov 2024 03:18:10 -0500
Subject: [PATCH] Add coverage for `User` validations (#33028)

---
 spec/models/user_spec.rb | 49 +++++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9a5a070d25..f39b80c942 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -33,14 +33,12 @@ RSpec.describe User do
     end
   end
 
-  describe 'validations' do
+  describe 'Associations' do
     it { is_expected.to belong_to(:account).required }
+  end
 
-    it 'is invalid without a valid email' do
-      user = Fabricate.build(:user, email: 'john@')
-      user.valid?
-      expect(user).to model_have_error_on_field(:email)
-    end
+  describe 'Validations' do
+    it { is_expected.to_not allow_value('john@').for(:email) }
 
     it 'is valid with an invalid e-mail that has already been saved' do
       user = Fabricate.build(:user, email: 'invalid-email')
@@ -48,11 +46,7 @@ RSpec.describe User do
       expect(user.valid?).to be true
     end
 
-    it 'is valid with a localhost e-mail address' do
-      user = Fabricate.build(:user, email: 'admin@localhost')
-      user.valid?
-      expect(user.valid?).to be true
-    end
+    it { is_expected.to allow_value('admin@localhost').for(:email) }
   end
 
   describe 'Normalizations' do
@@ -183,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)