Commit c5b72a34 authored by Sean McGivern's avatar Sean McGivern

Merge branch '213334-add-resource-state-events-pd' into 'master'

Track resource state events

See merge request gitlab-org/gitlab!29015
parents 5ea1d403 d2cc807b
...@@ -11,30 +11,44 @@ class EventCreateService ...@@ -11,30 +11,44 @@ class EventCreateService
IllegalActionError = Class.new(StandardError) IllegalActionError = Class.new(StandardError)
def open_issue(issue, current_user) def open_issue(issue, current_user)
create_resource_event(issue, current_user, :opened)
create_record_event(issue, current_user, Event::CREATED) create_record_event(issue, current_user, Event::CREATED)
end end
def close_issue(issue, current_user) def close_issue(issue, current_user)
create_resource_event(issue, current_user, :closed)
create_record_event(issue, current_user, Event::CLOSED) create_record_event(issue, current_user, Event::CLOSED)
end end
def reopen_issue(issue, current_user) def reopen_issue(issue, current_user)
create_resource_event(issue, current_user, :reopened)
create_record_event(issue, current_user, Event::REOPENED) create_record_event(issue, current_user, Event::REOPENED)
end end
def open_mr(merge_request, current_user) def open_mr(merge_request, current_user)
create_resource_event(merge_request, current_user, :opened)
create_record_event(merge_request, current_user, Event::CREATED) create_record_event(merge_request, current_user, Event::CREATED)
end end
def close_mr(merge_request, current_user) def close_mr(merge_request, current_user)
create_resource_event(merge_request, current_user, :closed)
create_record_event(merge_request, current_user, Event::CLOSED) create_record_event(merge_request, current_user, Event::CLOSED)
end end
def reopen_mr(merge_request, current_user) def reopen_mr(merge_request, current_user)
create_resource_event(merge_request, current_user, :reopened)
create_record_event(merge_request, current_user, Event::REOPENED) create_record_event(merge_request, current_user, Event::REOPENED)
end end
def merge_mr(merge_request, current_user) def merge_mr(merge_request, current_user)
create_resource_event(merge_request, current_user, :merged)
create_record_event(merge_request, current_user, Event::MERGED) create_record_event(merge_request, current_user, Event::MERGED)
end end
...@@ -157,6 +171,18 @@ class EventCreateService ...@@ -157,6 +171,18 @@ class EventCreateService
Event.create!(attributes) Event.create!(attributes)
end end
def create_resource_event(issuable, current_user, status)
return unless state_change_tracking_enabled?(issuable)
ResourceEvents::ChangeStateService.new(resource: issuable, user: current_user)
.execute(status)
end
def state_change_tracking_enabled?(issuable)
issuable&.respond_to?(:resource_state_events) &&
::Feature.enabled?(:track_resource_state_change_events, issuable&.project)
end
end end
EventCreateService.prepend_if_ee('EE::EventCreateService') EventCreateService.prepend_if_ee('EE::EventCreateService')
# frozen_string_literal: true
module ResourceEvents
class ChangeStateService
attr_reader :resource, :user
def initialize(user:, resource:)
@user, @resource = user, resource
end
def execute(state)
ResourceStateEvent.create(
user: user,
issue: issue,
merge_request: merge_request,
state: ResourceStateEvent.states[state],
created_at: Time.zone.now)
resource.expire_note_etag_cache
end
private
def issue
return unless resource.is_a?(Issue)
resource
end
def merge_request
return unless resource.is_a?(MergeRequest)
resource
end
end
end
...@@ -11,7 +11,8 @@ module ResourceEvents ...@@ -11,7 +11,8 @@ module ResourceEvents
SYNTHETIC_NOTE_BUILDER_SERVICES = [ SYNTHETIC_NOTE_BUILDER_SERVICES = [
SyntheticLabelNotesBuilderService, SyntheticLabelNotesBuilderService,
SyntheticMilestoneNotesBuilderService SyntheticMilestoneNotesBuilderService,
SyntheticStateNotesBuilderService
].freeze ].freeze
attr_reader :resource, :current_user, :params attr_reader :resource, :current_user, :params
...@@ -23,7 +24,7 @@ module ResourceEvents ...@@ -23,7 +24,7 @@ module ResourceEvents
end end
def execute(notes = []) def execute(notes = [])
(notes + synthetic_notes).sort_by { |n| n.created_at } (notes + synthetic_notes).sort_by(&:created_at)
end end
private private
......
# frozen_string_literal: true
module ResourceEvents
class SyntheticStateNotesBuilderService < BaseSyntheticNotesBuilderService
private
def synthetic_notes
state_change_events.map do |event|
StateNote.from_event(event, resource: resource, resource_parent: resource_parent)
end
end
def state_change_events
return [] unless resource.respond_to?(:resource_state_events)
events = resource.resource_state_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
since_fetch_at(events)
end
end
end
...@@ -225,7 +225,12 @@ module SystemNotes ...@@ -225,7 +225,12 @@ module SystemNotes
action = status == 'reopened' ? 'opened' : status action = status == 'reopened' ? 'opened' : status
create_note(NoteSummary.new(noteable, project, author, body, action: action)) # A state event which results in a synthetic note will be
# created by EventCreateService if change event tracking
# is enabled.
unless state_change_tracking_enabled?
create_note(NoteSummary.new(noteable, project, author, body, action: action))
end
end end
# Check if a cross reference to a noteable from a mentioner already exists # Check if a cross reference to a noteable from a mentioner already exists
...@@ -318,6 +323,11 @@ module SystemNotes ...@@ -318,6 +323,11 @@ module SystemNotes
def self.cross_reference?(note_text) def self.cross_reference?(note_text)
note_text =~ /\A#{cross_reference_note_prefix}/i note_text =~ /\A#{cross_reference_note_prefix}/i
end end
def state_change_tracking_enabled?
noteable.respond_to?(:resource_state_events) &&
::Feature.enabled?(:track_resource_state_change_events, noteable.project)
end
end end
end end
......
...@@ -65,16 +65,24 @@ describe Gitlab::Email::Handler::CreateNoteHandler do ...@@ -65,16 +65,24 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
end end
end end
context 'and current user can update noteable' do [true, false].each do |state_tracking_enabled|
before do context "and current user can update noteable #{state_tracking_enabled ? 'enabled' : 'disabled'}" do
project.add_developer(user) before do
end stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
it 'does not raise an error' do project.add_developer(user)
# One system note is created for the 'close' event end
expect { receiver.execute }.to change { noteable.notes.count }.by(1)
it 'does not raise an error' do
expect(noteable.reload).to be_closed if state_tracking_enabled
expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1)
else
# One system note is created for the 'close' event
expect { receiver.execute }.to change { noteable.notes.count }.by(1)
end
expect(noteable.reload).to be_closed
end
end end
end end
end end
......
...@@ -2157,7 +2157,34 @@ describe MergeRequest do ...@@ -2157,7 +2157,34 @@ describe MergeRequest do
end end
end end
context 'when merging note is persisted, but no metrics or merge event exists' do context 'when state event tracking is disabled' do
before do
stub_feature_flags(track_resource_state_change_events: false)
end
context 'when merging note is persisted, but no metrics or merge event exists' do
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request, :merged) }
before do
merge_request.metrics.destroy!
SystemNoteService.change_status(merge_request,
merge_request.target_project,
user,
merge_request.state, nil)
end
it 'returns merging note creation date' do
expect(merge_request.reload.metrics).to be_nil
expect(merge_request.merge_event).to be_nil
expect(merge_request.notes.count).to eq(1)
expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at)
end
end
end
context 'when state event tracking is enabled' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:merge_request) { create(:merge_request, :merged) } let(:merge_request) { create(:merge_request, :merged) }
...@@ -2170,11 +2197,8 @@ describe MergeRequest do ...@@ -2170,11 +2197,8 @@ describe MergeRequest do
merge_request.state, nil) merge_request.state, nil)
end end
it 'returns merging note creation date' do it 'does not create a system note' do
expect(merge_request.reload.metrics).to be_nil expect(merge_request.notes).to be_empty
expect(merge_request.merge_event).to be_nil
expect(merge_request.notes.count).to eq(1)
expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at)
end end
end end
end end
......
...@@ -13,6 +13,7 @@ describe EventCreateService do ...@@ -13,6 +13,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.open_issue(issue, issue.author) }.to change { Event.count } expect { service.open_issue(issue, issue.author) }.to change { Event.count }
expect { service.open_issue(issue, issue.author) }.to change { ResourceStateEvent.count }
end end
end end
...@@ -23,6 +24,7 @@ describe EventCreateService do ...@@ -23,6 +24,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.close_issue(issue, issue.author) }.to change { Event.count } expect { service.close_issue(issue, issue.author) }.to change { Event.count }
expect { service.close_issue(issue, issue.author) }.to change { ResourceStateEvent.count }
end end
end end
...@@ -33,6 +35,7 @@ describe EventCreateService do ...@@ -33,6 +35,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.reopen_issue(issue, issue.author) }.to change { Event.count } expect { service.reopen_issue(issue, issue.author) }.to change { Event.count }
expect { service.reopen_issue(issue, issue.author) }.to change { ResourceStateEvent.count }
end end
end end
end end
...@@ -45,6 +48,7 @@ describe EventCreateService do ...@@ -45,6 +48,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.open_mr(merge_request, merge_request.author) }.to change { Event.count } expect { service.open_mr(merge_request, merge_request.author) }.to change { Event.count }
expect { service.open_mr(merge_request, merge_request.author) }.to change { ResourceStateEvent.count }
end end
end end
...@@ -55,6 +59,7 @@ describe EventCreateService do ...@@ -55,6 +59,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.close_mr(merge_request, merge_request.author) }.to change { Event.count } expect { service.close_mr(merge_request, merge_request.author) }.to change { Event.count }
expect { service.close_mr(merge_request, merge_request.author) }.to change { ResourceStateEvent.count }
end end
end end
...@@ -65,6 +70,7 @@ describe EventCreateService do ...@@ -65,6 +70,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.merge_mr(merge_request, merge_request.author) }.to change { Event.count } expect { service.merge_mr(merge_request, merge_request.author) }.to change { Event.count }
expect { service.merge_mr(merge_request, merge_request.author) }.to change { ResourceStateEvent.count }
end end
end end
...@@ -75,6 +81,7 @@ describe EventCreateService do ...@@ -75,6 +81,7 @@ describe EventCreateService do
it "creates new event" do it "creates new event" do
expect { service.reopen_mr(merge_request, merge_request.author) }.to change { Event.count } expect { service.reopen_mr(merge_request, merge_request.author) }.to change { Event.count }
expect { service.reopen_mr(merge_request, merge_request.author) }.to change { ResourceStateEvent.count }
end end
end end
end end
......
...@@ -224,11 +224,26 @@ describe Issues::CloseService do ...@@ -224,11 +224,26 @@ describe Issues::CloseService do
expect(email.subject).to include(issue.title) expect(email.subject).to include(issue.title)
end end
it 'creates system note about issue reassign' do context 'when resource state events are disabled' do
close_issue before do
stub_feature_flags(track_resource_state_change_events: false)
end
it 'creates system note about the issue being closed' do
close_issue
note = issue.notes.last
expect(note.note).to include "closed"
end
end
note = issue.notes.last context 'when resource state events are enabled' do
expect(note.note).to include "closed" it 'creates resource state event about the issue being closed' do
close_issue
event = issue.resource_state_events.last
expect(event.state).to eq('closed')
end
end end
it 'marks todos as done' do it 'marks todos as done' do
......
...@@ -19,45 +19,54 @@ describe MergeRequests::CloseService do ...@@ -19,45 +19,54 @@ describe MergeRequests::CloseService do
describe '#execute' do describe '#execute' do
it_behaves_like 'cache counters invalidator' it_behaves_like 'cache counters invalidator'
context 'valid params' do [true, false].each do |state_tracking_enabled|
let(:service) { described_class.new(project, user, {}) } context "valid params with state_tracking #{state_tracking_enabled ? 'enabled' : 'disabled'}" do
let(:service) { described_class.new(project, user, {}) }
before do before do
allow(service).to receive(:execute_hooks) stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
perform_enqueued_jobs do allow(service).to receive(:execute_hooks)
@merge_request = service.execute(merge_request)
perform_enqueued_jobs do
@merge_request = service.execute(merge_request)
end
end end
end
it { expect(@merge_request).to be_valid } it { expect(@merge_request).to be_valid }
it { expect(@merge_request).to be_closed } it { expect(@merge_request).to be_closed }
it 'executes hooks with close action' do it 'executes hooks with close action' do
expect(service).to have_received(:execute_hooks) expect(service).to have_received(:execute_hooks)
.with(@merge_request, 'close') .with(@merge_request, 'close')
end end
it 'sends email to user2 about assign of new merge_request', :sidekiq_might_not_need_inline do it 'sends email to user2 about assign of new merge_request', :sidekiq_might_not_need_inline do
email = ActionMailer::Base.deliveries.last email = ActionMailer::Base.deliveries.last
expect(email.to.first).to eq(user2.email) expect(email.to.first).to eq(user2.email)
expect(email.subject).to include(merge_request.title) expect(email.subject).to include(merge_request.title)
end end
it 'creates system note about merge_request reassign' do it 'creates system note about merge_request reassign' do
note = @merge_request.notes.last if state_tracking_enabled
expect(note.note).to include 'closed' event = @merge_request.resource_state_events.last
end expect(event.state).to eq('closed')
else
note = @merge_request.notes.last
expect(note.note).to include 'closed'
end
end
it 'marks todos as done' do it 'marks todos as done' do
expect(todo.reload).to be_done expect(todo.reload).to be_done
end end
context 'when auto merge is enabled' do context 'when auto merge is enabled' do
let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
it 'cancels the auto merge' do it 'cancels the auto merge' do
expect(@merge_request).not_to be_auto_merge_enabled expect(@merge_request).not_to be_auto_merge_enabled
end
end end
end end
end end
......
...@@ -21,65 +21,74 @@ describe MergeRequests::FfMergeService do ...@@ -21,65 +21,74 @@ describe MergeRequests::FfMergeService do
end end
describe '#execute' do describe '#execute' do
context 'valid params' do [true, false].each do |state_tracking_enabled|
let(:service) { described_class.new(project, user, valid_merge_params) } context "valid params with state_tracking #{state_tracking_enabled ? 'enabled' : 'disabled'}" do
let(:service) { described_class.new(project, user, valid_merge_params) }
def execute_ff_merge
perform_enqueued_jobs do def execute_ff_merge
service.execute(merge_request) perform_enqueued_jobs do
service.execute(merge_request)
end
end end
end
before do before do
allow(service).to receive(:execute_hooks) stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
end
it "does not create merge commit" do allow(service).to receive(:execute_hooks)
execute_ff_merge end
source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha it "does not create merge commit" do
target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha execute_ff_merge
expect(source_branch_sha).to eq(target_branch_sha) source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha
end target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha
it 'keeps the merge request valid' do expect(source_branch_sha).to eq(target_branch_sha)
expect { execute_ff_merge } end
.not_to change { merge_request.valid? }
end
it 'updates the merge request to merged' do it 'keeps the merge request valid' do
expect { execute_ff_merge } expect { execute_ff_merge }
.to change { merge_request.merged? } .not_to change { merge_request.valid? }
.from(false) end
.to(true)
end
it 'sends email to user2 about merge of new merge_request' do it 'updates the merge request to merged' do
execute_ff_merge expect { execute_ff_merge }
.to change { merge_request.merged? }
.from(false)
.to(true)
end
email = ActionMailer::Base.deliveries.last it 'sends email to user2 about merge of new merge_request' do
expect(email.to.first).to eq(user2.email) execute_ff_merge
expect(email.subject).to include(merge_request.title)
end
it 'creates system note about merge_request merge' do email = ActionMailer::Base.deliveries.last
execute_ff_merge expect(email.to.first).to eq(user2.email)
expect(email.subject).to include(merge_request.title)
end
note = merge_request.notes.last it 'creates system note about merge_request merge' do
expect(note.note).to include 'merged' execute_ff_merge
end
it 'does not update squash_commit_sha if it is not a squash' do if state_tracking_enabled
expect { execute_ff_merge }.not_to change { merge_request.squash_commit_sha } event = merge_request.resource_state_events.last
end expect(event.state).to eq('merged')
else
note = merge_request.notes.last
expect(note.note).to include 'merged'
end
end
it 'updates squash_commit_sha if it is a squash' do it 'does not update squash_commit_sha if it is not a squash' do
merge_request.update!(squash: true) expect { execute_ff_merge }.not_to change { merge_request.squash_commit_sha }
end
expect { execute_ff_merge } it 'updates squash_commit_sha if it is a squash' do
.to change { merge_request.squash_commit_sha } merge_request.update!(squash: true)
.from(nil)
expect { execute_ff_merge }
.to change { merge_request.squash_commit_sha }
.from(nil)
end
end end
end end
......
...@@ -20,7 +20,11 @@ describe MergeRequests::MergeService do ...@@ -20,7 +20,11 @@ describe MergeRequests::MergeService do
end end
context 'valid params' do context 'valid params' do
let(:state_tracking) { true }
before do before do
stub_feature_flags(track_resource_state_change_events: state_tracking)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
perform_enqueued_jobs do perform_enqueued_jobs do
...@@ -42,9 +46,22 @@ describe MergeRequests::MergeService do ...@@ -42,9 +46,22 @@ describe MergeRequests::MergeService do
expect(email.subject).to include(merge_request.title) expect(email.subject).to include(merge_request.title)
end end
it 'creates system note about merge_request merge' do context 'note creation' do
note = merge_request.notes.last context 'when resource state event tracking is disabled' do
expect(note.note).to include 'merged' let(:state_tracking) { false }
it 'creates system note about merge_request merge' do
note = merge_request.notes.last
expect(note.note).to include 'merged'
end
end
context 'when resource state event tracking is enabled' do
it 'creates resource state event about merge_request merge' do
event = merge_request.resource_state_events.last
expect(event.state).to eq('merged')
end
end
end end
context 'when squashing' do context 'when squashing' do
...@@ -55,7 +72,7 @@ describe MergeRequests::MergeService do ...@@ -55,7 +72,7 @@ describe MergeRequests::MergeService do
end end
let(:merge_request) do let(:merge_request) do
# A merge reqeust with 5 commits # A merge request with 5 commits
create(:merge_request, :simple, create(:merge_request, :simple,
author: user2, author: user2,
assignees: [user2], assignees: [user2],
......
...@@ -362,76 +362,101 @@ describe MergeRequests::RefreshService do ...@@ -362,76 +362,101 @@ describe MergeRequests::RefreshService do
end end
end end
context 'push to origin repo target branch', :sidekiq_might_not_need_inline do [true, false].each do |state_tracking_enabled|
context 'when all MRs to the target branch had diffs' do context "push to origin repo target branch with state tracking #{state_tracking_enabled ? 'enabled' : 'disabled'}", :sidekiq_might_not_need_inline do
before do before do
service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
reload_mrs
end end
it 'updates the merge state' do context 'when all MRs to the target branch had diffs' do
expect(@merge_request.notes.last.note).to include('merged') before do
expect(@merge_request).to be_merged service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature')
expect(@fork_merge_request).to be_merged reload_mrs
expect(@fork_merge_request.notes.last.note).to include('merged') end
expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done it 'updates the merge state' do
expect(@merge_request).to be_merged
expect(@fork_merge_request).to be_merged
expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done
if state_tracking_enabled
expect(@merge_request.resource_state_events.last.state).to eq('merged')
expect(@fork_merge_request.resource_state_events.last.state).to eq('merged')
else
expect(@merge_request.notes.last.note).to include('merged')
expect(@fork_merge_request.notes.last.note).to include('merged')
end
end
end end
end
context 'when an MR to be closed was empty already' do context 'when an MR to be closed was empty already' do
let!(:empty_fork_merge_request) do let!(:empty_fork_merge_request) do
create(:merge_request, create(:merge_request,
source_project: @fork_project, source_project: @fork_project,
source_branch: 'master', source_branch: 'master',
target_branch: 'master', target_branch: 'master',
target_project: @project) target_project: @project)
end
before do
# This spec already has a fake push, so pretend that we were targeting
# feature all along.
empty_fork_merge_request.update_columns(target_branch: 'feature')
service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature')
reload_mrs
empty_fork_merge_request.reload
end
it 'only updates the non-empty MRs' do
expect(@merge_request).to be_merged
expect(@fork_merge_request).to be_merged
expect(empty_fork_merge_request).to be_open
expect(empty_fork_merge_request.merge_request_diff.state).to eq('empty')
expect(empty_fork_merge_request.notes).to be_empty
if state_tracking_enabled
expect(@merge_request.resource_state_events.last.state).to eq('merged')
expect(@fork_merge_request.resource_state_events.last.state).to eq('merged')
else
expect(@merge_request.notes.last.note).to include('merged')
expect(@fork_merge_request.notes.last.note).to include('merged')
end
end
end end
end
context "manual merge of source branch #{state_tracking_enabled ? 'enabled' : 'disabled'}", :sidekiq_might_not_need_inline do
before do before do
# This spec already has a fake push, so pretend that we were targeting stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
# feature all along.
empty_fork_merge_request.update_columns(target_branch: 'feature')
service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') # Merge master -> feature branch
@project.repository.merge(@user, @merge_request.diff_head_sha, @merge_request, 'Test message')
commit = @project.repository.commit('feature')
service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature')
reload_mrs reload_mrs
empty_fork_merge_request.reload
end end
it 'only updates the non-empty MRs' do it 'updates the merge state' do
expect(@merge_request).to be_merged if state_tracking_enabled
expect(@merge_request.notes.last.note).to include('merged') expect(@merge_request.resource_state_events.last.state).to eq('merged')
expect(@fork_merge_request.resource_state_events.last.state).to eq('merged')
else
expect(@merge_request.notes.last.note).to include('merged')
expect(@fork_merge_request.notes.last.note).to include('merged')
end
expect(@merge_request).to be_merged
expect(@merge_request.diffs.size).to be > 0
expect(@fork_merge_request).to be_merged expect(@fork_merge_request).to be_merged
expect(@fork_merge_request.notes.last.note).to include('merged') expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done
expect(empty_fork_merge_request).to be_open
expect(empty_fork_merge_request.merge_request_diff.state).to eq('empty')
expect(empty_fork_merge_request.notes).to be_empty
end end
end end
end end
context 'manual merge of source branch', :sidekiq_might_not_need_inline do
before do
# Merge master -> feature branch
@project.repository.merge(@user, @merge_request.diff_head_sha, @merge_request, 'Test message')
commit = @project.repository.commit('feature')
service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature')
reload_mrs
end
it 'updates the merge state' do
expect(@merge_request.notes.last.note).to include('merged')
expect(@merge_request).to be_merged
expect(@merge_request.diffs.size).to be > 0
expect(@fork_merge_request).to be_merged
expect(@fork_merge_request.notes.last.note).to include('merged')
expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done
end
end
context 'push to fork repo source branch', :sidekiq_might_not_need_inline do context 'push to fork repo source branch', :sidekiq_might_not_need_inline do
let(:refresh_service) { service.new(@fork_project, @user) } let(:refresh_service) { service.new(@fork_project, @user) }
...@@ -583,20 +608,29 @@ describe MergeRequests::RefreshService do ...@@ -583,20 +608,29 @@ describe MergeRequests::RefreshService do
end end
end end
context 'push to origin repo target branch after fork project was removed' do [true, false].each do |state_tracking_enabled|
before do context "push to origin repo target branch after fork project was removed #{state_tracking_enabled ? 'enabled' : 'disabled'}" do
@fork_project.destroy before do
service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
reload_mrs
end
it 'updates the merge request state' do @fork_project.destroy
expect(@merge_request.notes.last.note).to include('merged') service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature')
expect(@merge_request).to be_merged reload_mrs
expect(@fork_merge_request).to be_open end
expect(@fork_merge_request.notes).to be_empty
expect(@build_failed_todo).to be_done it 'updates the merge request state' do
expect(@fork_build_failed_todo).to be_done if state_tracking_enabled
expect(@merge_request.resource_state_events.last.state).to eq('merged')
else
expect(@merge_request.notes.last.note).to include('merged')
end
expect(@merge_request).to be_merged
expect(@fork_merge_request).to be_open
expect(@fork_merge_request.notes).to be_empty
expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done
end
end end
end end
......
...@@ -20,8 +20,11 @@ describe MergeRequests::ReopenService do ...@@ -20,8 +20,11 @@ describe MergeRequests::ReopenService do
context 'valid params' do context 'valid params' do
let(:service) { described_class.new(project, user, {}) } let(:service) { described_class.new(project, user, {}) }
let(:state_tracking) { true }
before do before do
stub_feature_flags(track_resource_state_change_events: state_tracking)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
perform_enqueued_jobs do perform_enqueued_jobs do
...@@ -43,9 +46,22 @@ describe MergeRequests::ReopenService do ...@@ -43,9 +46,22 @@ describe MergeRequests::ReopenService do
expect(email.subject).to include(merge_request.title) expect(email.subject).to include(merge_request.title)
end end
it 'creates system note about merge_request reopen' do context 'note creation' do
note = merge_request.notes.last context 'when state event tracking is disabled' do
expect(note.note).to include 'reopened' let(:state_tracking) { false }
it 'creates system note about merge_request reopen' do
note = merge_request.notes.last
expect(note.note).to include 'reopened'
end
end
context 'when state event tracking is enabled' do
it 'creates resource state event about merge_request reopen' do
event = merge_request.resource_state_events.last
expect(event.state).to eq('reopened')
end
end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe ResourceEvents::ChangeStateService do
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, source_project: project) }
describe '#execute' do
context 'when resource is an issue' do
%w[opened reopened closed locked].each do |state|
it "creates the expected event if issue has #{state} state" do
described_class.new(user: user, resource: issue).execute(state)
event = issue.resource_state_events.last
expect(event.issue).to eq(issue)
expect(event.merge_request).to be_nil
expect(event.state).to eq(state)
end
end
end
context 'when resource is a merge request' do
%w[opened reopened closed locked merged].each do |state|
it "creates the expected event if merge request has #{state} state" do
described_class.new(user: user, resource: merge_request).execute(state)
event = merge_request.resource_state_events.last
expect(event.issue).to be_nil
expect(event.merge_request).to eq(merge_request)
expect(event.state).to eq(state)
end
end
end
end
end
...@@ -157,7 +157,18 @@ describe ::SystemNotes::IssuablesService do ...@@ -157,7 +157,18 @@ describe ::SystemNotes::IssuablesService do
describe '#change_status' do describe '#change_status' do
subject { service.change_status(status, source) } subject { service.change_status(status, source) }
context 'when resource state event tracking is enabled' do
let(:status) { 'reopened' }
let(:source) { nil }
it { is_expected.to be_nil }
end
context 'with status reopened' do context 'with status reopened' do
before do
stub_feature_flags(track_resource_state_change_events: false)
end
let(:status) { 'reopened' } let(:status) { 'reopened' }
let(:source) { nil } let(:source) { nil }
...@@ -169,6 +180,10 @@ describe ::SystemNotes::IssuablesService do ...@@ -169,6 +180,10 @@ describe ::SystemNotes::IssuablesService do
end end
context 'with a source' do context 'with a source' do
before do
stub_feature_flags(track_resource_state_change_events: false)
end
let(:status) { 'opened' } let(:status) { 'opened' }
let(:source) { double('commit', gfm_reference: 'commit 123456') } let(:source) { double('commit', gfm_reference: 'commit 123456') }
......
...@@ -30,6 +30,8 @@ RSpec.shared_examples 'thread comments' do |resource_name| ...@@ -30,6 +30,8 @@ RSpec.shared_examples 'thread comments' do |resource_name|
click_button 'Comment & close issue' click_button 'Comment & close issue'
wait_for_all_requests
expect(page).to have_content(comment) expect(page).to have_content(comment)
expect(page).to have_content "@#{user.username} closed" expect(page).to have_content "@#{user.username} closed"
......
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