diff --git a/CHANGELOG b/CHANGELOG index d555d23860ddcb0b9983d04f38b563c1d3aac0e6..2b04c15b82743b5337f42241b77ec7a933847da2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,7 @@ v 8.11.0 (unreleased) - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Optimize maximum user access level lookup in loading of notes + - Add "No one can push" as an option for protected branches. !5081 - Limit git rev-list output count to one in forced push check - Clean up unused routes (Josef Strzibny) - Add green outline to New Branch button. !5447 (winniehell) diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index d212d66da1b595edc678e8056e1ca5481957f01c..9e6901962c6b3354581bd38491048c64bc70331a 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -171,6 +171,11 @@ break; case 'search:show': new Search(); + break; + case 'projects:protected_branches:index': + new ProtectedBranchesAccessSelect($(".new_protected_branch"), false, true); + new ProtectedBranchesAccessSelect($(".protected-branches-list"), true, false); + break; } switch (path.first()) { case 'admin': diff --git a/app/assets/javascripts/protected_branches.js b/app/assets/javascripts/protected_branches.js deleted file mode 100644 index db21a19964dc69bcdbefd534c43234ef8610ad5f..0000000000000000000000000000000000000000 --- a/app/assets/javascripts/protected_branches.js +++ /dev/null @@ -1,35 +0,0 @@ -(function() { - $(function() { - return $(".protected-branches-list :checkbox").change(function(e) { - var can_push, id, name, obj, url; - name = $(this).attr("name"); - if (name === "developers_can_push" || name === "developers_can_merge") { - id = $(this).val(); - can_push = $(this).is(":checked"); - url = $(this).data("url"); - return $.ajax({ - type: "PATCH", - url: url, - dataType: "json", - data: { - id: id, - protected_branch: ( - obj = {}, - obj["" + name] = can_push, - obj - ) - }, - success: function() { - var row; - row = $(e.target); - return row.closest('tr').effect('highlight'); - }, - error: function() { - return new Flash("Failed to update branch!", "alert"); - } - }); - } - }); - }); - -}).call(this); diff --git a/app/assets/javascripts/protected_branches_access_select.js.es6 b/app/assets/javascripts/protected_branches_access_select.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..e98312bbf37d2241917c28228aa3b944075bafdd --- /dev/null +++ b/app/assets/javascripts/protected_branches_access_select.js.es6 @@ -0,0 +1,63 @@ +class ProtectedBranchesAccessSelect { + constructor(container, saveOnSelect, selectDefault) { + this.container = container; + this.saveOnSelect = saveOnSelect; + + this.container.find(".allowed-to-merge").each((i, element) => { + var fieldName = $(element).data('field-name'); + var dropdown = $(element).glDropdown({ + data: gon.merge_access_levels, + selectable: true, + fieldName: fieldName, + clicked: _.chain(this.onSelect).partial(element).bind(this).value() + }); + + if (selectDefault) { + dropdown.data('glDropdown').selectRowAtIndex(document.createEvent("Event"), 0); + } + }); + + + this.container.find(".allowed-to-push").each((i, element) => { + var fieldName = $(element).data('field-name'); + var dropdown = $(element).glDropdown({ + data: gon.push_access_levels, + selectable: true, + fieldName: fieldName, + clicked: _.chain(this.onSelect).partial(element).bind(this).value() + }); + + if (selectDefault) { + dropdown.data('glDropdown').selectRowAtIndex(document.createEvent("Event"), 0); + } + }); + } + + onSelect(dropdown, selected, element, e) { + $(dropdown).find('.dropdown-toggle-text').text(selected.text); + if (this.saveOnSelect) { + return $.ajax({ + type: "POST", + url: $(dropdown).data('url'), + dataType: "json", + data: { + _method: 'PATCH', + id: $(dropdown).data('id'), + protected_branch: { + ["" + ($(dropdown).data('type')) + "_attributes"]: { + "access_level": selected.id + } + } + }, + success: function() { + var row; + row = $(e.target); + return row.closest('tr').effect('highlight'); + }, + error: function() { + return new Flash("Failed to update branch!", "alert"); + } + }); + } + } +} diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index cc3aef5199ecd24b457f729467210a2c5d9c5c66..4409477916f42799e9d827aeefa914224661e994 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -661,14 +661,28 @@ pre.light-well { } } +.new_protected_branch { + .dropdown { + display: inline; + margin-left: 15px; + } + + label { + min-width: 120px; + } +} + .protected-branches-list { a { color: $gl-gray; - font-weight: 600; &:hover { color: $gl-link-color; } + + &.is-active { + font-weight: 600; + } } } diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index 10dca47fdede1c49b774663d3a28c6f35899c48e..d28ec6e2eacc06d20f3d73fecf2bc8734743f5ca 100644 --- a/app/controllers/projects/protected_branches_controller.rb +++ b/app/controllers/projects/protected_branches_controller.rb @@ -3,19 +3,24 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController before_action :require_non_empty_project before_action :authorize_admin_project! before_action :load_protected_branch, only: [:show, :update, :destroy] + before_action :load_protected_branches, only: [:index] layout "project_settings" def index - @protected_branches = @project.protected_branches.order(:name).page(params[:page]) @protected_branch = @project.protected_branches.new - gon.push({ open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } }) + load_protected_branches_gon_variables end def create - @project.protected_branches.create(protected_branch_params) - redirect_to namespace_project_protected_branches_path(@project.namespace, - @project) + @protected_branch = ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute + if @protected_branch.persisted? + redirect_to namespace_project_protected_branches_path(@project.namespace, @project) + else + load_protected_branches + load_protected_branches_gon_variables + render :index + end end def show @@ -23,7 +28,9 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController end def update - if @protected_branch && @protected_branch.update_attributes(protected_branch_params) + @protected_branch = ProtectedBranches::UpdateService.new(@project, current_user, protected_branch_params).execute(@protected_branch) + + if @protected_branch.valid? respond_to do |format| format.json { render json: @protected_branch, status: :ok } end @@ -50,6 +57,18 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController end def protected_branch_params - params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge) + params.require(:protected_branch).permit(:name, + merge_access_level_attributes: [:access_level], + push_access_level_attributes: [:access_level]) + end + + def load_protected_branches + @protected_branches = @project.protected_branches.order(:name).page(params[:page]) + end + + def load_protected_branches_gon_variables + gon.push({ open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } }, + push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } }, + merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } } }) end end diff --git a/app/models/project.rb b/app/models/project.rb index dc44a757b4bcd676f1f056a4293705f2ce13fbb8..7aecd7860c51a770cc00ca97e603aca1654d34f5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -874,14 +874,6 @@ class Project < ActiveRecord::Base ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present? end - def developers_can_push_to_protected_branch?(branch_name) - protected_branches.matching(branch_name).any?(&:developers_can_push) - end - - def developers_can_merge_to_protected_branch?(branch_name) - protected_branches.matching(branch_name).any?(&:developers_can_merge) - end - def forked? !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?) end diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index b7011d7afdfcd827fc52761992ded119421309fc..226b3f54342134d15ef5a96e3c2ea5e67701a0d6 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -5,6 +5,12 @@ class ProtectedBranch < ActiveRecord::Base validates :name, presence: true validates :project, presence: true + has_one :merge_access_level, dependent: :destroy + has_one :push_access_level, dependent: :destroy + + accepts_nested_attributes_for :push_access_level + accepts_nested_attributes_for :merge_access_level + def commit project.commit(self.name) end diff --git a/app/models/protected_branch/merge_access_level.rb b/app/models/protected_branch/merge_access_level.rb new file mode 100644 index 0000000000000000000000000000000000000000..b1112ee737df98da6cac950331dba06aeec03c27 --- /dev/null +++ b/app/models/protected_branch/merge_access_level.rb @@ -0,0 +1,24 @@ +class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base + belongs_to :protected_branch + delegate :project, to: :protected_branch + + validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER, + Gitlab::Access::DEVELOPER] } + + def self.human_access_levels + { + Gitlab::Access::MASTER => "Masters", + Gitlab::Access::DEVELOPER => "Developers + Masters" + }.with_indifferent_access + end + + def check_access(user) + return true if user.is_admin? + + project.team.max_member_access(user.id) >= access_level + end + + def humanize + self.class.human_access_levels[self.access_level] + end +end diff --git a/app/models/protected_branch/push_access_level.rb b/app/models/protected_branch/push_access_level.rb new file mode 100644 index 0000000000000000000000000000000000000000..6a5e49cf45394b0176075844c19f33b097a57e8f --- /dev/null +++ b/app/models/protected_branch/push_access_level.rb @@ -0,0 +1,27 @@ +class ProtectedBranch::PushAccessLevel < ActiveRecord::Base + belongs_to :protected_branch + delegate :project, to: :protected_branch + + validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER, + Gitlab::Access::DEVELOPER, + Gitlab::Access::NO_ACCESS] } + + def self.human_access_levels + { + Gitlab::Access::MASTER => "Masters", + Gitlab::Access::DEVELOPER => "Developers + Masters", + Gitlab::Access::NO_ACCESS => "No one" + }.with_indifferent_access + end + + def check_access(user) + return false if access_level == Gitlab::Access::NO_ACCESS + return true if user.is_admin? + + project.team.max_member_access(user.id) >= access_level + end + + def humanize + self.class.human_access_levels[self.access_level] + end +end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e02b50ff9a2d8440bc1ffdc582699f70439e0ca8..3f6a177bf3aff35d55ed58dfa1c05fb26bd66c67 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -88,9 +88,18 @@ class GitPushService < BaseService # Set protection on the default branch if configured 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_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 }) + + params = { + name: @project.default_branch, + push_access_level_attributes: { + access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }, + merge_access_level_attributes: { + access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + } + } + + ProtectedBranches::CreateService.new(@project, current_user, params).execute end end diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..6150a2a83c93fa0d8aef395c7ec062bc4f3934b7 --- /dev/null +++ b/app/services/protected_branches/create_service.rb @@ -0,0 +1,27 @@ +module ProtectedBranches + class CreateService < BaseService + attr_reader :protected_branch + + def execute + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + + protected_branch = project.protected_branches.new(params) + + ProtectedBranch.transaction do + protected_branch.save! + + if protected_branch.push_access_level.blank? + protected_branch.create_push_access_level!(access_level: Gitlab::Access::MASTER) + end + + if protected_branch.merge_access_level.blank? + protected_branch.create_merge_access_level!(access_level: Gitlab::Access::MASTER) + end + end + + protected_branch + rescue ActiveRecord::RecordInvalid + protected_branch + end + end +end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..89d8ba601345f7cea3f7921aea14da7e1428c19b --- /dev/null +++ b/app/services/protected_branches/update_service.rb @@ -0,0 +1,13 @@ +module ProtectedBranches + class UpdateService < BaseService + attr_reader :protected_branch + + def execute(protected_branch) + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + + @protected_branch = protected_branch + @protected_branch.update(params) + @protected_branch + end + end +end diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml index 720d67dff7c4ff7faa08a3a0b8bd20a14b90db43..0603a014008cf43e9bb0c8312c80e605575c53f9 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -5,24 +5,22 @@ No branches are protected, protect a branch with the form above. - else - can_admin_project = can?(current_user, :admin_project, @project) - .table-responsive - %table.table.protected-branches-list - %colgroup - %col{ width: "20%" } - %col{ width: "30%" } - %col{ width: "25%" } - %col{ width: "25%" } + + %table.table.protected-branches-list + %colgroup + %col{ width: "20%" } + %col{ width: "30%" } + %col{ width: "25%" } + %col{ width: "25%" } + %thead + %tr + %th Branch + %th Last commit + %th Allowed to merge + %th Allowed to push - if can_admin_project - %col - %thead - %tr - %th Protected Branch - %th Commit - %th Developers Can Push - %th Developers Can Merge - - if can_admin_project - %th - %tbody - = render partial: @protected_branches, locals: { can_admin_project: can_admin_project } + %th + %tbody + = render partial: @protected_branches, locals: { can_admin_project: can_admin_project } = paginate @protected_branches, theme: 'gitlab' diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml index 7fda7f96047342065fd51532bb16c33b4e873e01..498e412235e44dca9a46eec653d37e5f5e0db97e 100644 --- a/app/views/projects/protected_branches/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_protected_branch.html.haml @@ -15,9 +15,15 @@ - else (branch was removed from repository) %td - = check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url }) + = hidden_field_tag "allowed_to_merge_#{protected_branch.id}", protected_branch.merge_access_level.access_level + = dropdown_tag(protected_branch.merge_access_level.humanize, + options: { title: "Allowed to merge", toggle_class: 'allowed-to-merge', dropdown_class: 'dropdown-menu-selectable merge', + data: { field_name: "allowed_to_merge_#{protected_branch.id}", url: url, id: protected_branch.id, type: "merge_access_level" }}) %td - = check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url }) + = hidden_field_tag "allowed_to_push_#{protected_branch.id}", protected_branch.push_access_level.access_level + = dropdown_tag(protected_branch.push_access_level.humanize, + options: { title: "Allowed to push", toggle_class: 'allowed-to-push', dropdown_class: 'dropdown-menu-selectable push', + data: { field_name: "allowed_to_push_#{protected_branch.id}", url: url, id: protected_branch.id, type: "push_access_level" }}) - if can_admin_project %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" diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 950df740bbca978339600ca1235a2878ae06c8b2..4efe44c72335db454cd746936acc6c543b317d11 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -32,18 +32,22 @@ are supported. .form-group - = f.check_box :developers_can_push, class: "pull-left" - .prepend-left-20 - = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" - %p.light.append-bottom-0 - Allow developers to push to this branch + = hidden_field_tag 'protected_branch[merge_access_level_attributes][access_level]' + = label_tag "Allowed to merge: ", nil, class: "label-light append-bottom-0" + = dropdown_tag("<Make a selection>", + options: { title: "Allowed to merge", toggle_class: 'allowed-to-merge', + dropdown_class: 'dropdown-menu-selectable', + data: { field_name: "protected_branch[merge_access_level_attributes][access_level]" }}) .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 + = hidden_field_tag 'protected_branch[push_access_level_attributes][access_level]' + = label_tag "Allowed to push: ", nil, class: "label-light append-bottom-0" + = dropdown_tag("<Make a selection>", + options: { title: "Allowed to push", toggle_class: 'allowed-to-push', + dropdown_class: 'dropdown-menu-selectable', + data: { field_name: "protected_branch[push_access_level_attributes][access_level]" }}) + + = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true %hr diff --git a/db/fixtures/development/16_protected_branches.rb b/db/fixtures/development/16_protected_branches.rb new file mode 100644 index 0000000000000000000000000000000000000000..103c7f9445c08f9cfa436e2f8e092095cc6c413f --- /dev/null +++ b/db/fixtures/development/16_protected_branches.rb @@ -0,0 +1,12 @@ +Gitlab::Seeder.quiet do + admin_user = User.find(1) + + Project.all.each do |project| + params = { + name: 'master' + } + + ProtectedBranches::CreateService.new(project, admin_user, params).execute + print '.' + end +end diff --git a/db/migrate/20160705054938_add_protected_branches_push_access.rb b/db/migrate/20160705054938_add_protected_branches_push_access.rb new file mode 100644 index 0000000000000000000000000000000000000000..f27295524e1b12cee8b96fd5c5c0c6650a405704 --- /dev/null +++ b/db/migrate/20160705054938_add_protected_branches_push_access.rb @@ -0,0 +1,17 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProtectedBranchesPushAccess < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :protected_branch_push_access_levels do |t| + t.references :protected_branch, index: { name: "index_protected_branch_push_access" }, foreign_key: true, null: false + + # Gitlab::Access::MASTER == 40 + t.integer :access_level, default: 40, null: false + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20160705054952_add_protected_branches_merge_access.rb b/db/migrate/20160705054952_add_protected_branches_merge_access.rb new file mode 100644 index 0000000000000000000000000000000000000000..32adfa266cd7c252583f536d5235ea249e818a8b --- /dev/null +++ b/db/migrate/20160705054952_add_protected_branches_merge_access.rb @@ -0,0 +1,17 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProtectedBranchesMergeAccess < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :protected_branch_merge_access_levels do |t| + t.references :protected_branch, index: { name: "index_protected_branch_merge_access" }, foreign_key: true, null: false + + # Gitlab::Access::MASTER == 40 + t.integer :access_level, default: 40, null: false + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb b/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb new file mode 100644 index 0000000000000000000000000000000000000000..fa93936ced7637deae62ac332de08757dadd4bc7 --- /dev/null +++ b/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MoveFromDevelopersCanMergeToProtectedBranchesMergeAccess < ActiveRecord::Migration + DOWNTIME = true + DOWNTIME_REASON = <<-HEREDOC + We're creating a `merge_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this + is running, we might be left with a `protected_branch` _without_ an associated `merge_access_level`. The `protected_branches` + table must not change while this is running, so downtime is required. + + https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410 + HEREDOC + + def up + execute <<-HEREDOC + INSERT into protected_branch_merge_access_levels (protected_branch_id, access_level, created_at, updated_at) + SELECT id, (CASE WHEN developers_can_merge THEN 1 ELSE 0 END), now(), now() + FROM protected_branches + HEREDOC + end + + def down + execute <<-HEREDOC + UPDATE protected_branches SET developers_can_merge = TRUE + WHERE id IN (SELECT protected_branch_id FROM protected_branch_merge_access_levels + WHERE access_level = 1); + HEREDOC + end +end diff --git a/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb b/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb new file mode 100644 index 0000000000000000000000000000000000000000..56f6159d1d8e6ce1a63ae22187fa5fb83f6cf26c --- /dev/null +++ b/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MoveFromDevelopersCanPushToProtectedBranchesPushAccess < ActiveRecord::Migration + DOWNTIME = true + DOWNTIME_REASON = <<-HEREDOC + We're creating a `push_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this + is running, we might be left with a `protected_branch` _without_ an associated `push_access_level`. The `protected_branches` + table must not change while this is running, so downtime is required. + + https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410 + HEREDOC + + def up + execute <<-HEREDOC + INSERT into protected_branch_push_access_levels (protected_branch_id, access_level, created_at, updated_at) + SELECT id, (CASE WHEN developers_can_push THEN 1 ELSE 0 END), now(), now() + FROM protected_branches + HEREDOC + end + + def down + execute <<-HEREDOC + UPDATE protected_branches SET developers_can_push = TRUE + WHERE id IN (SELECT protected_branch_id FROM protected_branch_push_access_levels + WHERE access_level = 1); + HEREDOC + end +end diff --git a/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb new file mode 100644 index 0000000000000000000000000000000000000000..f563f660ddf301069ba103662892cdcc7bc8d8b0 --- /dev/null +++ b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb @@ -0,0 +1,19 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveDevelopersCanPushFromProtectedBranches < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # This is only required for `#down` + disable_ddl_transaction! + + DOWNTIME = false + + def up + remove_column :protected_branches, :developers_can_push, :boolean + end + + def down + add_column_with_default(:protected_branches, :developers_can_push, :boolean, default: false, null: false) + end +end diff --git a/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa71e06d36e846abdefbbdcd6ad73fcc1ddb2135 --- /dev/null +++ b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb @@ -0,0 +1,19 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveDevelopersCanMergeFromProtectedBranches < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # This is only required for `#down` + disable_ddl_transaction! + + DOWNTIME = false + + def up + remove_column :protected_branches, :developers_can_merge, :boolean + end + + def down + add_column_with_default(:protected_branches, :developers_can_merge, :boolean, default: false, null: false) + end +end diff --git a/db/schema.rb b/db/schema.rb index 15cee55a7bfe1ea27ab3f8ea3a68ffd57ae1db56..2d2ae5fd8405026a6624035a05c792263f580185 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -867,13 +867,29 @@ ActiveRecord::Schema.define(version: 20160722221922) do add_index "projects", ["star_count"], name: "index_projects_on_star_count", using: :btree add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree + create_table "protected_branch_merge_access_levels", force: :cascade do |t| + t.integer "protected_branch_id", null: false + t.integer "access_level", default: 40, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "protected_branch_merge_access_levels", ["protected_branch_id"], name: "index_protected_branch_merge_access", using: :btree + + create_table "protected_branch_push_access_levels", force: :cascade do |t| + t.integer "protected_branch_id", null: false + t.integer "access_level", default: 40, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "protected_branch_push_access_levels", ["protected_branch_id"], name: "index_protected_branch_push_access", using: :btree + create_table "protected_branches", force: :cascade do |t| - t.integer "project_id", null: false - t.string "name", null: false + t.integer "project_id", null: false + t.string "name", null: false t.datetime "created_at" t.datetime "updated_at" - t.boolean "developers_can_push", default: false, null: false - t.boolean "developers_can_merge", default: false, null: false end add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree @@ -1136,5 +1152,7 @@ ActiveRecord::Schema.define(version: 20160722221922) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "personal_access_tokens", "users" + add_foreign_key "protected_branch_merge_access_levels", "protected_branches" + add_foreign_key "protected_branch_push_access_levels", "protected_branches" add_foreign_key "u2f_registrations", "users" end diff --git a/features/steps/project/commits/branches.rb b/features/steps/project/commits/branches.rb index 0a42931147dc1ee7f78562f97c6fe1172ec1fec1..4bfb7e92e99657f01c22b45170ca1a2ce3a2c185 100644 --- a/features/steps/project/commits/branches.rb +++ b/features/steps/project/commits/branches.rb @@ -25,7 +25,7 @@ class Spinach::Features::ProjectCommitsBranches < Spinach::FeatureSteps step 'project "Shop" has protected branches' do project = Project.find_by(name: "Shop") - project.protected_branches.create(name: "stable") + create(:protected_branch, project: project, name: "stable") end step 'I click new branch link' do diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 66b853eb342cb37244be0d442062ca4525a3052e..a77afe634f626d96737d9fbba5be9ede41400fbe 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -35,6 +35,10 @@ module API # Protect a single branch # + # Note: The internal data model moved from `developers_can_{merge,push}` to `allowed_to_{merge,push}` + # in `gitlab-org/gitlab-ce!5081`. The API interface has not been changed (to maintain compatibility), + # but it works with the changed data model to infer `developers_can_merge` and `developers_can_push`. + # # Parameters: # id (required) - The ID of a project # branch (required) - The name of the branch @@ -49,17 +53,36 @@ module API @branch = user_project.repository.find_branch(params[:branch]) not_found!('Branch') unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) - developers_can_push = to_boolean(params[:developers_can_push]) + developers_can_merge = to_boolean(params[:developers_can_merge]) + developers_can_push = to_boolean(params[:developers_can_push]) + + protected_branch_params = { + name: @branch.name + } + + unless developers_can_merge.nil? + protected_branch_params.merge!({ + merge_access_level_attributes: { + access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + } + }) + end + + unless developers_can_push.nil? + protected_branch_params.merge!({ + push_access_level_attributes: { + access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + } + }) + end if protected_branch - protected_branch.developers_can_push = developers_can_push unless developers_can_push.nil? - protected_branch.developers_can_merge = developers_can_merge unless developers_can_merge.nil? - protected_branch.save + service = ProtectedBranches::UpdateService.new(user_project, current_user, protected_branch_params) + service.execute(protected_branch) else - user_project.protected_branches.create(name: @branch.name, - developers_can_push: developers_can_push || false, - developers_can_merge: developers_can_merge || false) + service = ProtectedBranches::CreateService.new(user_project, current_user, protected_branch_params) + service.execute end present @branch, with: Entities::RepoBranch, project: user_project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e76e7304674da0edaaa69e41513f0c8a1cb17659..4eb95d8a2151a7cdf055af51bf85520568e32079 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -126,11 +126,13 @@ module API end expose :developers_can_push do |repo_branch, options| - options[:project].developers_can_push_to_protected_branch? repo_branch.name + project = options[:project] + project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.push_access_level.access_level == Gitlab::Access::DEVELOPER } end expose :developers_can_merge do |repo_branch, options| - options[:project].developers_can_merge_to_protected_branch? repo_branch.name + project = options[:project] + project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.merge_access_level.access_level == Gitlab::Access::DEVELOPER } end end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index c0f85e9b3a85a509be84b9ece06c8ddc364d1160..3a69027368fdbd2c32c08f905381e6f9ee31e4e2 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -29,8 +29,9 @@ module Gitlab def can_push_to_branch?(ref) return false unless user - if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref) - user.can?(:push_code_to_protected_branches, project) + if project.protected_branch?(ref) + access_levels = project.protected_branches.matching(ref).map(&:push_access_level) + access_levels.any? { |access_level| access_level.check_access(user) } else user.can?(:push_code, project) end @@ -39,8 +40,9 @@ module Gitlab 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) + if project.protected_branch?(ref) + access_levels = project.protected_branches.matching(ref).map(&:merge_access_level) + access_levels.any? { |access_level| access_level.check_access(user) } else user.can?(:push_code, project) end diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index 28ed8078157f37861fb75c7d077f558dda41496a..5575852c2d77af2f5105580147d0ff43d9a402b5 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -2,5 +2,28 @@ FactoryGirl.define do factory :protected_branch do name project + + after(:create) do |protected_branch| + protected_branch.create_push_access_level!(access_level: Gitlab::Access::MASTER) + protected_branch.create_merge_access_level!(access_level: Gitlab::Access::MASTER) + end + + trait :developers_can_push do + after(:create) do |protected_branch| + protected_branch.push_access_level.update!(access_level: Gitlab::Access::DEVELOPER) + end + end + + trait :developers_can_merge do + after(:create) do |protected_branch| + protected_branch.merge_access_level.update!(access_level: Gitlab::Access::DEVELOPER) + end + end + + trait :no_one_can_push do + after(:create) do |protected_branch| + protected_branch.push_access_level.update!(access_level: Gitlab::Access::NO_ACCESS) + end + end end end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index d94dee0c797628df3652597e98a97d0b4b93e61d..57734b33a44a81a89efeba35a2311a98b22e054a 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Projected Branches', feature: true, js: true do + include WaitForAjax + let(:user) { create(:user, :admin) } let(:project) { create(:project) } @@ -81,4 +83,68 @@ feature 'Projected Branches', feature: true, js: true do end end end + + describe "access control" do + ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| + it "allows creating protected branches that #{access_type_name} can push to" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + within('.new_protected_branch') do + find(".allowed-to-push").click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id) + end + + it "allows updating protected branches so that #{access_type_name} can push to them" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + + within(".protected-branches-list") do + find(".allowed-to-push").click + within('.dropdown-menu.push') { click_on access_type_name } + end + + wait_for_ajax + expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id) + end + end + + ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| + it "allows creating protected branches that #{access_type_name} can merge to" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + within('.new_protected_branch') do + find(".allowed-to-merge").click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id) + end + + it "allows updating protected branches so that #{access_type_name} can merge to them" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + + within(".protected-branches-list") do + find(".allowed-to-merge").click + within('.dropdown-menu.merge') { click_on access_type_name } + end + + wait_for_ajax + expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id) + end + end + end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ae064a878b03c687338c2be5795beda7826a72e8..8447305a316df18f55d809a5b2a627236350684f 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -151,7 +151,13 @@ describe Gitlab::GitAccess, lib: true do def self.run_permission_checks(permissions_matrix) permissions_matrix.keys.each do |role| describe "#{role} access" do - before { project.team << [user, role] } + before do + if role == :admin + user.update_attribute(:admin, true) + else + project.team << [user, role] + end + end permissions_matrix[role].each do |action, allowed| context action do @@ -165,6 +171,17 @@ describe Gitlab::GitAccess, lib: true do end permissions_matrix = { + admin: { + push_new_branch: true, + push_master: true, + push_protected_branch: true, + push_remove_protected_branch: false, + push_tag: true, + push_new_tag: true, + push_all: true, + merge_into_protected_branch: true + }, + master: { push_new_branch: true, push_master: true, @@ -217,19 +234,20 @@ describe Gitlab::GitAccess, lib: true do run_permission_checks(permissions_matrix) end - context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do - before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) } + context "when developers are allowed to push into the #{protected_branch_type} protected branch" do + before { create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) end - context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do - before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) } + context "developers are allowed to merge into the #{protected_branch_type} protected branch" do + before { create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) } context "when a merge request exists for the given source/target branch" do context "when the merge request is in progress" do before do - create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch) + create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', + state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch) end run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true })) @@ -242,51 +260,59 @@ describe Gitlab::GitAccess, lib: true do run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) end - end - context "when a merge request does not exist for the given source/target branch" do - run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) + context "when a merge request does not exist for the given source/target branch" do + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) + end end end - context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do - before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, developers_can_push: true, project: project) } + context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do + before { create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) end + + context "when no one is allowed to push to the #{protected_branch_name} protected branch" do + before { create(:protected_branch, :no_one_can_push, name: protected_branch_name, project: project) } + + run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }, + master: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }, + admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false })) + end end + end - describe 'deploy key permissions' do - let(:key) { create(:deploy_key) } - let(:actor) { key } + describe 'deploy key permissions' do + let(:key) { create(:deploy_key) } + let(:actor) { key } - context 'push code' do - subject { access.check('git-receive-pack') } + context 'push code' do + subject { access.check('git-receive-pack') } - context 'when project is authorized' do - before { key.projects << project } + context 'when project is authorized' do + before { key.projects << project } - it { expect(subject).not_to be_allowed } - end + it { expect(subject).not_to be_allowed } + end - context 'when unauthorized' do - context 'to public project' do - let(:project) { create(:project, :public) } + context 'when unauthorized' do + context 'to public project' do + let(:project) { create(:project, :public) } - it { expect(subject).not_to be_allowed } - end + it { expect(subject).not_to be_allowed } + end - context 'to internal project' do - let(:project) { create(:project, :internal) } + context 'to internal project' do + let(:project) { create(:project, :internal) } - it { expect(subject).not_to be_allowed } - end + it { expect(subject).not_to be_allowed } + end - context 'to private project' do - let(:project) { create(:project, :internal) } + context 'to private project' do + let(:project) { create(:project, :internal) } - it { expect(subject).not_to be_allowed } - end + it { expect(subject).not_to be_allowed } end end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index aa9ec243498bd9d706ec135fb7aa6afee2f26fa1..5bb095366fa1d7cc2650d4f3b90ad1e498a6e2ff 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -44,7 +44,7 @@ describe Gitlab::UserAccess, lib: true do describe 'push to protected branch if allowed for developers' do before do - @branch = create :protected_branch, project: project, developers_can_push: true + @branch = create :protected_branch, :developers_can_push, project: project end it 'returns true if user is a master' do @@ -65,7 +65,7 @@ describe Gitlab::UserAccess, lib: true do describe 'merge to protected branch if allowed for developers' do before do - @branch = create :protected_branch, project: project, developers_can_merge: true + @branch = create :protected_branch, :developers_can_merge, project: project end it 'returns true if user is a master' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 72b8a4e25bdd29cb1d86761acc999a00b5778672..e365e4e98b2bdf2ad9551e615671a8e3d383681f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1095,46 +1095,6 @@ describe Project, models: true do end end - describe "#developers_can_push_to_protected_branch?" do - let(:project) { create(:empty_project) } - - context "when the branch matches a protected branch via direct match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('production')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production')).to be false - end - end - - context "when the branch matches a protected branch via wilcard match" do - it "returns true if 'Developers can Push' is turned on" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true - end - - it "returns false if 'Developers can Push' is turned off" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: false) - - expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false - end - end - - context "when the branch does not match a protected branch" do - it "returns false" do - create(:protected_branch, name: "production/*", project: project, developers_can_push: true) - - expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false - end - end - end - describe '#container_registry_path_with_namespace' do let(:project) { create(:empty_project, path: 'PROJECT') } diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 719da27f91982c5f7182be71fabb271f4933f4e5..e8fd697965f3c9bdd86be29940ce5c5d44ecca10 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -112,7 +112,7 @@ describe API::API, api: true do before do project.repository.add_branch(user, protected_branch, 'master') - create(:protected_branch, project: project, name: protected_branch, developers_can_push: true, developers_can_merge: true) + create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: protected_branch) end it 'updates that a developer can push' do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 47c0580e0f044339888250b46ceecf915ab4f947..ffa998dffc3e18ead1d88c46d8210361201bc163 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -7,6 +7,7 @@ describe GitPushService, services: true do let(:project) { create :project } before do + project.team << [user, :master] @blankrev = Gitlab::Git::BLANK_SHA @oldrev = sample_commit.parent_id @newrev = sample_commit.id @@ -172,7 +173,7 @@ describe GitPushService, services: true do describe "Push Event" do before do service = execute_service(project, user, @oldrev, @newrev, @ref ) - @event = Event.last + @event = Event.find_by_action(Event::PUSHED) @push_data = service.push_data end @@ -224,8 +225,10 @@ describe GitPushService, services: true do it "when pushing a branch for the first time" do 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: false }) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER) + expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::MASTER) end it "when pushing a branch for the first time with default branch protection disabled" do @@ -233,8 +236,8 @@ describe GitPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - expect(project.protected_branches).not_to receive(:create) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) + expect(project.protected_branches).to be_empty end it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do @@ -242,9 +245,12 @@ describe GitPushService, services: true do 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: true, developers_can_merge: false }) - execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master') + execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) + + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_level.access_level).to eq(Gitlab::Access::DEVELOPER) + expect(project.protected_branches.last.merge_access_level.access_level).to eq(Gitlab::Access::MASTER) end it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do @@ -252,8 +258,10 @@ describe GitPushService, services: true do 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' ) + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER) + expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::DEVELOPER) end it "when pushing new commits to existing branch" do