Commit 16d41a4a authored by Robert May's avatar Robert May

Various optimisations and improvements

parent b0d7832c
...@@ -9,8 +9,6 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -9,8 +9,6 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker
weight 3 weight 3
loggable_arguments 2, 3, 4 loggable_arguments 2, 3, 4
LOG_TIME_THRESHOLD = 90 # seconds
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def perform(project_id, user_id, oldrev, newrev, ref) def perform(project_id, user_id, oldrev, newrev, ref)
project = Project.find_by(id: project_id) project = Project.find_by(id: project_id)
......
...@@ -161,7 +161,7 @@ ...@@ -161,7 +161,7 @@
- - merge_request_mergeability_check - - merge_request_mergeability_check
- 1 - 1
- - merge_request_reset_approvals - - merge_request_reset_approvals
- 3 - 1
- - metrics_dashboard_prune_old_annotations - - metrics_dashboard_prune_old_annotations
- 1 - 1
- - migrate_external_diffs - - migrate_external_diffs
......
...@@ -640,7 +640,7 @@ ...@@ -640,7 +640,7 @@
:has_external_dependencies: :has_external_dependencies:
:urgency: :high :urgency: :high
:resource_boundary: :cpu :resource_boundary: :cpu
:weight: 3 :weight: 1
:idempotent: :idempotent:
:tags: [] :tags: []
- :name: new_epic - :name: new_epic
......
...@@ -6,11 +6,8 @@ class MergeRequestResetApprovalsWorker # rubocop:disable Scalability/IdempotentW ...@@ -6,11 +6,8 @@ class MergeRequestResetApprovalsWorker # rubocop:disable Scalability/IdempotentW
feature_category :source_code_management feature_category :source_code_management
urgency :high urgency :high
worker_resource_boundary :cpu worker_resource_boundary :cpu
weight 3
loggable_arguments 2, 3 loggable_arguments 2, 3
LOG_TIME_THRESHOLD = 90 # seconds
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def perform(project_id, user_id, ref, newrev) def perform(project_id, user_id, ref, newrev)
project = Project.find_by(id: project_id) project = Project.find_by(id: project_id)
......
...@@ -343,7 +343,7 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -343,7 +343,7 @@ RSpec.describe MergeRequests::RefreshService do
Todo.where(action: Todo::APPROVAL_REQUIRED, target: merge_request) Todo.where(action: Todo::APPROVAL_REQUIRED, target: merge_request)
end end
context 'push to origin repo source branch' do context 'push to origin repo source branch', :sidekiq_inline do
let(:notification_service) { spy('notification_service') } let(:notification_service) { spy('notification_service') }
before do before do
...@@ -354,7 +354,6 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -354,7 +354,6 @@ RSpec.describe MergeRequests::RefreshService do
it 'resets approvals' do it 'resets approvals' do
service.execute(oldrev, newrev, 'refs/heads/master') service.execute(oldrev, newrev, 'refs/heads/master')
reload_mrs reload_mrs
MergeRequestResetApprovalsWorker.drain
expect(merge_request.approvals).to be_empty expect(merge_request.approvals).to be_empty
expect(forked_merge_request.approvals).not_to be_empty expect(forked_merge_request.approvals).not_to be_empty
...@@ -446,14 +445,13 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -446,14 +445,13 @@ RSpec.describe MergeRequests::RefreshService do
end end
end end
context 'resetting approvals if they are enabled' do context 'resetting approvals if they are enabled', :sidekiq_inline do
context 'when approvals_before_merge is disabled' do context 'when approvals_before_merge is disabled' do
before do before do
project.update(approvals_before_merge: 0) project.update(approvals_before_merge: 0)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(oldrev, newrev, 'refs/heads/master') service.execute(oldrev, newrev, 'refs/heads/master')
reload_mrs reload_mrs
MergeRequestResetApprovalsWorker.drain
end end
it 'resets approvals' do it 'resets approvals' do
...@@ -490,14 +488,13 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -490,14 +488,13 @@ RSpec.describe MergeRequests::RefreshService do
end end
end end
context 'when there are approvals' do context 'when there are approvals', :sidekiq_inline do
context 'closed merge request' do context 'closed merge request' do
before do before do
merge_request.close! merge_request.close!
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(oldrev, newrev, 'refs/heads/master') service.execute(oldrev, newrev, 'refs/heads/master')
reload_mrs reload_mrs
MergeRequestResetApprovalsWorker.drain
end end
it 'resets the approvals' do it 'resets the approvals' do
...@@ -511,7 +508,6 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -511,7 +508,6 @@ RSpec.describe MergeRequests::RefreshService do
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(oldrev, newrev, 'refs/heads/master') service.execute(oldrev, newrev, 'refs/heads/master')
reload_mrs reload_mrs
MergeRequestResetApprovalsWorker.drain
end end
it 'resets the approvals' do it 'resets the approvals' do
......
...@@ -3,9 +3,6 @@ ...@@ -3,9 +3,6 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe EE::MergeRequests::ResetApprovalsService do RSpec.describe EE::MergeRequests::ResetApprovalsService do
include ProjectForksHelper
include ProjectHelpers
let(:service) { described_class.new(project, current_user) } let(:service) { described_class.new(project, current_user) }
let(:current_user) { merge_request.author } let(:current_user) { merge_request.author }
let(:group) { create(:group) } let(:group) { create(:group) }
...@@ -34,7 +31,6 @@ RSpec.describe EE::MergeRequests::ResetApprovalsService do ...@@ -34,7 +31,6 @@ RSpec.describe EE::MergeRequests::ResetApprovalsService do
describe "#execute" do describe "#execute" do
before do before do
merge_request.approvals.create!(user_id: user.id)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
allow(NotificationService).to receive(:new) { notification_service } allow(NotificationService).to receive(:new) { notification_service }
project.add_developer(approver) project.add_developer(approver)
...@@ -42,6 +38,8 @@ RSpec.describe EE::MergeRequests::ResetApprovalsService do ...@@ -42,6 +38,8 @@ RSpec.describe EE::MergeRequests::ResetApprovalsService do
perform_enqueued_jobs do perform_enqueued_jobs do
merge_request.update!(approver_ids: [approver].map(&:id).join(',')) merge_request.update!(approver_ids: [approver].map(&:id).join(','))
end end
merge_request.approvals.create!(user_id: approver.id)
end end
it 'resets approvals' do it 'resets approvals' do
......
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