Commit f0577d83 authored by Mathias Vestergaard's avatar Mathias Vestergaard Committed by Timothy Andrew

Added "developers can merge" setting to protected branches

- Cherry-picked from `mvestergaard:branch-protection-dev-merge`
- https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4220
parent ecd301dd
...@@ -497,6 +497,7 @@ v 8.7.7 ...@@ -497,6 +497,7 @@ v 8.7.7
- Prevent unauthorized access to other projects build traces - Prevent unauthorized access to other projects build traces
- Forbid scripting for wiki files - Forbid scripting for wiki files
- Only show notes through JSON on confidential issues that the user has access to - Only show notes through JSON on confidential issues that the user has access to
- Added protected branch setting that allows Developers to accept merge requests while push is still disallowed. !4220 (Mathias Vestergaard)
v 8.7.6 v 8.7.6
- Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko) - Fix links on wiki pages for relative url setups. !4131 (Artem Sidorenko)
......
$ -> $ ->
$(".protected-branches-list :checkbox").change (e) -> $(".protected-branches-list :checkbox").change (e) ->
name = $(this).attr("name") name = $(this).attr("name")
if name == "developers_can_push" row = $(this).parents("tr")
if name == "developers_can_push" || name == "developers_can_merge"
id = $(this).val() id = $(this).val()
checked = $(this).is(":checked") can_push = row.find("input[name=developers_can_push]").is(":checked")
can_merge = row.find("input[name=developers_can_merge]").is(":checked")
url = $(this).data("url") url = $(this).data("url")
$.ajax $.ajax
type: "PUT" type: "PUT"
...@@ -12,7 +14,8 @@ $ -> ...@@ -12,7 +14,8 @@ $ ->
data: data:
id: id id: id
protected_branch: protected_branch:
developers_can_push: checked developers_can_push: can_push
developers_can_merge: can_merge
success: -> success: ->
row = $(e.target) row = $(e.target)
......
...@@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController ...@@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
end end
def protected_branch_params def protected_branch_params
params.require(:protected_branch).permit(:name, :developers_can_push) params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge)
end end
end end
...@@ -552,7 +552,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -552,7 +552,8 @@ class MergeRequest < ActiveRecord::Base
end end
def can_be_merged_by?(user) def can_be_merged_by?(user)
::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch) access = ::Gitlab::GitAccess.new(user, project, 'web')
access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
end end
def mergeable_ci_state? def mergeable_ci_state?
......
...@@ -832,6 +832,10 @@ class Project < ActiveRecord::Base ...@@ -832,6 +832,10 @@ class Project < ActiveRecord::Base
protected_branches.matching(branch_name).any?(&:developers_can_push) protected_branches.matching(branch_name).any?(&:developers_can_push)
end end
def developers_can_merge_to_protected_branch?(branch_name)
protected_branches.matching(branch_name).any?(&:developers_can_merge)
end
def forked? def forked?
!(forked_project_link.nil? || forked_project_link.forked_from_project.nil?) !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
end end
......
...@@ -89,7 +89,8 @@ class GitPushService < BaseService ...@@ -89,7 +89,8 @@ class GitPushService < BaseService
# Set protection on the default branch if configured # Set protection on the default branch if configured
if current_application_settings.default_branch_protection != PROTECTION_NONE if current_application_settings.default_branch_protection != PROTECTION_NONE
developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
@project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push }) developers_can_merge = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? true : false
@project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge })
end end
end end
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
.table-responsive .table-responsive
%table.table.protected-branches-list %table.table.protected-branches-list
%colgroup %colgroup
%col{ width: "27%" }
%col{ width: "30%" } %col{ width: "30%" }
%col{ width: "25%" } %col{ width: "25%" }
%col{ width: "25%" } %col{ width: "25%" }
...@@ -18,6 +19,7 @@ ...@@ -18,6 +19,7 @@
%th Protected Branch %th Protected Branch
%th Commit %th Commit
%th Developers Can Push %th Developers Can Push
%th Developers can Merge
- if can_admin_project - if can_admin_project
%th %th
%tbody %tbody
......
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
(branch was removed from repository) (branch was removed from repository)
%td %td
= check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url }) = check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url })
%td
= check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url })
- if can_admin_project - if can_admin_project
%td %td
= link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right" = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right"
...@@ -36,6 +36,14 @@ ...@@ -36,6 +36,14 @@
= f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0"
%p.light.append-bottom-0 %p.light.append-bottom-0
Allow developers to push to this branch Allow developers to push to this branch
.form-group
= f.check_box :developers_can_merge, class: "pull-left"
.prepend-left-20
= f.label :developers_can_merge, "Developers can merge", class: "label-light append-bottom-0"
%p.light.append-bottom-0
Allow developers to accept merge requests to this branch
= f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true
%hr %hr
= render "branches_list" = render "branches_list"
class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
def change
add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false
end
end
...@@ -860,11 +860,12 @@ ActiveRecord::Schema.define(version: 20160712171823) do ...@@ -860,11 +860,12 @@ ActiveRecord::Schema.define(version: 20160712171823) do
add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree
create_table "protected_branches", force: :cascade do |t| create_table "protected_branches", force: :cascade do |t|
t.integer "project_id", null: false t.integer "project_id", null: false
t.string "name", null: false t.string "name", null: false
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.boolean "developers_can_push", default: false, null: false t.boolean "developers_can_push", default: false, null: false
t.boolean "developers_can_merge", default: false, null: false
end end
add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree
......
...@@ -14,9 +14,10 @@ module Gitlab ...@@ -14,9 +14,10 @@ module Gitlab
OWNER = 50 OWNER = 50
# Branch protection settings # Branch protection settings
PROTECTION_NONE = 0 PROTECTION_NONE = 0
PROTECTION_DEV_CAN_PUSH = 1 PROTECTION_DEV_CAN_PUSH = 1
PROTECTION_FULL = 2 PROTECTION_FULL = 2
PROTECTION_DEV_CAN_MERGE = 3
class << self class << self
def values def values
...@@ -54,6 +55,7 @@ module Gitlab ...@@ -54,6 +55,7 @@ module Gitlab
def protection_options def protection_options
{ {
"Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE, "Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE,
"Protected against pushes: Developers cannot push new commits, but are allowed to accept merge requests to the branch." => PROTECTION_DEV_CAN_MERGE,
"Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH, "Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH,
"Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL, "Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL,
} }
......
...@@ -39,6 +39,16 @@ module Gitlab ...@@ -39,6 +39,16 @@ module Gitlab
end end
end end
def can_merge_to_branch?(ref)
return false unless user
if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref)
user.can?(:push_code_to_protected_branches, project)
else
user.can?(:push_code, project)
end
end
def can_read_project? def can_read_project?
if user if user
user.can?(:read_project, project) user.can?(:read_project, project)
......
...@@ -65,6 +65,27 @@ describe Gitlab::GitAccess, lib: true do ...@@ -65,6 +65,27 @@ describe Gitlab::GitAccess, lib: true do
expect(access.can_push_to_branch?(@branch.name)).to be_falsey expect(access.can_push_to_branch?(@branch.name)).to be_falsey
end end
end end
describe 'merge to protected branch if allowed for developers' do
before do
@branch = create :protected_branch, project: project, developers_can_merge: true
end
it "returns true if user is a master" do
project.team << [user, :master]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end
it "returns true if user is a developer" do
project.team << [user, :developer]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
end
end
end end
describe '#check with single protocols allowed' do describe '#check with single protocols allowed' do
......
...@@ -224,7 +224,7 @@ describe GitPushService, services: true do ...@@ -224,7 +224,7 @@ describe GitPushService, services: true do
it "when pushing a branch for the first time" do it "when pushing a branch for the first time" do
expect(project).to receive(:execute_hooks) expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master") expect(project.default_branch).to eq("master")
expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false }) expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: false })
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
end end
...@@ -242,7 +242,16 @@ describe GitPushService, services: true do ...@@ -242,7 +242,16 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks) expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master") expect(project.default_branch).to eq("master")
expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true }) expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true, developers_can_merge: false })
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
end
it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master")
expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: true })
execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' )
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