Browse Source

Refactoring: Simplyfied if statement and removed reference to issue #2431 because it's more of a byproduct than an actual fix.

Billy Zhou 6 years ago
parent
commit
399fef668d
2 changed files with 31 additions and 1 deletions
  1. 4 1
      app/models/user.rb
  2. 27 0
      spec/models/user_spec.rb

+ 4 - 1
app/models/user.rb

@@ -634,7 +634,10 @@ returns
 =end
 
   def update_last_login
-    self.last_login = Time.zone.now
+    # reduce DB/ES load by updating last_login every 10 minutes only
+    if !last_login || last_login < 10.minutes.ago
+      self.last_login = Time.zone.now
+    end
 
     # reset login failed
     self.login_failed = 0

+ 27 - 0
spec/models/user_spec.rb

@@ -40,6 +40,33 @@ RSpec.describe User, type: :model do
               .to be(nil)
           end
         end
+
+        it "updates user's updates last_login and updated_at attributes" do
+          expect { described_class.authenticate(user.login, password) }
+            .to change { user.reload.last_login }
+            .and change { user.reload.updated_at }
+        end
+
+        context 'when authenticated multiple after another' do
+
+          before { described_class.authenticate(user.login, password) }
+
+          it "doesn't update last_login and updated_at when last login was less than 10 minutes ago" do
+            travel 9.minutes
+
+            expect { described_class.authenticate(user.login, password) }
+              .to not_change { user.reload.last_login }
+              .and not_change { user.reload.updated_at }
+          end
+
+          it 'updates last_login and updated_at when last login was more than 10 minutes ago' do
+            travel 11.minutes
+
+            expect { described_class.authenticate(user.login, password) }
+              .to change { user.reload.last_login }
+              .and change { user.reload.updated_at }
+          end
+        end
       end
 
       context 'with valid user and invalid password' do