diff --git a/app/models/user.rb b/app/models/user.rb index ff09acbf796..aca72daff04 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -209,10 +209,8 @@ 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.last_sign_in_at = current_sign_in_at || new_current self.current_sign_in_at = new_current increment(:sign_in_count) if new_sign_in diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 32c9b7f9f8f..a9ab15a956e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -163,12 +163,13 @@ RSpec.describe User do 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 } + let!(:user) { Fabricate :user, last_sign_in_at: 10.days.ago, current_sign_in_at:, sign_in_count: 123 } + let(:current_sign_in_at) { 1.hour.ago } 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) + .to change(user, :last_sign_in_at).to(current_sign_in_at) .and change(user, :current_sign_in_at) .and not_change(user, :sign_in_count) end @@ -177,11 +178,22 @@ RSpec.describe User do 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) + .to change(user, :last_sign_in_at).to(current_sign_in_at) .and change(user, :current_sign_in_at) .and change(user, :sign_in_count).by(1) end end + + context 'when the user does not have a current_sign_in_at value' do + let(:current_sign_in_at) { nil } + + before { travel_to(1.minute.ago) } + + it 'updates last sign in to now' do + expect { user.update_sign_in! } + .to change(user, :last_sign_in_at).to(Time.now.utc) + end + end end context 'with a new user' do