Update last_activity_on for Users on some main GET endpoints

In order to have an accurate date about the last activity of a User
we need to update the last_activity_on field when the User is visiting
some basic pages of GitLab like pages related to Dashboards, Projects,
Issues and Merge Requests

11 files changed, 155 insertions, 0 deletions

diff --git a/app/controllers/concerns/record_user_last_activity.rb b/app/controllers/concerns/record_user_last_activity.rb
new file mode 100644
index 00000000000..884c17b3f68
--- /dev/null
+++ b/app/controllers/concerns/record_user_last_activity.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+# == RecordUserLastActivity
+#
+# Controller concern that updates the `last_activity_on` field of `users`
+# for any authenticated GET request. The DB update will only happen once per day
+# if the client supports cookies.
+#
+# In order to determine if you should include this concern or not, please check the
+# description and discussion on this issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/54947
+module RecordUserLastActivity
+  include CookiesHelper
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :set_user_last_activity
+  end
+
+  def set_user_last_activity
+    return unless request.get?
+    return unless Feature.enabled?(:set_user_last_activity, default_enabled: true)
+    return if Gitlab::Database.read_only?
+
+    if current_user && current_user.last_activity_on != Date.today
+      Users::ActivityService.new(current_user, "visited #{request.path}").execute
+    end
+  end
+end
diff --git a/app/controllers/dashboard/application_controller.rb b/app/controllers/dashboard/application_controller.rb
index cee0753a021..0e9fdc60363 100644
--- a/app/controllers/dashboard/application_controller.rb
+++ b/app/controllers/dashboard/application_controller.rb
@@ -2,6 +2,7 @@
 
 class Dashboard::ApplicationController < ApplicationController
   include ControllerWithCrossProjectAccessCheck
+  include RecordUserLastActivity
 
   layout 'dashboard'
 
diff --git a/app/controllers/groups/boards_controller.rb b/app/controllers/groups/boards_controller.rb
index cdc6f53df8e..51fdb6c05fb 100644
--- a/app/controllers/groups/boards_controller.rb
+++ b/app/controllers/groups/boards_controller.rb
@@ -2,6 +2,7 @@
 
 class Groups::BoardsController < Groups::ApplicationController
   include BoardsResponses
+  include RecordUserLastActivity
 
   before_action :assign_endpoint_vars
   before_action :boards, only: :index
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 15aadf3f74b..4e50106398a 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -5,6 +5,7 @@ class GroupsController < Groups::ApplicationController
   include IssuableCollectionsAction
   include ParamsBackwardCompatibility
   include PreviewMarkdown
+  include RecordUserLastActivity
 
   respond_to :html
 
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index f9a80aa3cfb..b9d02a62fc3 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -8,6 +8,7 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuableCollections
   include IssuesCalendar
   include SpammableActions
+  include RecordUserLastActivity
 
   def self.issue_except_actions
     %i[index calendar new create bulk_update import_csv]
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index bc0a3d3526d..7c4dc95529a 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -7,6 +7,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   include RendersCommits
   include ToggleAwardEmoji
   include IssuableCollections
+  include RecordUserLastActivity
 
   skip_before_action :merge_request, only: [:index, :bulk_update]
   before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index d3af35723ac..33c6608d321 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -6,6 +6,7 @@ class ProjectsController < Projects::ApplicationController
   include ExtractsPath
   include PreviewMarkdown
   include SendFileUpload
+  include RecordUserLastActivity
 
   prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
 
diff --git a/changelogs/unreleased/rd-update-last_activity_on-on-logins-and-browsing-activity-54947.yml b/changelogs/unreleased/rd-update-last_activity_on-on-logins-and-browsing-activity-54947.yml
new file mode 100644
index 00000000000..abce9dcc0c6
--- /dev/null
+++ b/changelogs/unreleased/rd-update-last_activity_on-on-logins-and-browsing-activity-54947.yml
@@ -0,0 +1,5 @@
+---
+title: Update last_activity_on for Users on some main GET endpoints
+merge_request: 24642
+author:
+type: changed
diff --git a/doc/api/users.md b/doc/api/users.md
index 6000b9b900f..fd8778abb17 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -1212,6 +1212,7 @@ The activities that update the timestamp are:
 
   - Git HTTP/SSH activities (such as clone, push)
   - User logging in into GitLab
+  - User visiting pages related to Dashboards, Projects, Issues and Merge Requests ([introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/54947) in GitLab 11.8)
 
 By default, it shows the activity for all users in the last 6 months, but this can be
 amended by using the `from` parameter.
diff --git a/doc/user/instance_statistics/user_cohorts.md b/doc/user/instance_statistics/user_cohorts.md
index f52f24ef5f7..e76363a6d9f 100644
--- a/doc/user/instance_statistics/user_cohorts.md
+++ b/doc/user/instance_statistics/user_cohorts.md
@@ -25,3 +25,4 @@ How do we measure the activity of users? GitLab considers a user active if:
 
 - The user signs in.
 - The user has Git activity (whether push or pull).
+- The user visits pages related to Dashboards, Projects, Issues and Merge Requests ([introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/54947) in GitLab 11.8).
diff --git a/spec/requests/user_activity_spec.rb b/spec/requests/user_activity_spec.rb
new file mode 100644
index 00000000000..15666e00b9f
--- /dev/null
+++ b/spec/requests/user_activity_spec.rb
@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Update of user activity' do
+  let(:user) { create(:user, last_activity_on: nil) }
+
+  before do
+    group = create(:group, name: 'group')
+    project = create(:project, :public, namespace: group, name: 'project')
+
+    create(:issue, project: project, iid: 10)
+    create(:merge_request, source_project: project, iid: 15)
+
+    project.add_maintainer(user)
+  end
+
+  paths_to_visit = [
+    '/group',
+    '/group/project',
+    '/groups/group/-/issues',
+    '/groups/group/-/boards',
+    '/dashboard/projects',
+    '/dashboard/snippets',
+    '/dashboard/groups',
+    '/dashboard/todos',
+    '/group/project/issues',
+    '/group/project/issues/10',
+    '/group/project/merge_requests',
+    '/group/project/merge_requests/15'
+  ]
+
+  context 'without an authenticated user' do
+    it 'does not set the last activity cookie' do
+      get "/group/project"
+
+      expect(response.cookies['user_last_activity_on']).to be_nil
+    end
+  end
+
+  context 'with an authenticated user' do
+    before do
+      login_as(user)
+    end
+
+    context 'with a POST request' do
+      it 'does not set the last activity cookie' do
+        post "/group/project/archive"
+
+        expect(response.cookies['user_last_activity_on']).to be_nil
+      end
+    end
+
+    paths_to_visit.each do |path|
+      context "on GET to #{path}" do
+        it 'updates the last activity date' do
+          expect(Users::ActivityService).to receive(:new).and_call_original
+
+          get path
+
+          expect(user.last_activity_on).to eq(Date.today)
+        end
+
+        context 'when calling it twice' do
+          it 'updates last_activity_on just once' do
+            expect(Users::ActivityService).to receive(:new).once.and_call_original
+
+            2.times do
+              get path
+            end
+          end
+        end
+
+        context 'when last_activity_on is nil' do
+          before do
+            user.update_attribute(:last_activity_on, nil)
+          end
+
+          it 'updates the last activity date' do
+            expect(user.last_activity_on).to be_nil
+
+            get path
+
+            expect(user.last_activity_on).to eq(Date.today)
+          end
+        end
+
+        context 'when last_activity_on is stale' do
+          before do
+            user.update_attribute(:last_activity_on, 2.days.ago.to_date)
+          end
+
+          it 'updates the last activity date' do
+            get path
+
+            expect(user.last_activity_on).to eq(Date.today)
+          end
+        end
+
+        context 'when last_activity_on is up to date' do
+          before do
+            user.update_attribute(:last_activity_on, Date.today)
+          end
+
+          it 'does not try to update it' do
+            expect(Users::ActivityService).not_to receive(:new)
+
+            get path
+          end
+        end
+      end
+    end
+  end
+end