c946ee1282
Signed-off-by: Rémy Coutable <remy@rymai.me>
523 lines
16 KiB
Ruby
523 lines
16 KiB
Ruby
require 'spec_helper'
|
|
|
|
describe MergeRequests::UpdateService, :mailer do
|
|
let(:project) { create(:project, :repository) }
|
|
let(:user) { create(:user) }
|
|
let(:user2) { create(:user) }
|
|
let(:user3) { create(:user) }
|
|
let(:label) { create(:label, project: project) }
|
|
let(:label2) { create(:label) }
|
|
|
|
let(:merge_request) do
|
|
create(:merge_request, :simple, title: 'Old title',
|
|
description: "FYI #{user2.to_reference}",
|
|
assignee_id: user3.id,
|
|
source_project: project)
|
|
end
|
|
|
|
before do
|
|
project.team << [user, :master]
|
|
project.team << [user2, :developer]
|
|
project.team << [user3, :developer]
|
|
end
|
|
|
|
describe 'execute' do
|
|
def find_note(starting_with)
|
|
@merge_request.notes.find do |note|
|
|
note && note.note.start_with?(starting_with)
|
|
end
|
|
end
|
|
|
|
def find_notes(action)
|
|
@merge_request
|
|
.notes
|
|
.joins(:system_note_metadata)
|
|
.where(system_note_metadata: { action: action })
|
|
end
|
|
|
|
def update_merge_request(opts)
|
|
@merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
|
|
@merge_request.reload
|
|
end
|
|
|
|
context 'valid params' do
|
|
let(:opts) do
|
|
{
|
|
title: 'New title',
|
|
description: 'Also please fix',
|
|
assignee_id: user2.id,
|
|
state_event: 'close',
|
|
label_ids: [label.id],
|
|
target_branch: 'target',
|
|
force_remove_source_branch: '1'
|
|
}
|
|
end
|
|
|
|
let(:service) { described_class.new(project, user, opts) }
|
|
|
|
before do
|
|
allow(service).to receive(:execute_hooks)
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = service.execute(merge_request)
|
|
@merge_request.reload
|
|
end
|
|
end
|
|
|
|
it 'mathces base expectations' do
|
|
expect(@merge_request).to be_valid
|
|
expect(@merge_request.title).to eq('New title')
|
|
expect(@merge_request.assignee).to eq(user2)
|
|
expect(@merge_request).to be_closed
|
|
expect(@merge_request.labels.count).to eq(1)
|
|
expect(@merge_request.labels.first.title).to eq(label.name)
|
|
expect(@merge_request.target_branch).to eq('target')
|
|
expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1')
|
|
end
|
|
|
|
it 'executes hooks with update action' do
|
|
expect(service).to have_received(:execute_hooks)
|
|
.with(@merge_request, 'update')
|
|
end
|
|
|
|
it 'sends email to user2 about assign of new merge request and email to user3 about merge request unassignment' do
|
|
deliveries = ActionMailer::Base.deliveries
|
|
email = deliveries.last
|
|
recipients = deliveries.last(2).map(&:to).flatten
|
|
expect(recipients).to include(user2.email, user3.email)
|
|
expect(email.subject).to include(merge_request.title)
|
|
end
|
|
|
|
it 'creates system note about merge_request reassign' do
|
|
note = find_note('assigned to')
|
|
|
|
expect(note).not_to be_nil
|
|
expect(note.note).to include "assigned to #{user2.to_reference}"
|
|
end
|
|
|
|
it 'creates system note about merge_request label edit' do
|
|
note = find_note('added ~')
|
|
|
|
expect(note).not_to be_nil
|
|
expect(note.note).to include "added #{label.to_reference} label"
|
|
end
|
|
|
|
it 'creates system note about title change' do
|
|
note = find_note('changed title')
|
|
|
|
expect(note).not_to be_nil
|
|
expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
|
|
end
|
|
|
|
it 'creates system note about description change' do
|
|
note = find_note('changed the description')
|
|
|
|
expect(note).not_to be_nil
|
|
expect(note.note).to eq('changed the description')
|
|
end
|
|
|
|
it 'creates system note about branch change' do
|
|
note = find_note('changed target')
|
|
|
|
expect(note).not_to be_nil
|
|
expect(note.note).to eq 'changed target branch from `master` to `target`'
|
|
end
|
|
|
|
context 'when not including source branch removal options' do
|
|
before do
|
|
opts.delete(:force_remove_source_branch)
|
|
end
|
|
|
|
it 'maintains the original options' do
|
|
update_merge_request(opts)
|
|
|
|
expect(@merge_request.merge_params["force_remove_source_branch"]).to eq("1")
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'merge' do
|
|
let(:opts) do
|
|
{
|
|
merge: merge_request.diff_head_sha
|
|
}
|
|
end
|
|
|
|
let(:service) { described_class.new(project, user, opts) }
|
|
|
|
context 'without pipeline' do
|
|
before do
|
|
merge_request.merge_error = 'Error'
|
|
|
|
perform_enqueued_jobs do
|
|
service.execute(merge_request)
|
|
@merge_request = MergeRequest.find(merge_request.id)
|
|
end
|
|
end
|
|
|
|
it 'merges the MR' do
|
|
expect(@merge_request).to be_valid
|
|
expect(@merge_request.state).to eq('merged')
|
|
expect(@merge_request.merge_error).to be_nil
|
|
end
|
|
end
|
|
|
|
context 'with finished pipeline' do
|
|
before do
|
|
create(:ci_pipeline_with_one_job,
|
|
project: project,
|
|
ref: merge_request.source_branch,
|
|
sha: merge_request.diff_head_sha,
|
|
status: :success)
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = service.execute(merge_request)
|
|
@merge_request = MergeRequest.find(merge_request.id)
|
|
end
|
|
end
|
|
|
|
it 'merges the MR' do
|
|
expect(@merge_request).to be_valid
|
|
expect(@merge_request.state).to eq('merged')
|
|
end
|
|
end
|
|
|
|
context 'with active pipeline' do
|
|
before do
|
|
service_mock = double
|
|
create(
|
|
:ci_pipeline_with_one_job,
|
|
project: project,
|
|
ref: merge_request.source_branch,
|
|
sha: merge_request.diff_head_sha,
|
|
head_pipeline_of: merge_request
|
|
)
|
|
|
|
expect(MergeRequests::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user)
|
|
.and_return(service_mock)
|
|
expect(service_mock).to receive(:execute).with(merge_request)
|
|
end
|
|
|
|
it { service.execute(merge_request) }
|
|
end
|
|
|
|
context 'with a non-authorised user' do
|
|
let(:visitor) { create(:user) }
|
|
let(:service) { described_class.new(project, visitor, opts) }
|
|
|
|
before do
|
|
merge_request.update_attribute(:merge_error, 'Error')
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = service.execute(merge_request)
|
|
@merge_request = MergeRequest.find(merge_request.id)
|
|
end
|
|
end
|
|
|
|
it 'does not merge the MR' do
|
|
expect(@merge_request.state).to eq('opened')
|
|
expect(@merge_request.merge_error).not_to be_nil
|
|
end
|
|
end
|
|
|
|
context 'MR can not be merged when note sha != MR sha' do
|
|
let(:opts) do
|
|
{
|
|
merge: 'other_commit'
|
|
}
|
|
end
|
|
|
|
before do
|
|
perform_enqueued_jobs do
|
|
@merge_request = service.execute(merge_request)
|
|
@merge_request = MergeRequest.find(merge_request.id)
|
|
end
|
|
end
|
|
|
|
it { expect(@merge_request.state).to eq('opened') }
|
|
end
|
|
end
|
|
|
|
context 'todos' do
|
|
let!(:pending_todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) }
|
|
|
|
context 'when the title change' do
|
|
before do
|
|
update_merge_request({ title: 'New title' })
|
|
end
|
|
|
|
it 'marks pending todos as done' do
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
|
|
it 'does not create any new todos' do
|
|
expect(Todo.count).to eq(1)
|
|
end
|
|
end
|
|
|
|
context 'when the description change' do
|
|
before do
|
|
update_merge_request({ description: "Also please fix #{user2.to_reference} #{user3.to_reference}" })
|
|
end
|
|
|
|
it 'marks pending todos as done' do
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
|
|
it 'creates only 1 new todo' do
|
|
expect(Todo.count).to eq(2)
|
|
end
|
|
end
|
|
|
|
context 'when is reassigned' do
|
|
before do
|
|
update_merge_request({ assignee: user2 })
|
|
end
|
|
|
|
it 'marks previous assignee pending todos as done' do
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
|
|
it 'creates a pending todo for new assignee' do
|
|
attributes = {
|
|
project: project,
|
|
author: user,
|
|
user: user2,
|
|
target_id: merge_request.id,
|
|
target_type: merge_request.class.name,
|
|
action: Todo::ASSIGNED,
|
|
state: :pending
|
|
}
|
|
|
|
expect(Todo.where(attributes).count).to eq 1
|
|
end
|
|
end
|
|
|
|
context 'when the milestone change' do
|
|
it 'marks pending todos as done' do
|
|
update_merge_request({ milestone: create(:milestone) })
|
|
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
|
|
it_behaves_like 'system notes for milestones'
|
|
end
|
|
|
|
context 'when the labels change' do
|
|
before do
|
|
update_merge_request({ label_ids: [label.id] })
|
|
end
|
|
|
|
it 'marks pending todos as done' do
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
end
|
|
|
|
context 'when the assignee changes' do
|
|
it 'updates open merge request counter for assignees when merge request is reassigned' do
|
|
update_merge_request(assignee_id: user2.id)
|
|
|
|
expect(user3.assigned_open_merge_requests_count).to eq 0
|
|
expect(user2.assigned_open_merge_requests_count).to eq 1
|
|
end
|
|
end
|
|
|
|
context 'when the target branch change' do
|
|
before do
|
|
update_merge_request({ target_branch: 'target' })
|
|
end
|
|
|
|
it 'marks pending todos as done' do
|
|
expect(pending_todo.reload).to be_done
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'when the issue is relabeled' do
|
|
let!(:non_subscriber) { create(:user) }
|
|
let!(:subscriber) { create(:user) { |u| label.toggle_subscription(u, project) } }
|
|
|
|
before do
|
|
project.team << [non_subscriber, :developer]
|
|
project.team << [subscriber, :developer]
|
|
end
|
|
|
|
it 'sends notifications for subscribers of newly added labels' do
|
|
opts = { label_ids: [label.id] }
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = described_class.new(project, user, opts).execute(merge_request)
|
|
end
|
|
|
|
should_email(subscriber)
|
|
should_not_email(non_subscriber)
|
|
end
|
|
|
|
context 'when issue has the `label` label' do
|
|
before do
|
|
merge_request.labels << label
|
|
end
|
|
|
|
it 'does not send notifications for existing labels' do
|
|
opts = { label_ids: [label.id, label2.id] }
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = described_class.new(project, user, opts).execute(merge_request)
|
|
end
|
|
|
|
should_not_email(subscriber)
|
|
should_not_email(non_subscriber)
|
|
end
|
|
|
|
it 'does not send notifications for removed labels' do
|
|
opts = { label_ids: [label2.id] }
|
|
|
|
perform_enqueued_jobs do
|
|
@merge_request = described_class.new(project, user, opts).execute(merge_request)
|
|
end
|
|
|
|
should_not_email(subscriber)
|
|
should_not_email(non_subscriber)
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'updating mentions' do
|
|
let(:mentionable) { merge_request }
|
|
include_examples 'updating mentions', described_class
|
|
end
|
|
|
|
context 'when MergeRequest has tasks' do
|
|
before do
|
|
update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" })
|
|
end
|
|
|
|
it { expect(@merge_request.tasks?).to eq(true) }
|
|
|
|
context 'when tasks are marked as completed' do
|
|
before do
|
|
update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" })
|
|
end
|
|
|
|
it 'creates system note about task status change' do
|
|
note1 = find_note('marked the task **Task 1** as completed')
|
|
note2 = find_note('marked the task **Task 2** as completed')
|
|
|
|
expect(note1).not_to be_nil
|
|
expect(note2).not_to be_nil
|
|
|
|
description_notes = find_notes('description')
|
|
expect(description_notes.length).to eq(1)
|
|
end
|
|
end
|
|
|
|
context 'when tasks are marked as incomplete' do
|
|
before do
|
|
update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" })
|
|
update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" })
|
|
end
|
|
|
|
it 'creates system note about task status change' do
|
|
note1 = find_note('marked the task **Task 1** as incomplete')
|
|
note2 = find_note('marked the task **Task 2** as incomplete')
|
|
|
|
expect(note1).not_to be_nil
|
|
expect(note2).not_to be_nil
|
|
|
|
description_notes = find_notes('description')
|
|
expect(description_notes.length).to eq(1)
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'while saving references to issues that the updated merge request closes' do
|
|
let(:first_issue) { create(:issue, project: project) }
|
|
let(:second_issue) { create(:issue, project: project) }
|
|
|
|
it 'creates a `MergeRequestsClosingIssues` record for each issue' do
|
|
issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" }
|
|
service = described_class.new(project, user, issue_closing_opts)
|
|
allow(service).to receive(:execute_hooks)
|
|
service.execute(merge_request)
|
|
|
|
issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
|
|
expect(issue_ids).to match_array([first_issue.id, second_issue.id])
|
|
end
|
|
|
|
it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do
|
|
opts = {
|
|
title: 'Awesome merge_request',
|
|
description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}",
|
|
source_branch: 'feature',
|
|
target_branch: 'master',
|
|
force_remove_source_branch: '1'
|
|
}
|
|
|
|
merge_request = MergeRequests::CreateService.new(project, user, opts).execute
|
|
|
|
issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
|
|
expect(issue_ids).to match_array([first_issue.id, second_issue.id])
|
|
|
|
service = described_class.new(project, user, description: "not closing any issues")
|
|
allow(service).to receive(:execute_hooks)
|
|
service.execute(merge_request.reload)
|
|
|
|
issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
|
|
expect(issue_ids).to be_empty
|
|
end
|
|
end
|
|
|
|
context 'updating asssignee_id' do
|
|
it 'does not update assignee when assignee_id is invalid' do
|
|
merge_request.update(assignee_id: user.id)
|
|
|
|
update_merge_request(assignee_id: -1)
|
|
|
|
expect(merge_request.reload.assignee).to eq(user)
|
|
end
|
|
|
|
it 'unassigns assignee when user id is 0' do
|
|
merge_request.update(assignee_id: user.id)
|
|
|
|
update_merge_request(assignee_id: 0)
|
|
|
|
expect(merge_request.assignee_id).to be_nil
|
|
end
|
|
|
|
it 'saves assignee when user id is valid' do
|
|
update_merge_request(assignee_id: user.id)
|
|
|
|
expect(merge_request.assignee_id).to eq(user.id)
|
|
end
|
|
|
|
it 'does not update assignee_id when user cannot read issue' do
|
|
non_member = create(:user)
|
|
original_assignee = merge_request.assignee
|
|
|
|
update_merge_request(assignee_id: non_member.id)
|
|
|
|
expect(merge_request.assignee_id).to eq(original_assignee.id)
|
|
end
|
|
|
|
context "when issuable feature is private" do
|
|
levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]
|
|
|
|
levels.each do |level|
|
|
it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
|
|
assignee = create(:user)
|
|
project.update(visibility_level: level)
|
|
feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level"
|
|
project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE)
|
|
|
|
expect { update_merge_request(assignee_id: assignee) }.not_to change { merge_request.assignee }
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
include_examples 'issuable update service' do
|
|
let(:open_issuable) { merge_request }
|
|
let(:closed_issuable) { create(:closed_merge_request, source_project: project) }
|
|
end
|
|
end
|
|
end
|