Merge branch 'rd-update-last_activity_on-on-logins-and-browsing-activity-54947' into 'master'
Update User's last_activity_on for any GET request on projects Closes #54947 See merge request gitlab-org/gitlab-ce!24642
This commit is contained in:
commit
a156689849
|
@ -0,0 +1,27 @@
|
||||||
|
# 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.
|
||||||
|
#
|
||||||
|
# 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
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
class Dashboard::ApplicationController < ApplicationController
|
class Dashboard::ApplicationController < ApplicationController
|
||||||
include ControllerWithCrossProjectAccessCheck
|
include ControllerWithCrossProjectAccessCheck
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
layout 'dashboard'
|
layout 'dashboard'
|
||||||
|
|
||||||
|
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
class Groups::BoardsController < Groups::ApplicationController
|
class Groups::BoardsController < Groups::ApplicationController
|
||||||
include BoardsResponses
|
include BoardsResponses
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
before_action :assign_endpoint_vars
|
before_action :assign_endpoint_vars
|
||||||
before_action :boards, only: :index
|
before_action :boards, only: :index
|
||||||
|
|
|
@ -5,6 +5,7 @@ class GroupsController < Groups::ApplicationController
|
||||||
include IssuableCollectionsAction
|
include IssuableCollectionsAction
|
||||||
include ParamsBackwardCompatibility
|
include ParamsBackwardCompatibility
|
||||||
include PreviewMarkdown
|
include PreviewMarkdown
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
respond_to :html
|
respond_to :html
|
||||||
|
|
||||||
|
|
|
@ -8,6 +8,7 @@ class Projects::IssuesController < Projects::ApplicationController
|
||||||
include IssuableCollections
|
include IssuableCollections
|
||||||
include IssuesCalendar
|
include IssuesCalendar
|
||||||
include SpammableActions
|
include SpammableActions
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
def self.issue_except_actions
|
def self.issue_except_actions
|
||||||
%i[index calendar new create bulk_update import_csv]
|
%i[index calendar new create bulk_update import_csv]
|
||||||
|
|
|
@ -7,6 +7,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
|
||||||
include RendersCommits
|
include RendersCommits
|
||||||
include ToggleAwardEmoji
|
include ToggleAwardEmoji
|
||||||
include IssuableCollections
|
include IssuableCollections
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
skip_before_action :merge_request, only: [:index, :bulk_update]
|
skip_before_action :merge_request, only: [:index, :bulk_update]
|
||||||
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
|
before_action :whitelist_query_limiting, only: [:assign_related_issues, :update]
|
||||||
|
|
|
@ -6,6 +6,7 @@ class ProjectsController < Projects::ApplicationController
|
||||||
include ExtractsPath
|
include ExtractsPath
|
||||||
include PreviewMarkdown
|
include PreviewMarkdown
|
||||||
include SendFileUpload
|
include SendFileUpload
|
||||||
|
include RecordUserLastActivity
|
||||||
|
|
||||||
prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
|
prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Update last_activity_on for Users on some main GET endpoints
|
||||||
|
merge_request: 24642
|
||||||
|
author:
|
||||||
|
type: changed
|
|
@ -1212,6 +1212,7 @@ The activities that update the timestamp are:
|
||||||
|
|
||||||
- Git HTTP/SSH activities (such as clone, push)
|
- Git HTTP/SSH activities (such as clone, push)
|
||||||
- User logging in into GitLab
|
- 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
|
By default, it shows the activity for all users in the last 6 months, but this can be
|
||||||
amended by using the `from` parameter.
|
amended by using the `from` parameter.
|
||||||
|
|
|
@ -25,3 +25,4 @@ How do we measure the activity of users? GitLab considers a user active if:
|
||||||
|
|
||||||
- The user signs in.
|
- The user signs in.
|
||||||
- The user has Git activity (whether push or pull).
|
- 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).
|
||||||
|
|
|
@ -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
|
Loading…
Reference in New Issue