From fdcf884cf719b682e413ee047542861b5a5cf3b7 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <mjankowski@thoughtbot.com>
Date: Sat, 29 Apr 2017 18:28:16 -0400
Subject: [PATCH] Extract user tracking into concern (#2600)

---
 app/controllers/application_controller.rb     | 12 +---
 .../concerns/user_tracking_concern.rb         | 30 ++++++++++
 .../concerns/user_tracking_concern_spec.rb    | 59 +++++++++++++++++++
 3 files changed, 90 insertions(+), 11 deletions(-)
 create mode 100644 app/controllers/concerns/user_tracking_concern.rb
 create mode 100644 spec/controllers/concerns/user_tracking_concern_spec.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0c324762df8..8456095fbcb 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -8,6 +8,7 @@ class ApplicationController < ActionController::Base
   force_ssl if: :https_enabled?
 
   include Localized
+  include UserTrackingConcern
 
   helper_method :current_account
   helper_method :single_user_mode?
@@ -17,7 +18,6 @@ class ApplicationController < ActionController::Base
   rescue_from ActionController::InvalidAuthenticityToken, with: :unprocessable_entity
 
   before_action :store_current_location, except: :raise_not_found, unless: :devise_controller?
-  before_action :set_user_activity
   before_action :check_suspension, if: :user_signed_in?
 
   def raise_not_found
@@ -38,16 +38,6 @@ class ApplicationController < ActionController::Base
     redirect_to root_path unless current_user&.admin?
   end
 
-  def set_user_activity
-    return unless !current_user.nil? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < 24.hours.ago)
-
-    # Mark user as signed-in today
-    current_user.update_tracked_fields(request)
-
-    # If the sign in is after a two week break, we need to regenerate their feed
-    RegenerationWorker.perform_async(current_user.account_id) if current_user.last_sign_in_at < 14.days.ago
-  end
-
   def check_suspension
     head 403 if current_user.account.suspended?
   end
diff --git a/app/controllers/concerns/user_tracking_concern.rb b/app/controllers/concerns/user_tracking_concern.rb
new file mode 100644
index 00000000000..2a3449617c7
--- /dev/null
+++ b/app/controllers/concerns/user_tracking_concern.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module UserTrackingConcern
+  extend ActiveSupport::Concern
+
+  REGENERATE_FEED_DAYS = 14
+  UPDATE_SIGN_IN_HOURS = 24
+
+  included do
+    before_action :set_user_activity, if: %i(user_signed_in? user_needs_sign_in_update?)
+  end
+
+  private
+
+  def set_user_activity
+    # Mark as signed-in today
+    current_user.update_tracked_fields!(request)
+
+    # Regenerate feed if needed
+    RegenerationWorker.perform_async(current_user.account_id) if user_needs_feed_update?
+  end
+
+  def user_needs_sign_in_update?
+    current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago
+  end
+
+  def user_needs_feed_update?
+    current_user.last_sign_in_at < REGENERATE_FEED_DAYS.days.ago
+  end
+end
diff --git a/spec/controllers/concerns/user_tracking_concern_spec.rb b/spec/controllers/concerns/user_tracking_concern_spec.rb
new file mode 100644
index 00000000000..6bd29f8870c
--- /dev/null
+++ b/spec/controllers/concerns/user_tracking_concern_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe ApplicationController, type: :controller do
+  controller do
+    include UserTrackingConcern
+    def show
+      render plain: 'show'
+    end
+  end
+
+  before do
+    routes.draw { get 'show' => 'anonymous#show' }
+  end
+
+  describe 'when signed in' do
+    let(:user) { Fabricate(:user) }
+
+    it 'does not track when there is a recent sign in' do
+      user.update(current_sign_in_at: 60.minutes.ago)
+      prior = user.current_sign_in_at
+      sign_in user, scope: :user
+      get :show
+
+      expect(user.reload.current_sign_in_at).to be_within(1.0).of(prior)
+    end
+
+    it 'tracks when sign in is nil' do
+      user.update(current_sign_in_at: nil)
+      sign_in user, scope: :user
+      get :show
+
+      expect_updated_sign_in_at(user)
+    end
+
+    it 'tracks when sign in is older than one day' do
+      user.update(current_sign_in_at: 2.days.ago)
+      sign_in user, scope: :user
+      get :show
+
+      expect_updated_sign_in_at(user)
+    end
+
+    it 'regenerates feed when sign in is older than two weeks' do
+      allow(RegenerationWorker).to receive(:perform_async)
+      user.update(current_sign_in_at: 3.weeks.ago)
+      sign_in user, scope: :user
+      get :show
+
+      expect_updated_sign_in_at(user)
+      expect(RegenerationWorker).to have_received(:perform_async)
+    end
+
+    def expect_updated_sign_in_at(user)
+      expect(user.reload.current_sign_in_at).to be_within(1.0).of(Time.now.utc)
+    end
+  end
+end