Commit 9faa00a7 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'wip-means-no-mr' into 'master'

Don't allow a merge request to be merged when its title starts with "WIP".

Addresses private issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2078

![Screen_Shot_2015-04-30_at_15.47.03](https://gitlab.com/gitlab-org/gitlab-ce/uploads/9035743e6f482a9c31cf86d84388074c/Screen_Shot_2015-04-30_at_15.47.03.png)

See merge request !590
parents 39a55bdf 7e0eb486
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 7.11.0 (unreleased) v 7.11.0 (unreleased)
- Don't allow a merge request to be merged when its title starts with "WIP".
- Get Gitorious importer to work again. - Get Gitorious importer to work again.
- Fix clone URL field and X11 Primary selection (Dmitry Medvinsky) - Fix clone URL field and X11 Primary selection (Dmitry Medvinsky)
- Ignore invalid lines in .gitmodules - Ignore invalid lines in .gitmodules
......
...@@ -124,13 +124,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -124,13 +124,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@merge_request.check_if_can_be_merged @merge_request.check_if_can_be_merged
end end
render json: { merge_status: @merge_request.merge_status_name } render json: { merge_status: @merge_request.automerge_status }
end end
def automerge def automerge
return access_denied! unless allowed_to_merge? return access_denied! unless allowed_to_merge?
if @merge_request.open? && @merge_request.can_be_merged? if @merge_request.automergeable?
AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params) AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params)
@status = true @status = true
else else
......
...@@ -199,6 +199,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -199,6 +199,8 @@ class MergeRequest < ActiveRecord::Base
end end
def automerge!(current_user, commit_message = nil) def automerge!(current_user, commit_message = nil)
return unless automergeable?
MergeRequests::AutoMergeService. MergeRequests::AutoMergeService.
new(target_project, current_user). new(target_project, current_user).
execute(self, commit_message) execute(self, commit_message)
...@@ -208,6 +210,22 @@ class MergeRequest < ActiveRecord::Base ...@@ -208,6 +210,22 @@ class MergeRequest < ActiveRecord::Base
opened? || reopened? opened? || reopened?
end end
def work_in_progress?
title =~ /\A\[?WIP\]?:? /i
end
def automergeable?
open? && !work_in_progress? && can_be_merged?
end
def automerge_status
if work_in_progress?
"work_in_progress"
else
merge_status_name
end
end
def mr_and_commit_notes def mr_and_commit_notes
# Fetch comments only from last 100 commits # Fetch comments only from last 100 commits
commits_for_notes_limit = 100 commits_for_notes_limit = 100
......
...@@ -72,6 +72,6 @@ ...@@ -72,6 +72,6 @@
check_enable: #{@merge_request.unchecked? ? "true" : "false"}, check_enable: #{@merge_request.unchecked? ? "true" : "false"},
url_to_ci_check: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", url_to_ci_check: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}",
ci_enable: #{@project.ci_service ? "true" : "false"}, ci_enable: #{@project.ci_service ? "true" : "false"},
current_status: "#{@merge_request.merge_status_name}", current_status: "#{@merge_request.automerge_status}",
action: "#{controller.action_name}" action: "#{controller.action_name}"
}); });
...@@ -4,9 +4,11 @@ ...@@ -4,9 +4,11 @@
%strong Archived projects cannot be committed to! %strong Archived projects cannot be committed to!
- else - else
.automerge_widget.cannot_be_merged.hide .automerge_widget.cannot_be_merged.hide
%strong This can't be merged automatically, even if it could be merged you don't have the permission to do so. %strong This request can't be merged automatically. Even if it could be merged, you don't have permission to do so.
.automerge_widget.work_in_progress.hide
%strong This request can't be merged automatically because it is marked a Work In Progress. Even if it could be merged, you don't have permission to do so.
.automerge_widget.can_be_merged.hide .automerge_widget.can_be_merged.hide
%strong This can be merged automatically but you don't have the permission to do so. %strong This request can be merged automatically, but you don't have permission to do so.
- if @show_merge_controls - if @show_merge_controls
...@@ -57,6 +59,18 @@ ...@@ -57,6 +59,18 @@
&nbsp; &nbsp;
This usually happens when git can not resolve conflicts between branches automatically. This usually happens when git can not resolve conflicts between branches automatically.
.automerge_widget.work_in_progress.hide
%h4
This request can't be merged because it is marked a <strong>Work In Progress</strong>.
%p
%button.btn.disabled
%i.fa.fa-warning
Accept Merge Request
&nbsp;
When the merge request is ready, remove the "WIP" prefix from the title to allow it to be merged.
.automerge_widget.unchecked .automerge_widget.unchecked
%p %p
%strong %strong
......
...@@ -186,7 +186,7 @@ module API ...@@ -186,7 +186,7 @@ module API
merge_request.check_if_can_be_merged merge_request.check_if_can_be_merged
end end
if merge_request.open? if merge_request.open? && !merge_request.work_in_progress?
if merge_request.can_be_merged? if merge_request.can_be_merged?
merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message)
present merge_request, with: Entities::MergeRequest present merge_request, with: Entities::MergeRequest
...@@ -195,7 +195,7 @@ module API ...@@ -195,7 +195,7 @@ module API
end end
else else
# Merge request can not be merged # Merge request can not be merged
# because it is already closed/merged # because it is already closed/merged or marked as WIP
not_allowed! not_allowed!
end end
else else
......
...@@ -115,6 +115,32 @@ describe MergeRequest do ...@@ -115,6 +115,32 @@ describe MergeRequest do
end end
end end
describe "#work_in_progress?" do
it "detects the 'WIP ' prefix" do
subject.title = "WIP #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the 'WIP: ' prefix" do
subject.title = "WIP: #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the '[WIP] ' prefix" do
subject.title = "[WIP] #{subject.title}"
expect(subject).to be_work_in_progress
end
it "doesn't detect WIP for words starting with WIP" do
subject.title = "Wipwap #{subject.title}"
expect(subject).not_to be_work_in_progress
end
it "doesn't detect WIP by default" do
expect(subject).not_to be_work_in_progress
end
end
it_behaves_like 'an editable mentionable' do it_behaves_like 'an editable mentionable' do
subject { create(:merge_request, source_project: project, target_project: project) } subject { create(:merge_request, source_project: project, target_project: project) }
......
...@@ -312,6 +312,13 @@ describe API::API, api: true do ...@@ -312,6 +312,13 @@ describe API::API, api: true do
expect(json_response['message']).to eq('405 Method Not Allowed') expect(json_response['message']).to eq('405 Method Not Allowed')
end end
it "should return 405 if merge_request is a work in progress" do
merge_request.update_attribute(:title, "WIP: #{merge_request.title}")
put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user)
expect(response.status).to eq(405)
expect(json_response['message']).to eq('405 Method Not Allowed')
end
it "should return 401 if user has no permissions to merge" do it "should return 401 if user has no permissions to merge" do
user2 = create(:user) user2 = create(:user)
project.team << [user2, :reporter] project.team << [user2, :reporter]
......
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