BigW Consortium Gitlab

Commit 380e40fe by Sean McGivern Committed by Rémy Coutable

Remove unused user activities code

parent 00e9568e
......@@ -949,10 +949,6 @@ class User < ActiveRecord::Base
end
end
def record_activity
Gitlab::UserActivities::ActivitySet.record(self)
end
private
def access_level=(new_level)
......
......@@ -212,8 +212,8 @@ Settings.gitlab['email_from'] ||= ENV['GITLAB_EMAIL_FROM'] || "gitlab@#{Settings
Settings.gitlab['email_display_name'] ||= ENV['GITLAB_EMAIL_DISPLAY_NAME'] || 'GitLab'
Settings.gitlab['email_reply_to'] ||= ENV['GITLAB_EMAIL_REPLY_TO'] || "noreply@#{Settings.gitlab.host}"
Settings.gitlab['email_subject_suffix'] ||= ENV['GITLAB_EMAIL_SUBJECT_SUFFIX'] || ""
Settings.gitlab['base_url'] ||= Settings.send(:build_base_gitlab_url)
Settings.gitlab['url'] ||= Settings.send(:build_gitlab_url)
Settings.gitlab['base_url'] ||= Settings.__send__(:build_base_gitlab_url)
Settings.gitlab['url'] ||= Settings.__send__(:build_gitlab_url)
Settings.gitlab['user'] ||= 'git'
Settings.gitlab['user_home'] ||= begin
Etc.getpwnam(Settings.gitlab['user']).dir
......
class CreateUserActivities < ActiveRecord::Migration
# Set this constant to true if this migration requires downtime.
DOWNTIME = true
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
DOWNTIME_REASON = 'Adding foreign key'
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
DOWNTIME = false
# This migration is a no-op. It just exists to match EE.
def change
create_table :user_activities do |t|
t.belongs_to :user, index: { unique: true }, foreign_key: { on_delete: :cascade }
t.datetime :last_activity_at, null: false
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class DropUserActivitiesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
# This migration is a no-op. It just exists to match EE.
def change
drop_table :user_activities
end
end
......@@ -62,8 +62,7 @@ module API
end
def log_user_activity(actor)
commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS +
Gitlab::GitAccess::GIT_ANNEX_COMMANDS
commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS
::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action])
end
......
......@@ -15,16 +15,6 @@ module API
# project - project path with namespace
# action - git action (git-upload-pack or git-receive-pack)
# changes - changes as "oldrev newrev ref", see Gitlab::ChangesList
helpers do
def log_user_activity(actor)
commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS +
Gitlab::GitAccess::PUSH_COMMANDS +
Gitlab::GitAccess::GIT_ANNEX_COMMANDS
::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action])
end
end
post "/allowed" do
status 200
......
......@@ -535,7 +535,6 @@ module API
current_user.update_secondary_emails!
end
desc 'Get a list of user activities'
params do
optional :from, type: DateTime, default: 6.months.ago, desc: 'Date string in the format YEAR-MONTH-DAY'
......
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
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 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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment