Commit dcf3425d authored by Magdalena Frankiewicz's avatar Magdalena Frankiewicz Committed by Imre Farkas

Add approved type of event

And approve_mr method
parent ef80d9ff
...@@ -21,6 +21,7 @@ class Event < ApplicationRecord ...@@ -21,6 +21,7 @@ class Event < ApplicationRecord
LEFT = 9 # User left project LEFT = 9 # User left project
DESTROYED = 10 DESTROYED = 10
EXPIRED = 11 # User left project due to expiry EXPIRED = 11 # User left project due to expiry
APPROVED = 12
ACTIONS = HashWithIndifferentAccess.new( ACTIONS = HashWithIndifferentAccess.new(
created: CREATED, created: CREATED,
...@@ -33,7 +34,8 @@ class Event < ApplicationRecord ...@@ -33,7 +34,8 @@ class Event < ApplicationRecord
joined: JOINED, joined: JOINED,
left: LEFT, left: LEFT,
destroyed: DESTROYED, destroyed: DESTROYED,
expired: EXPIRED expired: EXPIRED,
approved: APPROVED
).freeze ).freeze
WIKI_ACTIONS = [CREATED, UPDATED, DESTROYED].freeze WIKI_ACTIONS = [CREATED, UPDATED, DESTROYED].freeze
......
...@@ -13,6 +13,7 @@ module EE ...@@ -13,6 +13,7 @@ module EE
scope :created, -> { where(action: ::Event::CREATED) } scope :created, -> { where(action: ::Event::CREATED) }
scope :closed, -> { where(action: ::Event::CLOSED) } scope :closed, -> { where(action: ::Event::CLOSED) }
scope :merged, -> { where(action: ::Event::MERGED) } scope :merged, -> { where(action: ::Event::MERGED) }
scope :approved, -> { where(action: ::Event::APPROVED) }
scope :totals_by_author, -> { group(:author_id).count } scope :totals_by_author, -> { group(:author_id).count }
scope :totals_by_author_target_type_action, -> { group(:author_id, :target_type, :action).count } scope :totals_by_author_target_type_action, -> { group(:author_id, :target_type, :action).count }
scope :epics, -> { where(target_type: 'Epic') } scope :epics, -> { where(target_type: 'Epic') }
...@@ -29,6 +30,15 @@ module EE ...@@ -29,6 +30,15 @@ module EE
end end
end end
override :action_name
def action_name
if approved_action?
'approved'
else
super
end
end
def epic_note? def epic_note?
note? && note_target.is_a?(::Epic) note? && note_target.is_a?(::Epic)
end end
...@@ -36,5 +46,9 @@ module EE ...@@ -36,5 +46,9 @@ module EE
def epic? def epic?
target_type == 'Epic' target_type == 'Epic'
end end
def approved_action?
action == ::Event::APPROVED
end
end end
end end
...@@ -13,5 +13,9 @@ module EE ...@@ -13,5 +13,9 @@ module EE
def reopen_epic(epic, current_user) def reopen_epic(epic, current_user)
create_record_event(epic, current_user, ::Event::REOPENED) create_record_event(epic, current_user, ::Event::REOPENED)
end end
def approve_mr(merge_request, current_user)
create_record_event(merge_request, current_user, ::Event::APPROVED)
end
end end
end end
...@@ -13,10 +13,9 @@ module MergeRequests ...@@ -13,10 +13,9 @@ module MergeRequests
if save_approval(approval) if save_approval(approval)
merge_request.reset_approval_cache! merge_request.reset_approval_cache!
create_event(merge_request)
create_approval_note(merge_request) create_approval_note(merge_request)
mark_pending_todos_as_done(merge_request) mark_pending_todos_as_done(merge_request)
calculate_approvals_metrics(merge_request)
if merge_request.approvals_left.zero? if merge_request.approvals_left.zero?
notification_service.async.approve_mr(merge_request, current_user) notification_service.async.approve_mr(merge_request, current_user)
...@@ -51,7 +50,16 @@ module MergeRequests ...@@ -51,7 +50,16 @@ module MergeRequests
def calculate_approvals_metrics(merge_request) def calculate_approvals_metrics(merge_request)
return unless merge_request.project.feature_available?(:code_review_analytics) return unless merge_request.project.feature_available?(:code_review_analytics)
::Analytics::RefreshApprovalsData.new(merge_request).execute_async ::Analytics::RefreshApprovalsData.new(merge_request).execute
end
def create_event(merge_request)
# Making sure MergeRequest::Metrics updates are in sync with
# Event creation.
Event.transaction do
event_service.approve_mr(merge_request, current_user)
calculate_approvals_metrics(merge_request)
end
end end
end end
end end
...@@ -9,5 +9,6 @@ FactoryBot.modify do ...@@ -9,5 +9,6 @@ FactoryBot.modify do
action { Event::CREATED } action { Event::CREATED }
project { nil } project { nil }
end end
trait(:approved) { action { Event::APPROVED } }
end end
end end
...@@ -121,4 +121,24 @@ describe Event do ...@@ -121,4 +121,24 @@ describe Event do
end end
end end
end end
describe '#action_name' do
let_it_be(:approved_event) {create(:event, :approved)}
let_it_be(:created_event) {create(:event, :created)}
it 'returns the appropriate action name' do
expect(approved_event.action_name).to eq 'approved'
expect(created_event.action_name).to eq 'created'
end
end
describe '#approved_action?' do
let_it_be(:approved_event) {create(:event, :approved)}
let_it_be(:created_event) {create(:event, :created)}
it 'return true only for approved event type' do
expect(approved_event.approved_action?).to be true
expect(created_event.approved_action?).to be false
end
end
end end
...@@ -49,4 +49,19 @@ describe EventCreateService do ...@@ -49,4 +49,19 @@ describe EventCreateService do
expect(event.group_id).to eq epic.group_id expect(event.group_id).to eq epic.group_id
end end
end end
describe 'Merge Requests' do
let_it_be(:user) { create(:user) }
let(:merge_request) { create(:merge_request) }
describe '#approve_mr' do
it { expect(service.approve_mr(merge_request, user)).to be_truthy }
it 'creates new event' do
service.approve_mr(merge_request, user)
change { Event.approved.where(target: merge_request).count }.by(1)
end
end
end
end end
...@@ -52,6 +52,15 @@ describe MergeRequests::ApprovalService do ...@@ -52,6 +52,15 @@ describe MergeRequests::ApprovalService do
service.execute(merge_request) service.execute(merge_request)
end end
it 'creates approve MR event' do
expect_next_instance_of(EventCreateService) do |instance|
expect(instance).to receive(:approve_mr)
.with(merge_request, user)
end
service.execute(merge_request)
end
context 'with remaining approvals' do context 'with remaining approvals' do
it 'fires an approval webhook' do it 'fires an approval webhook' do
expect(merge_request).to receive(:approvals_left).and_return(5) expect(merge_request).to receive(:approvals_left).and_return(5)
...@@ -97,8 +106,8 @@ describe MergeRequests::ApprovalService do ...@@ -97,8 +106,8 @@ describe MergeRequests::ApprovalService do
end end
it 'schedules RefreshApprovalsData' do it 'schedules RefreshApprovalsData' do
expect(Analytics::CodeReviewMetricsWorker) expect(::Analytics::RefreshApprovalsData)
.to receive(:perform_async).with('Analytics::RefreshApprovalsData', merge_request.id, {}) .to receive_message_chain(:new, :execute)
service.execute(merge_request) service.execute(merge_request)
end end
...@@ -110,7 +119,7 @@ describe MergeRequests::ApprovalService do ...@@ -110,7 +119,7 @@ describe MergeRequests::ApprovalService do
end end
it 'does not schedule for RefreshApprovalsData' do it 'does not schedule for RefreshApprovalsData' do
expect(Analytics::CodeReviewMetricsWorker).not_to receive(:perform_async) expect(::Analytics::RefreshApprovalsData).not_to receive(:new)
service.execute(merge_request) service.execute(merge_request)
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