Commit ffc78215 authored by Andy Soiron's avatar Andy Soiron Committed by Bob Van Landuyt

Remove unused attribute from ActivityService

the `activity` attribute is not used anymore and
can be removed. This commit also adds a spec
for the happy path of git http info_refs to test
the new attributes on ActivityService
parent 403b60b1
...@@ -21,7 +21,7 @@ module RecordUserLastActivity ...@@ -21,7 +21,7 @@ module RecordUserLastActivity
return if Gitlab::Database.read_only? return if Gitlab::Database.read_only?
if current_user && current_user.last_activity_on != Date.today if current_user && current_user.last_activity_on != Date.today
Users::ActivityService.new(current_user, "visited #{request.path}").execute Users::ActivityService.new(current_user).execute
end end
end end
end end
...@@ -109,7 +109,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController ...@@ -109,7 +109,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
end end
def log_user_activity def log_user_activity
Users::ActivityService.new(user, 'pull').execute Users::ActivityService.new(user).execute
end end
end end
......
...@@ -262,7 +262,7 @@ class SessionsController < Devise::SessionsController ...@@ -262,7 +262,7 @@ class SessionsController < Devise::SessionsController
def log_user_activity(user) def log_user_activity(user)
login_counter.increment login_counter.increment
Users::ActivityService.new(user, 'login').execute Users::ActivityService.new(user).execute
end end
def load_recaptcha def load_recaptcha
......
...@@ -101,7 +101,7 @@ class EventCreateService ...@@ -101,7 +101,7 @@ class EventCreateService
Users::LastPushEventService.new(current_user) Users::LastPushEventService.new(current_user)
.cache_last_push_event(event) .cache_last_push_event(event)
Users::ActivityService.new(current_user, 'push').execute Users::ActivityService.new(current_user).execute
end end
def create_event(resource_parent, current_user, status, attributes = {}) def create_event(resource_parent, current_user, status, attributes = {})
......
...@@ -4,7 +4,7 @@ module Users ...@@ -4,7 +4,7 @@ module Users
class ActivityService class ActivityService
LEASE_TIMEOUT = 1.minute.to_i LEASE_TIMEOUT = 1.minute.to_i
def initialize(author, activity) def initialize(author)
@user = if author.respond_to?(:username) @user = if author.respond_to?(:username)
author author
elsif author.respond_to?(:user) elsif author.respond_to?(:user)
...@@ -12,7 +12,6 @@ module Users ...@@ -12,7 +12,6 @@ module Users
end end
@user = nil unless @user.is_a?(User) @user = nil unless @user.is_a?(User)
@activity = activity
end end
def execute def execute
......
...@@ -52,7 +52,7 @@ module API ...@@ -52,7 +52,7 @@ module API
def log_user_activity(actor) def log_user_activity(actor)
commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS
::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action]) ::Users::ActivityService.new(actor).execute if commands.include?(params[:action])
end end
def merge_request_urls def merge_request_urls
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe Projects::GitHttpController do describe Projects::GitHttpController do
include GitHttpHelpers
let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:project) { create(:project, :public, :repository) }
let(:project_params) do let(:project_params) do
{ {
...@@ -31,6 +33,28 @@ describe Projects::GitHttpController do ...@@ -31,6 +33,28 @@ describe Projects::GitHttpController do
expect(response.status).to eq(401) expect(response.status).to eq(401)
end end
context 'with authorized user' do
let(:user) { project.owner }
before do
request.headers.merge! auth_env(user.username, user.password, nil)
end
it 'returns 200' do
get :info_refs, params: params
expect(response.status).to eq(200)
end
it 'updates the user activity' do
expect_next_instance_of(Users::ActivityService) do |activity_service|
expect(activity_service).to receive(:execute)
end
get :info_refs, params: params
end
end
context 'with exceptions' do context 'with exceptions' do
before do before do
allow(controller).to receive(:verify_workhorse_api!).and_return(true) allow(controller).to receive(:verify_workhorse_api!).and_return(true)
......
...@@ -7,7 +7,7 @@ describe Users::ActivityService do ...@@ -7,7 +7,7 @@ describe Users::ActivityService do
let(:user) { create(:user, last_activity_on: last_activity_on) } let(:user) { create(:user, last_activity_on: last_activity_on) }
subject { described_class.new(user, 'type') } subject { described_class.new(user) }
describe '#execute', :clean_gitlab_redis_shared_state do describe '#execute', :clean_gitlab_redis_shared_state do
context 'when last activity is nil' do context 'when last activity is nil' do
...@@ -40,7 +40,7 @@ describe Users::ActivityService do ...@@ -40,7 +40,7 @@ describe Users::ActivityService do
let(:fake_object) { double(username: 'hello') } let(:fake_object) { double(username: 'hello') }
it 'does not record activity' do it 'does not record activity' do
service = described_class.new(fake_object, 'pull') service = described_class.new(fake_object)
expect(service).not_to receive(:record_activity) expect(service).not_to receive(:record_activity)
......
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