diff --git a/app/models/user.rb b/app/models/user.rb index 83e17bcb04e187158c9be0d1464302d3f7a74074..2a351b679eae04139c33198981c501c905885d97 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -950,9 +950,7 @@ class User < ActiveRecord::Base end def record_activity - Gitlab::Redis.with do |redis| - redis.zadd('user/activities', Time.now.to_i, self.username) - end + Gitlab::UserActivities::ActivitySet.record(self) end private diff --git a/doc/api/users.md b/doc/api/users.md index 2ada4d09c84476451403ea2d2eba8ca541f68a9c..84dbc350e6b1ca77ff5860b8379ecb955acce258 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -986,3 +986,56 @@ Parameters: | --------- | ---- | -------- | ----------- | | `user_id` | integer | yes | The ID of the user | | `impersonation_token_id` | integer | yes | The ID of the impersonation token | + +### Get user activities (admin only) + +>**Note:** This API endpoint is only available on 8.15 (EE) and 9.1 (CE) and above. + +Get the last activity date for all users, sorted from oldest to newest. + +The activities that update the timestamp are: + + - Git HTTP/SSH activities (such as clone, push) + - User logging in into GitLab + +The data is stored in Redis and it depends on it for being recorded and displayed +over time. This means that we will lose the data if Redis gets flushed, or a custom +TTL is reached. + +By default, it shows the activity for all users in the last 6 months, but this can be +amended by using the `from` parameter. + +This function takes pagination parameters `page` and `per_page` to restrict the list of users. + + +``` +GET /user/activities +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `from` | string | no | Date string in the format YEAR-MONTH-DAY, e.g. `2016-03-11`. Defaults to 6 months ago. | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/user/activities +``` + +Example response: + +```json +[ + { + "username": "user1", + "last_activity_at": "2015-12-14 01:00:00" + }, + { + "username": "user2", + "last_activity_at": "2015-12-15 01:00:00" + }, + { + "username": "user3", + "last_activity_at": "2015-12-16 01:00:00" + } +] diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9919762cd820f7ac1e49821c9f922466cf8c602e..939cedc1b270544cd9080765c4f722609b03592a 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -18,6 +18,11 @@ module API expose :bio, :location, :skype, :linkedin, :twitter, :website_url, :organization end + class UserActivity < Grape::Entity + expose :username + expose :last_activity_at + end + class Identity < Grape::Entity expose :provider, :extern_uid end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 05423c174498059be3010e5aad0353bab16f63b5..244725bb2920da31e42902ffd7f169d6cb9e0445 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -35,7 +35,7 @@ module API optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue' optional :labels, type: String, desc: 'Comma-separated list of label names' - optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY' + optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' end diff --git a/lib/api/users.rb b/lib/api/users.rb index eedc59f86368c89dfd103ecef769d6cca07bf7d2..16fa1ef683641ac31f65f778c0f2adad71050a3f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -534,6 +534,24 @@ module API email.destroy current_user.update_secondary_emails! end + + + desc 'Get a list of user activities' + params do + optional :from, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' + use :pagination + end + get ":activities" do + authenticated_as_admin! + + activity_set = Gitlab::UserActivities::ActivitySet.new(from: params[:from], + page: params[:page], + per_page: params[:per_page]) + + add_pagination_headers(activity_set) + + present activity_set.activities, with: Entities::UserActivity + end end end end diff --git a/lib/gitlab/pagination_delegate.rb b/lib/gitlab/pagination_delegate.rb new file mode 100644 index 0000000000000000000000000000000000000000..d4913e908b2a74e36f011d8122cbba4adb0e7409 --- /dev/null +++ b/lib/gitlab/pagination_delegate.rb @@ -0,0 +1,65 @@ +module Gitlab + class PaginationDelegate + DEFAULT_PER_PAGE = Kaminari.config.default_per_page + MAX_PER_PAGE = Kaminari.config.max_per_page + + def initialize(page:, per_page:, count:, options: {}) + @count = count + @options = { default_per_page: DEFAULT_PER_PAGE, + max_per_page: MAX_PER_PAGE }.merge(options) + + @per_page = sanitize_per_page(per_page) + @page = sanitize_page(page) + end + + def total_count + @count + end + + def total_pages + (total_count.to_f / @per_page).ceil + end + + def next_page + current_page + 1 unless last_page? + end + + def prev_page + current_page - 1 unless first_page? + end + + def current_page + @page + end + + def limit_value + @per_page + end + + def first_page? + current_page == 1 + end + + def last_page? + current_page >= total_pages + end + + def offset + (current_page - 1) * limit_value + end + + private + + def sanitize_per_page(per_page) + return @options[:default_per_page] unless per_page && per_page > 0 + + [@options[:max_per_page], per_page].min + end + + def sanitize_page(page) + return 1 unless page && page > 1 + + [total_pages, page].min + end + end +end diff --git a/lib/gitlab/user_activities/activity.rb b/lib/gitlab/user_activities/activity.rb new file mode 100644 index 0000000000000000000000000000000000000000..ec052870ee366c9d018f048b7431a53ea3c66b81 --- /dev/null +++ b/lib/gitlab/user_activities/activity.rb @@ -0,0 +1,16 @@ +module Gitlab + module UserActivities + class Activity + attr_reader :username + + def initialize(username, time) + @username = username + @time = time + end + + def last_activity_at + @last_activity_at ||= Time.at(@time).to_s(:db) + end + end + end +end diff --git a/lib/gitlab/user_activities/activity_set.rb b/lib/gitlab/user_activities/activity_set.rb new file mode 100644 index 0000000000000000000000000000000000000000..6b8e540e99b5e3aed37147f27638e179153f870d --- /dev/null +++ b/lib/gitlab/user_activities/activity_set.rb @@ -0,0 +1,67 @@ +module Gitlab + module UserActivities + class ActivitySet + delegate :total_count, + :total_pages, + :current_page, + :limit_value, + :first_page?, + :prev_page, + :last_page?, + :next_page, to: :pagination_delegate + + KEY = 'user/activities' + + def self.record(user) + Gitlab::Redis.with do |redis| + redis.zadd(KEY, Time.now.to_i, user.username) + end + end + + def initialize(from: nil, page: nil, per_page: nil) + @from = sanitize_date(from) + @to = Time.now.to_i + @page = page + @per_page = per_page + end + + def activities + @activities ||= raw_activities.map { |activity| Activity.new(*activity) } + end + + private + + def sanitize_date(date) + Time.strptime(date, "%Y-%m-%d").to_i + rescue TypeError, ArgumentError + default_from + end + + def pagination_delegate + @pagination_delegate ||= Gitlab::PaginationDelegate.new(page: @page, + per_page: @per_page, + count: count) + end + + def raw_activities + Gitlab::Redis.with do |redis| + redis.zrangebyscore(KEY, @from, @to, with_scores: true, limit: limit) + end + end + + def count + Gitlab::Redis.with do |redis| + redis.zcount(KEY, @from, @to) + end + end + + def limit + [pagination_delegate.offset, pagination_delegate.limit_value] + end + + def default_from + 6.months.ago.to_i + end + end + end +end diff --git a/spec/lib/gitlab/pagination_delegate_spec.rb b/spec/lib/gitlab/pagination_delegate_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3220d6112742938bddbb6704c8d995c78f345389 --- /dev/null +++ b/spec/lib/gitlab/pagination_delegate_spec.rb @@ -0,0 +1,155 @@ +require 'spec_helper' + +describe Gitlab::PaginationDelegate, lib: true do + context 'no data' do + let(:delegate) do + described_class.new(page: 1, + per_page: 10, + count: 0) + end + + it 'shows the correct total count' do + expect(delegate.total_count).to eq(0) + end + + it 'shows the correct total pages' do + expect(delegate.total_pages).to eq(0) + end + + it 'shows the correct next page' do + expect(delegate.next_page).to be_nil + end + + it 'shows the correct previous page' do + expect(delegate.prev_page).to be_nil + end + + it 'shows the correct current page' do + expect(delegate.current_page).to eq(1) + end + + it 'shows the correct limit value' do + expect(delegate.limit_value).to eq(10) + end + + it 'shows the correct first page' do + expect(delegate.first_page?).to be true + end + + it 'shows the correct last page' do + expect(delegate.last_page?).to be true + end + + it 'shows the correct offset' do + expect(delegate.offset).to eq(0) + end + end + + context 'with data' do + let(:delegate) do + described_class.new(page: 5, + per_page: 100, + count: 1000) + end + + it 'shows the correct total count' do + expect(delegate.total_count).to eq(1000) + end + + it 'shows the correct total pages' do + expect(delegate.total_pages).to eq(10) + end + + it 'shows the correct next page' do + expect(delegate.next_page).to eq(6) + end + + it 'shows the correct previous page' do + expect(delegate.prev_page).to eq(4) + end + + it 'shows the correct current page' do + expect(delegate.current_page).to eq(5) + end + + it 'shows the correct limit value' do + expect(delegate.limit_value).to eq(100) + end + + it 'shows the correct first page' do + expect(delegate.first_page?).to be false + end + + it 'shows the correct last page' do + expect(delegate.last_page?).to be false + end + + it 'shows the correct offset' do + expect(delegate.offset).to eq(400) + end + end + + context 'last page' do + let(:delegate) do + described_class.new(page: 10, + per_page: 100, + count: 1000) + end + + it 'shows the correct total count' do + expect(delegate.total_count).to eq(1000) + end + + it 'shows the correct total pages' do + expect(delegate.total_pages).to eq(10) + end + + it 'shows the correct next page' do + expect(delegate.next_page).to be_nil + end + + it 'shows the correct previous page' do + expect(delegate.prev_page).to eq(9) + end + + it 'shows the correct current page' do + expect(delegate.current_page).to eq(10) + end + + it 'shows the correct limit value' do + expect(delegate.limit_value).to eq(100) + end + + it 'shows the correct first page' do + expect(delegate.first_page?).to be false + end + + it 'shows the correct last page' do + expect(delegate.last_page?).to be true + end + + it 'shows the correct offset' do + expect(delegate.offset).to eq(900) + end + end + + context 'limits and defaults' do + it 'has a maximum limit per page' do + expect(described_class.new(page: nil, + per_page: 1000, + count: 0).limit_value).to eq(described_class::MAX_PER_PAGE) + end + + it 'has a default per page' do + expect(described_class.new(page: nil, + per_page: nil, + count: 0).limit_value).to eq(described_class::DEFAULT_PER_PAGE) + end + + it 'has a maximum page' do + expect(described_class.new(page: 100, + per_page: 10, + count: 1).current_page).to eq(1) + end + end +end diff --git a/spec/lib/gitlab/user_activities/activity_set_spec.rb b/spec/lib/gitlab/user_activities/activity_set_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..56745bdf0d12ac39e9f03baaf2ab4a4ef619b237 --- /dev/null +++ b/spec/lib/gitlab/user_activities/activity_set_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Gitlab::UserActivities::ActivitySet, :redis, lib: true do + let(:user) { create(:user) } + + it 'shows the last user activity' do + Timecop.freeze do + user.record_activity + + expect(described_class.new.activities.first).to be_an_instance_of(Gitlab::UserActivities::Activity) + end + end + + context 'pagination delegation' do + let(:pagination_delegate) do + Gitlab::PaginationDelegate.new(page: 1, + per_page: 10, + count: 20) + end + + let(:delegated_methods) { %i[total_count total_pages current_page limit_value first_page? prev_page last_page? next_page] } + + before do + allow(described_class.new).to receive(:pagination_delegate).and_return(pagination_delegate) + end + + it 'includes the delegated methods' do + expect(described_class.new.public_methods).to include(*delegated_methods) + end + end + + context 'paginated activities' do + before do + Timecop.scale(3600) + + 7.times do + create(:user).record_activity + end + end + + after do + Timecop.return + end + + it 'shows the 5 oldest user activities paginated' do + expect(described_class.new(per_page: 5).activities.count).to eq(5) + end + + it 'shows the 2 reamining user activities paginated' do + expect(described_class.new(per_page: 5, page: 2).activities.count).to eq(2) + end + + it 'shows the oldest first' do + activities = described_class.new.activities + + expect(activities.first.last_activity_at).to be < activities.last.last_activity_at + end + end + + context 'filter by date' do + before do + create(:user).record_activity + end + + it 'shows activities from today' do + today = Date.today.to_s("%Y-%m-%d") + + expect(described_class.new(from: today).activities.count).to eq(1) + end + + it 'filter activities from tomorrow' do + tomorrow = Date.tomorrow.to_s("%Y-%m-%d") + + expect(described_class.new(from: tomorrow).activities.count).to eq(0) + end + end +end diff --git a/spec/lib/gitlab/user_activities/activity_spec.rb b/spec/lib/gitlab/user_activities/activity_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6a1150f50c1b2744340f5e074779160fc28fe8b1 --- /dev/null +++ b/spec/lib/gitlab/user_activities/activity_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe Gitlab::UserActivities::Activity, :redis, lib: true do + let(:username) { 'user' } + let(:activity) { described_class.new('user', Time.new(2016, 12, 12).to_i) } + + it 'has the username' do + expect(activity.username).to eq(username) + end + + it 'has the last activity at' do + expect(activity.last_activity_at).to eq('2016-12-12 00:00:00') + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index f793c0db2f34eabb49560237e20cfb2e3fc5fe27..a4e8d8e4156a8ed5905013d663b45bbf203fcec8 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1,12 +1,12 @@ require 'spec_helper' -describe API::Users, api: true do +describe API::Users, api: true do include ApiHelpers - let(:user) { create(:user) } + let(:user) { create(:user) } let(:admin) { create(:admin) } - let(:key) { create(:key, user: user) } - let(:email) { create(:email, user: user) } + let(:key) { create(:key, user: user) } + let(:email) { create(:email, user: user) } let(:omniauth_user) { create(:omniauth_user) } let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } @@ -129,7 +129,7 @@ describe API::Users, api: true do end describe "POST /users" do - before{ admin } + before { admin } it "creates user" do expect do @@ -214,9 +214,9 @@ describe API::Users, api: true do it "does not create user with invalid email" do post api('/users', admin), - email: 'invalid email', - password: 'password', - name: 'test' + email: 'invalid email', + password: 'password', + name: 'test' expect(response).to have_http_status(400) end @@ -242,12 +242,12 @@ describe API::Users, api: true do it 'returns 400 error if user does not validate' do post api('/users', admin), - password: 'pass', - email: 'test@example.com', - username: 'test!', - name: 'test', - bio: 'g' * 256, - projects_limit: -1 + password: 'pass', + email: 'test@example.com', + username: 'test!', + name: 'test', + bio: 'g' * 256, + projects_limit: -1 expect(response).to have_http_status(400) expect(json_response['message']['password']). to eq(['is too short (minimum is 8 characters)']) @@ -267,19 +267,19 @@ describe API::Users, api: true do context 'with existing user' do before do post api('/users', admin), - email: 'test@example.com', - password: 'password', - username: 'test', - name: 'foo' + email: 'test@example.com', + password: 'password', + username: 'test', + name: 'foo' end it 'returns 409 conflict error if user with same email exists' do expect do post api('/users', admin), - name: 'foo', - email: 'test@example.com', - password: 'password', - username: 'foo' + name: 'foo', + email: 'test@example.com', + password: 'password', + username: 'foo' end.to change { User.count }.by(0) expect(response).to have_http_status(409) expect(json_response['message']).to eq('Email has already been taken') @@ -288,10 +288,10 @@ describe API::Users, api: true do it 'returns 409 conflict error if same username exists' do expect do post api('/users', admin), - name: 'foo', - email: 'foo@example.com', - password: 'password', - username: 'test' + name: 'foo', + email: 'foo@example.com', + password: 'password', + username: 'test' end.to change { User.count }.by(0) expect(response).to have_http_status(409) expect(json_response['message']).to eq('Username has already been taken') @@ -416,12 +416,12 @@ describe API::Users, api: true do it 'returns 400 error if user does not validate' do put api("/users/#{user.id}", admin), - password: 'pass', - email: 'test@example.com', - username: 'test!', - name: 'test', - bio: 'g' * 256, - projects_limit: -1 + password: 'pass', + email: 'test@example.com', + username: 'test!', + name: 'test', + bio: 'g' * 256, + projects_limit: -1 expect(response).to have_http_status(400) expect(json_response['message']['password']). to eq(['is too short (minimum is 8 characters)']) @@ -488,7 +488,7 @@ describe API::Users, api: true do key_attrs = attributes_for :key expect do post api("/users/#{user.id}/keys", admin), key_attrs - end.to change{ user.keys.count }.by(1) + end.to change { user.keys.count }.by(1) end it "returns 400 for invalid ID" do @@ -580,7 +580,7 @@ describe API::Users, api: true do email_attrs = attributes_for :email expect do post api("/users/#{user.id}/emails", admin), email_attrs - end.to change{ user.emails.count }.by(1) + end.to change { user.emails.count }.by(1) end it "returns a 400 for invalid ID" do @@ -842,7 +842,7 @@ describe API::Users, api: true do key_attrs = attributes_for :key expect do post api("/user/keys", user), key_attrs - end.to change{ user.keys.count }.by(1) + end.to change { user.keys.count }.by(1) expect(response).to have_http_status(201) end @@ -880,7 +880,7 @@ describe API::Users, api: true do delete api("/user/keys/#{key.id}", user) expect(response).to have_http_status(204) - end.to change{user.keys.count}.by(-1) + end.to change { user.keys.count}.by(-1) end it "returns 404 if key ID not found" do @@ -963,7 +963,7 @@ describe API::Users, api: true do email_attrs = attributes_for :email expect do post api("/user/emails", user), email_attrs - end.to change{ user.emails.count }.by(1) + end.to change { user.emails.count }.by(1) expect(response).to have_http_status(201) end @@ -989,7 +989,7 @@ describe API::Users, api: true do delete api("/user/emails/#{email.id}", user) expect(response).to have_http_status(204) - end.to change{user.emails.count}.by(-1) + end.to change { user.emails.count}.by(-1) end it "returns 404 if email ID not found" do diff --git a/spec/requests/api/users_spec.rb.rej b/spec/requests/api/users_spec.rb.rej new file mode 100644 index 0000000000000000000000000000000000000000..f7ade32ce429496a6d005bf5a4b29a6bd564403f --- /dev/null +++ b/spec/requests/api/users_spec.rb.rej @@ -0,0 +1,124 @@ +diff a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb (rejected hunks) +@@ -1,12 +1,12 @@ + require 'spec_helper' + +-describe API::Users, api: true do ++describe API::Users, api: true do + include ApiHelpers + +- let(:user) { create(:user) } ++ let(:user) { create(:user) } + let(:admin) { create(:admin) } +- let(:key) { create(:key, user: user) } +- let(:email) { create(:email, user: user) } ++ let(:key) { create(:key, user: user) } ++ let(:email) { create(:email, user: user) } + let(:omniauth_user) { create(:omniauth_user) } + let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } + let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } +@@ -827,7 +827,7 @@ describe API::Users, api: true do + user.save + expect do + delete api("/user/keys/#{key.id}", user) +- end.to change{user.keys.count}.by(-1) ++ end.to change { user.keys.count }.by(-1) + expect(response).to have_http_status(200) + end + +@@ -931,7 +931,7 @@ describe API::Users, api: true do + user.save + expect do + delete api("/user/emails/#{email.id}", user) +- end.to change{user.emails.count}.by(-1) ++ end.to change { user.emails.count }.by(-1) + expect(response).to have_http_status(200) + end + +@@ -984,7 +984,7 @@ describe API::Users, api: true do + end + + describe 'PUT /users/:id/unblock' do +- let(:blocked_user) { create(:user, state: 'blocked') } ++ let(:blocked_user) { create(:user, state: 'blocked') } + before { admin } + + it 'unblocks existing user' do +@@ -1100,4 +1100,78 @@ describe API::Users, api: true do + expect(json_response['message']).to eq('404 User Not Found') + end + end ++ ++ context "user activities", :redis do ++ it_behaves_like 'a paginated resources' do ++ let(:request) { get api("/user/activities", admin) } ++ end ++ ++ context 'last activity as normal user' do ++ it 'has no permission' do ++ user.record_activity ++ ++ get api("/user/activities", user) ++ ++ expect(response).to have_http_status(403) ++ end ++ end ++ ++ context 'last activity as admin' do ++ it 'returns the last activity' do ++ allow(Time).to receive(:now).and_return(Time.new(2000, 1, 1)) ++ ++ user.record_activity ++ ++ get api("/user/activities", admin) ++ ++ activity = json_response.last ++ ++ expect(activity['username']).to eq(user.username) ++ expect(activity['last_activity_at']).to eq('2000-01-01 00:00:00') ++ end ++ end ++ ++ context 'last activities paginated', :redis do ++ let(:activity) { json_response.first } ++ let(:old_date) { 2.months.ago.to_date } ++ ++ before do ++ 5.times do |num| ++ Timecop.freeze(old_date + num) ++ ++ create(:user, username: num.to_s).record_activity ++ end ++ end ++ ++ after do ++ Timecop.return ++ end ++ ++ it 'returns 3 activities' do ++ get api("/user/activities?page=1&per_page=3", admin) ++ ++ expect(json_response.count).to eq(3) ++ end ++ ++ it 'contains the first activities' do ++ get api("/user/activities?page=1&per_page=3", admin) ++ ++ expect(json_response.map { |activity| activity['username'] }).to eq(%w[0 1 2]) ++ end ++ ++ it 'contains the last activities' do ++ get api("/user/activities?page=2&per_page=3", admin) ++ ++ expect(json_response.map { |activity| activity['username'] }).to eq(%w[3 4]) ++ end ++ ++ it 'contains activities created after user 3 was created' do ++ from = (old_date + 3).to_s("%Y-%m-%d") ++ ++ get api("/user/activities?page=1&per_page=5&from=#{from}", admin) ++ ++ expect(json_response.map { |activity| activity['username'] }).to eq(%w[3 4]) ++ end ++ end ++ end + end