diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb
index c5454883060bb27aef5bd06b116a94f53d02e204..d4f7d0bc52155fc0e8f422b675d9747800b49b74 100644
--- a/app/controllers/projects/protected_branches_controller.rb
+++ b/app/controllers/projects/protected_branches_controller.rb
@@ -19,9 +19,13 @@ def access_levels
[:merge_access_levels, :push_access_levels]
end
- def protected_ref_params
- params.require(:protected_branch).permit(:name,
- merge_access_levels_attributes: access_level_attributes,
- push_access_levels_attributes: access_level_attributes)
+ def protected_ref_params(*attrs)
+ attrs = ([:name,
+ merge_access_levels_attributes: access_level_attributes,
+ push_access_levels_attributes: access_level_attributes] + attrs).uniq
+
+ params.require(:protected_branch).permit(attrs)
end
end
+
+Projects::ProtectedBranchesController.prepend_if_ee('EE::Projects::ProtectedBranchesController')
diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index 8769d3eb916b8a8e05c839b4ca5d5cb29ab5e7f3..1857a59e01c86d8d0190fc666f0ec0afc0e82542 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -40,6 +40,11 @@ def self.default_branch_protected?
def self.protected_refs(project)
project.protected_branches.select(:name)
end
+
+ def self.branch_requires_code_owner_approval?(project, branch_name)
+ # NOOP
+ #
+ end
end
ProtectedBranch.prepend_if_ee('EE::ProtectedBranch')
diff --git a/app/views/projects/protected_branches/shared/_branches_list.html.haml b/app/views/projects/protected_branches/shared/_branches_list.html.haml
index 1913d06a6f87f042785bf85b21f6672550b83a27..ff8dae08ad0530516abf7c1960bcf99d11d44b20 100644
--- a/app/views/projects/protected_branches/shared/_branches_list.html.haml
+++ b/app/views/projects/protected_branches/shared/_branches_list.html.haml
@@ -1,9 +1,9 @@
.protected-branches-list.js-protected-branches-list.qa-protected-branches-list
- if @protected_branches.empty?
.card-header.bg-white
- Protected branch (#{@protected_branches_count})
+ = s_("ProtectedBranch|Protected branch (%{protected_branches_count})") % { protected_branches_count: @protected_branches_count }
%p.settings-message.text-center
- There are currently no protected branches, protect a branch with the form above.
+ = s_("ProtectedBranch|There are currently no protected branches, protect a branch with the form above.")
- else
%table.table.table-bordered
%colgroup
@@ -15,10 +15,17 @@
%col
%thead
%tr
- %th Protected branch (#{@protected_branches_count})
- %th Last commit
- %th Allowed to merge
- %th Allowed to push
+ %th
+ = s_("ProtectedBranch|Protected branch (%{protected_branches_count})") % { protected_branches_count: @protected_branches_count }
+ %th
+ = s_("ProtectedBranch|Last commit")
+ %th
+ = s_("ProtectedBranch|Allowed to merge")
+ %th
+ = s_("ProtectedBranch|Allowed to push")
+
+ = render_if_exists 'projects/protected_branches/ee/code_owner_approval_table_head'
+
- if can_admin_project
%th
%tbody
diff --git a/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml b/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml
index bba4949277d449a81b68dbfbcc4d27e34b37da00..f84c7b39733e69d98fa73dd24cb6be95ee2a06f1 100644
--- a/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml
+++ b/app/views/projects/protected_branches/shared/_create_protected_branch.html.haml
@@ -2,7 +2,7 @@
%input{ type: 'hidden', name: 'update_section', value: 'js-protected-branches-settings' }
.card
.card-header
- Protect a branch
+ = s_("ProtectedBranch|Protect a branch")
.card-body
= form_errors(@protected_branch)
.form-group.row
@@ -11,22 +11,19 @@
.col-md-10
= render partial: "projects/protected_branches/shared/dropdown", locals: { f: f }
.form-text.text-muted
- = link_to 'Wildcards', help_page_path('user/project/protected_branches', anchor: 'wildcard-protected-branches')
- such as
- %code *-stable
- or
- %code production/*
- are supported
+ - wildcards_url = help_page_url('user/project/protected_branches', anchor: 'wildcard-protected-branches')
+ - wildcards_link_start = ''.html_safe % { url: wildcards_url }
+ = (s_("ProtectedBranch|%{wildcards_link_start}Wildcards%{wildcards_link_end} such as %{code_tag_start}*-stable%{code_tag_end} or %{code_tag_start}production/*%{code_tag_end} are supported") % { wildcards_link_start: wildcards_link_start, wildcards_link_end: '', code_tag_start: '', code_tag_end: '' }).html_safe
.form-group.row
%label.col-md-2.text-right{ for: 'merge_access_levels_attributes' }
- Allowed to merge:
+ = s_("ProtectedBranch|Allowed to merge:")
.col-md-10
= yield :merge_access_levels
.form-group.row
%label.col-md-2.text-right{ for: 'push_access_levels_attributes' }
- Allowed to push:
+ = s_("ProtectedBranch|Allowed to push:")
.col-md-10
= yield :push_access_levels
-
+ = render_if_exists 'projects/protected_branches/ee/code_owner_approval_form'
.card-footer
- = f.submit 'Protect', class: 'btn-success btn', disabled: true, data: { qa_selector: 'protect_button' }
+ = f.submit s_('ProtectedBranch|Protect'), class: 'btn-success btn', disabled: true, data: { qa_selector: 'protect_button' }
diff --git a/app/views/projects/protected_branches/shared/_protected_branch.html.haml b/app/views/projects/protected_branches/shared/_protected_branch.html.haml
index 81dcab1d1ab440305aab2f9c568fcbc6d8f7eded..2768e4ac5a546c92742ebe9d68d3c430e95b0bd6 100644
--- a/app/views/projects/protected_branches/shared/_protected_branch.html.haml
+++ b/app/views/projects/protected_branches/shared/_protected_branch.html.haml
@@ -19,6 +19,8 @@
= yield
+ = render_if_exists 'projects/protected_branches/ee/code_owner_approval_table', protected_branch: protected_branch
+
- if can_admin_project
%td
= link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch, { update_section: 'js-protected-branches-settings' }], disabled: local_assigns[:disabled], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning"
diff --git a/db/post_migrate/20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb b/db/post_migrate/20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b109f582909e67aae5e0b8ca8f2c5cefc4c85b1d
--- /dev/null
+++ b/db/post_migrate/20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+class MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ disable_ddl_transaction!
+
+ DOWNTIME = false
+ BATCH_SIZE = 200
+
+ class Project < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'projects'
+ self.inheritance_column = :_type_disabled
+
+ has_many :protected_branches
+ end
+
+ class ProtectedBranch < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'protected_branches'
+ self.inheritance_column = :_type_disabled
+
+ belongs_to :project
+ end
+
+ def up
+ add_concurrent_index :projects, :id, name: "temp_active_projects_with_prot_branches", where: 'archived = false and pending_delete = false and merge_requests_require_code_owner_approval = true'
+
+ ProtectedBranch
+ .joins(:project)
+ .where(projects: { archived: false, pending_delete: false, merge_requests_require_code_owner_approval: true })
+ .each_batch(of: BATCH_SIZE) do |batch|
+ batch.update_all(code_owner_approval_required: true)
+ end
+
+ remove_concurrent_index_by_name(:projects, "temp_active_projects_with_prot_branches")
+ end
+
+ def down
+ # noop
+ #
+ end
+end
diff --git a/doc/api/protected_branches.md b/doc/api/protected_branches.md
index debf1b264f9e36d32c58d12960b9baf6c88f7b3f..4a750b42f65f878f1994dab8771616fbe82b996b 100644
--- a/doc/api/protected_branches.md
+++ b/doc/api/protected_branches.md
@@ -296,3 +296,21 @@ curl --request DELETE --header "PRIVATE-TOKEN: " 'https://git
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `name` | string | yes | The name of the branch |
+
+## Require code owner approvals for a single branch
+
+Update the "code owner approval required" option for the given protected branch protected branch.
+
+```
+PATCH /projects/:id/protected_branches/:name
+```
+
+```bash
+curl --request PATCH --header "PRIVATE-TOKEN: " 'https://gitlab.example.com/api/v4/projects/5/protected_branches/feature-branch'
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `name` | string | yes | The name of the branch |
+| `code_owner_approval_required` | boolean | no | **(PREMIUM)** Prevent pushes to this branch if it matches an item in the [`CODEOWNERS` file](../user/project/code_owners.md). (defaults: false)|
diff --git a/doc/user/project/img/protected_branches_list_v12_3.png b/doc/user/project/img/protected_branches_list_v12_3.png
index 365d8d99e5a605d0067bcc97baf510bd7ba92b7c..2353ddd23beb838a093a6047c3cce8c8a1e40a90 100644
Binary files a/doc/user/project/img/protected_branches_list_v12_3.png and b/doc/user/project/img/protected_branches_list_v12_3.png differ
diff --git a/doc/user/project/img/protected_branches_page_v12_3.png b/doc/user/project/img/protected_branches_page_v12_3.png
index 17f196425527646a6d9d51c3b8f1102405b4abca..9a194c85c41b03d72fee6c15bf0dbf269b72cf1f 100644
Binary files a/doc/user/project/img/protected_branches_page_v12_3.png and b/doc/user/project/img/protected_branches_page_v12_3.png differ
diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md
index 9a6cd3dc3626630a92199f6e6c36c450aadd6824..1bd272bdd0c0e098638f76faf6c5a0b94024f11b 100644
--- a/doc/user/project/protected_branches.md
+++ b/doc/user/project/protected_branches.md
@@ -86,6 +86,20 @@ Click **Protect** and the branch will appear in the "Protected branch" list.
+## Code Owners approvals **(PREMIUM)**
+
+It is possible to require at least one approval for each entry in the
+[`CODEOWNERS` file](code_owners.md) that matches a file changed in
+the merge request. To enable this feature:
+
+1. Toggle the **Require approval from code owners** slider.
+
+1. Click **Protect**.
+
+When this feature is enabled, all merge requests need approval
+from one code owner per matched rule before they can be merged. Additionally,
+pushes to the protected branch are denied if a rule is matched.
+
## Wildcard protected branches
> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/4665) in GitLab 8.10.
diff --git a/ee/app/assets/javascripts/protected_branches/protected_branch_create.js b/ee/app/assets/javascripts/protected_branches/protected_branch_create.js
index 973e256a1f9dacce0f249d21c5ee61b599b1f683..c54dd2cf92b33b33b716c3e7c1ab9c5d5d51254a 100644
--- a/ee/app/assets/javascripts/protected_branches/protected_branch_create.js
+++ b/ee/app/assets/javascripts/protected_branches/protected_branch_create.js
@@ -14,13 +14,19 @@ export default class ProtectedBranchCreate {
this.currentProjectUserDefaults = {};
this.buildDropdowns();
this.$branchInput = this.$form.find('input[name="protected_branch[name]"]');
+ this.$codeOwnerToggle = this.$form.find('.js-code-owner-toggle');
this.bindEvents();
}
bindEvents() {
+ this.$codeOwnerToggle.on('click', this.onCodeOwnerToggleClick.bind(this));
this.$form.on('submit', this.onFormSubmit.bind(this));
}
+ onCodeOwnerToggleClick() {
+ this.$codeOwnerToggle.toggleClass('is-checked');
+ }
+
buildDropdowns() {
const $allowedToMergeDropdown = this.$form.find('.js-allowed-to-merge');
const $allowedToPushDropdown = this.$form.find('.js-allowed-to-push');
@@ -75,6 +81,7 @@ export default class ProtectedBranchCreate {
authenticity_token: this.$form.find('input[name="authenticity_token"]').val(),
protected_branch: {
name: this.$form.find('input[name="protected_branch[name]"]').val(),
+ code_owner_approval_required: this.$codeOwnerToggle.hasClass('is-checked'),
},
};
diff --git a/ee/app/assets/javascripts/protected_branches/protected_branch_edit.js b/ee/app/assets/javascripts/protected_branches/protected_branch_edit.js
index 6eb7541501968565c21fb821059a8d0616ae4797..d94c42c20d1d82a858307cb6419cf02fab8c03f7 100644
--- a/ee/app/assets/javascripts/protected_branches/protected_branch_edit.js
+++ b/ee/app/assets/javascripts/protected_branches/protected_branch_edit.js
@@ -1,4 +1,3 @@
-import $ from 'jquery';
import _ from 'underscore';
import axios from '~/lib/utils/axios_utils';
import Flash from '~/flash';
@@ -13,6 +12,7 @@ export default class ProtectedBranchEdit {
this.$wrap = options.$wrap;
this.$allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge');
this.$allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push');
+ this.$codeOwnerToggle = this.$wrap.find('.js-code-owner-toggle');
this.$wraps[ACCESS_LEVELS.MERGE] = this.$allowedToMergeDropdown.closest(
`.${ACCESS_LEVELS.MERGE}-container`,
@@ -22,6 +22,35 @@ export default class ProtectedBranchEdit {
);
this.buildDropdowns();
+ this.bindEvents();
+ }
+
+ bindEvents() {
+ this.$codeOwnerToggle.on('click', this.onCodeOwnerToggleClick.bind(this));
+ }
+
+ onCodeOwnerToggleClick() {
+ this.$codeOwnerToggle.toggleClass('is-checked');
+ this.$codeOwnerToggle.prop('disabled', true);
+
+ const formData = {
+ code_owner_approval_required: this.$codeOwnerToggle.hasClass('is-checked'),
+ };
+
+ this.updateCodeOwnerApproval(formData);
+ }
+
+ updateCodeOwnerApproval(formData) {
+ axios
+ .patch(this.$wrap.data('url'), {
+ protected_branch: formData,
+ })
+ .then(() => {
+ this.$codeOwnerToggle.prop('disabled', false);
+ })
+ .catch(() => {
+ Flash(__('Failed to update branch!'));
+ });
}
buildDropdowns() {
@@ -85,7 +114,7 @@ export default class ProtectedBranchEdit {
.catch(() => {
this.$allowedToMergeDropdown.enable();
this.$allowedToPushDropdown.enable();
- Flash(__('Failed to update branch!'), null, $('.js-protected-branches-list'));
+ Flash(__('Failed to update branch!'));
});
}
diff --git a/ee/app/controllers/ee/projects/protected_branches_controller.rb b/ee/app/controllers/ee/projects/protected_branches_controller.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f1964210371557990855b35b35da268c291ed0ff
--- /dev/null
+++ b/ee/app/controllers/ee/projects/protected_branches_controller.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module EE
+ module Projects
+ module ProtectedBranchesController
+ extend ::Gitlab::Utils::Override
+
+ protected
+
+ override :protected_ref_params
+ def protected_ref_params(*attrs)
+ params_hash = super(:code_owner_approval_required)
+
+ params_hash[:code_owner_approval_required] = false unless project.code_owner_approval_required_available?
+
+ params_hash
+ end
+ end
+ end
+end
diff --git a/ee/app/models/approval_wrapped_rule.rb b/ee/app/models/approval_wrapped_rule.rb
index 0b7d2f3550d88d53cb8fb6fd4da6374e5ad2be61..4fa106aac60414e01312b0de10c3f3dc04ec4d45 100644
--- a/ee/app/models/approval_wrapped_rule.rb
+++ b/ee/app/models/approval_wrapped_rule.rb
@@ -92,16 +92,15 @@ def approvals_required
def code_owner_approvals_required
strong_memoize(:code_owner_approvals_required) do
- next 0 unless merge_requests_require_code_owner_approval?
+ next 0 unless branch_requires_code_owner_approval?
approvers.any? ? REQUIRED_APPROVALS_PER_CODE_OWNER_RULE : 0
end
end
- def merge_requests_require_code_owner_approval?
+ def branch_requires_code_owner_approval?
return false unless project.code_owner_approval_required_available?
- project.merge_requests_require_code_owner_approval? ||
- project.branch_requires_code_owner_approval?(merge_request.target_branch)
+ ProtectedBranch.branch_requires_code_owner_approval?(project, merge_request.target_branch)
end
end
diff --git a/ee/app/models/concerns/ee/protected_branch.rb b/ee/app/models/concerns/ee/protected_branch.rb
index ab79e18fa12909c705d86bbb571d0d99b5456dda..e6f05f505a4a717c4f58916f806e0659a26857b2 100644
--- a/ee/app/models/concerns/ee/protected_branch.rb
+++ b/ee/app/models/concerns/ee/protected_branch.rb
@@ -8,6 +8,14 @@ module ProtectedBranch
protected_ref_access_levels :unprotect
end
+ class_methods do
+ def branch_requires_code_owner_approval?(project, branch_name)
+ return false unless project.code_owner_approval_required_available?
+
+ project.protected_branches.requiring_code_owner_approval.matching(branch_name).any?
+ end
+ end
+
def code_owner_approval_required
super && project.code_owner_approval_required_available?
end
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 8f27a315efc06f5038614ad023ac80d23ec8acc2..11c3fa9d8eb07d8b983797124bffc231a1fcd94a 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -106,7 +106,7 @@ module Project
scope :verification_failed_wikis, -> { joins(:repository_state).merge(ProjectRepositoryState.verification_failed_wikis) }
scope :for_plan_name, -> (name) { joins(namespace: :plan).where(plans: { name: name }) }
scope :requiring_code_owner_approval,
- -> { where(merge_requests_require_code_owner_approval: true) }
+ -> { joins(:protected_branches).where(protected_branches: { code_owner_approval_required: true }) }
scope :with_active_services, -> { joins(:services).merge(::Service.active) }
scope :with_active_jira_services, -> { joins(:services).merge(::JiraService.active) }
scope :with_jira_dvcs_cloud, -> { joins(:feature_usage).merge(ProjectFeatureUsage.with_jira_dvcs_integration_enabled(cloud: true)) }
@@ -397,13 +397,14 @@ def approver_group_ids=(value)
end
def merge_requests_require_code_owner_approval?
- super && code_owner_approval_required_available?
+ code_owner_approval_required_available? &&
+ protected_branches.requiring_code_owner_approval.any?
end
def branch_requires_code_owner_approval?(branch_name)
return false unless code_owner_approval_required_available?
- protected_branches.requiring_code_owner_approval.matching(branch_name).any?
+ ::ProtectedBranch.branch_requires_code_owner_approval?(self, branch_name)
end
def require_password_to_approve
diff --git a/ee/app/services/ee/protected_branches/create_service.rb b/ee/app/services/ee/protected_branches/create_service.rb
index 996bfa910b3faaa18f6b114ead636db70cb3c7d2..17c9ac21b64683a5dba46d89e069a767c49441ab 100644
--- a/ee/app/services/ee/protected_branches/create_service.rb
+++ b/ee/app/services/ee/protected_branches/create_service.rb
@@ -18,6 +18,8 @@ def execute(skip_authorization: false)
override :save_protected_branch
def save_protected_branch
+ protected_branch.code_owner_approval_required = false unless project.feature_available?(:code_owner_approval_required)
+
super
sync_code_owner_approval_rules if project.feature_available?(:code_owners)
diff --git a/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml b/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml
index 49179755d8ebc48f5b6ad52cb73b805c9e0b2f82..8800650f4297058358dc2034085898f84fca66ac 100644
--- a/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml
+++ b/ee/app/views/projects/_merge_request_approvals_settings_form.html.haml
@@ -11,14 +11,6 @@
.text-center.prepend-top-default
= sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
-- if project.code_owner_approval_required_available?
- .form-group.require-code-owner-approval
- .form-check
- = form.check_box(:merge_requests_require_code_owner_approval, class: 'form-check-input qa-require-code-owners-approval-checkbox')
- = form.label :merge_requests_require_code_owner_approval, class: 'form-check-label' do
- %span= _('Require approval from code owners')
- = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'editing-approvals-premium'), target: '_blank'
-
.form-group
.form-check
= form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input' }, false, true)
diff --git a/ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml b/ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml
index 6243e28818178c1435035fb1415924fa3bf21ff9..b20b98f5104dc0d31c916bf91fb80a7575e6614a 100644
--- a/ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml
+++ b/ee/app/views/projects/merge_requests/_code_owner_approval_rules.html.haml
@@ -1,4 +1,4 @@
-- return unless @project.merge_requests_require_code_owner_approval?
+- return unless ProtectedBranch.branch_requires_code_owner_approval?(@project, merge_request.target_branch)
- code_owner_rules = merge_request.code_owner_rules_with_users
- return unless code_owner_rules.any?
diff --git a/ee/app/views/projects/protected_branches/ee/_code_owner_approval_form.html.haml b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_form.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..bea2e694d25aedfe4c3bc79e8b63b7fc81538aeb
--- /dev/null
+++ b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_form.html.haml
@@ -0,0 +1,13 @@
+- if @project.feature_available?(:code_owner_approval_required)
+ .form-group.row
+ %label.col-md-2.text-right{ for: 'code_owner_approval_required' }
+ = s_("ProtectedBranch|Require approval from code owners:")
+ .col-md-10
+ %button{ type: 'button',
+ class: "js-code-owner-toggle project-feature-toggle is-checked",
+ "aria-label": s_("ProtectedBranch|Toggle code owner approval") }
+ %span.toggle-icon
+ = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked')
+ = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked')
+ .form-text.text-muted
+ = s_("ProtectedBranch|Pushes that change filenames matched by the CODEOWNERS file will be rejected")
diff --git a/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table.html.haml b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..7c62aa17c41876e49150dcad66eb3e766a3d4a2b
--- /dev/null
+++ b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table.html.haml
@@ -0,0 +1,8 @@
+- if @project.feature_available?(:code_owner_approval_required)
+ %td
+ %button{ type: 'button',
+ class: "js-code-owner-toggle project-feature-toggle mr-5 #{'is-checked' if protected_branch.code_owner_approval_required}",
+ "aria-label": s_("ProtectedBranch|Toggle code owner approval") }
+ %span.toggle-icon
+ = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked')
+ = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked')
diff --git a/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table_head.html.haml b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table_head.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..7fe0438944acafec563c7c3341ec2d08693ca7ff
--- /dev/null
+++ b/ee/app/views/projects/protected_branches/ee/_code_owner_approval_table_head.html.haml
@@ -0,0 +1,3 @@
+- if @project.feature_available?(:code_owner_approval_required)
+ %th
+ = s_("ProtectedBranch|Code owner approval")
diff --git a/ee/changelogs/unreleased/13251-fe-require-approval-from-code-owners.yml b/ee/changelogs/unreleased/13251-fe-require-approval-from-code-owners.yml
new file mode 100644
index 0000000000000000000000000000000000000000..318da606dce978a17d4c2e92d1a2271895fe21d7
--- /dev/null
+++ b/ee/changelogs/unreleased/13251-fe-require-approval-from-code-owners.yml
@@ -0,0 +1,5 @@
+---
+title: Add configurable Code Owner approvals for protected branches
+merge_request: 15862
+author:
+type: added
diff --git a/ee/lib/ee/api/protected_branches.rb b/ee/lib/ee/api/protected_branches.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6f37a7364a38294f7b563f4bcec93a502ced2904
--- /dev/null
+++ b/ee/lib/ee/api/protected_branches.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module EE
+ module API
+ module ProtectedBranches
+ extend ActiveSupport::Concern
+
+ BRANCH_ENDPOINT_REQUIREMENTS = ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(name: ::API::API::NO_SLASH_URL_PART_REGEX)
+
+ prepended do
+ params do
+ requires :id, type: String, desc: 'The ID of a project'
+ end
+ resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+ desc 'Update the code_owner_approval_required state of an existing protected branch' do
+ success ::API::Entities::ProtectedBranch
+ end
+ params do
+ requires :name, type: String, desc: 'The name of the branch'
+
+ use :optional_params_ee
+ end
+ # rubocop: disable CodeReuse/ActiveRecord
+ patch ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
+ render_api_error!("Feature 'Code Owner Approval Required' is not enabled", 403) unless user_project.code_owner_approval_required_available?
+
+ protected_branch = user_project.protected_branches.find_by!(name: params[:name])
+
+ protected_branch.update_attribute(:code_owner_approval_required, declared_params[:code_owner_approval_required])
+
+ present protected_branch, with: ::API::Entities::ProtectedBranch, project: user_project
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..828b3d075a5244c561a17b599995295c4812c409
--- /dev/null
+++ b/ee/spec/controllers/ee/projects/protected_branches_controller_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require "spec_helper"
+
+describe Projects::ProtectedBranchesController do
+ let(:project) { create(:project, :repository) }
+ let(:protected_branch) { create(:protected_branch, project: project) }
+ let(:project_params) { { namespace_id: project.namespace.to_param, project_id: project } }
+ let(:user) { create(:user) }
+
+ before do
+ project.add_maintainer(user)
+ end
+
+ shared_examples "protected branch with code owner approvals feature" do |boolean|
+ it "sets code owner approvals to #{boolean} when protecting the branch" do
+ expect do
+ post(:create, params: project_params.merge(protected_branch: create_params))
+ end.to change(ProtectedBranch, :count).by(1)
+
+ expect(ProtectedBranch.last.attributes["code_owner_approval_required"]).to eq(boolean)
+ end
+ end
+
+ describe "POST #create" do
+ let(:maintainer_access_level) { [{ access_level: Gitlab::Access::MAINTAINER }] }
+ let(:access_level_params) do
+ { merge_access_levels_attributes: maintainer_access_level,
+ push_access_levels_attributes: maintainer_access_level }
+ end
+ let(:create_params) do
+ attributes_for(:protected_branch).merge(access_level_params)
+ end
+
+ before do
+ sign_in(user)
+ end
+
+ context "when code_owner_approval_required is 'false'" do
+ before do
+ create_params[:code_owner_approval_required] = false
+ end
+
+ it_behaves_like "protected branch with code owner approvals feature", false
+ end
+
+ context "when code_owner_approval_required is 'true'" do
+ before do
+ create_params[:code_owner_approval_required] = true
+ end
+
+ context "when the feature is enabled" do
+ before do
+ stub_licensed_features(code_owner_approval_required: true)
+ end
+
+ it_behaves_like "protected branch with code owner approvals feature", true
+ end
+
+ context "when the feature is not enabled" do
+ before do
+ stub_licensed_features(code_owner_approval_required: false)
+ end
+
+ it_behaves_like "protected branch with code owner approvals feature", false
+ end
+ end
+ end
+end
diff --git a/ee/spec/features/merge_request/user_sees_approval_widget_spec.rb b/ee/spec/features/merge_request/user_sees_approval_widget_spec.rb
index a6cd0ce26a1413cca5c8cc2777b836c6ee612f7b..5e5e85b84d9376dbbc097a4a5acca43bc7a90fe9 100644
--- a/ee/spec/features/merge_request/user_sees_approval_widget_spec.rb
+++ b/ee/spec/features/merge_request/user_sees_approval_widget_spec.rb
@@ -109,7 +109,11 @@
context 'when code owner approval is required' do
before do
stub_licensed_features(code_owner_approval_required: true, multiple_approval_rules: true)
+
project.update!(merge_requests_require_code_owner_approval: true)
+
+ allow(ProtectedBranch)
+ .to receive(:branch_requires_code_owner_approval?).and_return(true)
end
it 'shows the code owner rule as required' do
diff --git a/ee/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb b/ee/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb
index a6cb06eaff8577c2b3ba61e7b3ccc981bf8e5df0..52ab52783456e772924b2155d2c5cee1cbe99315 100644
--- a/ee/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb
+++ b/ee/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb
@@ -18,7 +18,6 @@
let(:project) do
create(:project, :custom_repo,
- merge_requests_require_code_owner_approval: true,
files: { 'docs/CODEOWNERS' => "*.rb @ruby-owner\n*.js @js-owner" })
end
@@ -38,6 +37,11 @@
message: 'Add a ruby file',
branch_name: 'feature')
+ create(:protected_branch,
+ name: 'master',
+ code_owner_approval_required: true,
+ project: project)
+
# To make sure the rules are created for the merge request, the services
# that do that aren't triggered from factories
MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute
diff --git a/ee/spec/features/projects/settings/user_manages_approval_settings_spec.rb b/ee/spec/features/projects/settings/user_manages_approval_settings_spec.rb
index 395d2226258f22cca1c465821c0007bfda775357..17f64d10e6e040c99472027669cd3e84af4f597f 100644
--- a/ee/spec/features/projects/settings/user_manages_approval_settings_spec.rb
+++ b/ee/spec/features/projects/settings/user_manages_approval_settings_spec.rb
@@ -34,24 +34,6 @@
end
end
- context 'when `code_owner_approval_required` is available' do
- let(:licensed_features) { { code_owner_approval_required: true } }
-
- it_behaves_like 'dirty submit form', [{ form: '#js-merge-request-approval-settings', input: '#project_merge_requests_author_approval' }]
-
- it 'allows the user to enforce code owner approval' do
- within('.require-code-owner-approval') do
- check('Require approval from code owners')
- end
-
- within('.merge-request-approval-settings-form') do
- click_on('Save changes')
- end
-
- expect(project.reload.merge_requests_require_code_owner_approval?).to be_truthy
- end
- end
-
context 'when `code_owner_approval_required` is not available' do
let(:licensed_features) { { code_owner_approval_required: false } }
diff --git a/ee/spec/features/protected_branches_spec.rb b/ee/spec/features/protected_branches_spec.rb
index e64e4c7074a020d8f0325b885310047c3c1333e8..df7c0bcf532fbd2f19af213ee6c898d983b7b207 100644
--- a/ee/spec/features/protected_branches_spec.rb
+++ b/ee/spec/features/protected_branches_spec.rb
@@ -12,6 +12,117 @@
sign_in(user)
end
+ describe 'code owner approval' do
+ describe 'when project requires code owner approval' do
+ before do
+ stub_licensed_features(protected_refs_for_users: true, code_owner_approval_required: true)
+ end
+
+ describe 'protect a branch form' do
+ let!(:protected_branch) { create(:protected_branch, project: project) }
+ let(:container) { page.find('#new_protected_branch') }
+ let(:code_owner_toggle) { container.find('.js-code-owner-toggle') }
+ let(:branch_input) { container.find('.js-protected-branch-select') }
+ let(:allowed_to_merge_input) { container.find('.js-allowed-to-merge') }
+ let(:allowed_to_push) { container.find('.js-allowed-to-push') }
+
+ before do
+ visit project_settings_repository_path(project)
+ end
+
+ def fill_in_form(branch_name)
+ branch_input.click
+ click_on branch_name
+
+ allowed_to_merge_input.click
+ wait_for_requests
+ page.find('.dropdown.show').click_on 'No one'
+
+ allowed_to_push.click
+ wait_for_requests
+ page.find('.dropdown.show').click_on 'No one'
+ end
+
+ def submit_form
+ click_on 'Protect'
+ wait_for_requests
+ end
+
+ it 'has code owner toggle' do
+ expect(page).to have_content("Require approval from code owners")
+ expect(code_owner_toggle[:class]).to include("is-checked")
+ end
+
+ it 'can create new protected branch with code owner disabled' do
+ fill_in_form "with-codeowners"
+
+ code_owner_toggle.click
+ expect(code_owner_toggle[:class]).not_to include("is-checked")
+
+ submit_form
+
+ expect(project.protected_branches.find_by_name("with-codeowners").code_owner_approval_required).to be(false)
+ end
+
+ it 'can create new protected branch with code owner enabled' do
+ fill_in_form "with-codeowners"
+
+ expect(code_owner_toggle[:class]).to include("is-checked")
+
+ submit_form
+
+ expect(project.protected_branches.find_by_name("with-codeowners").code_owner_approval_required).to be(true)
+ end
+ end
+
+ describe 'protect branch table' do
+ context 'has a protected branch with code owner approval toggled on' do
+ let!(:protected_branch) { create(:protected_branch, project: project, code_owner_approval_required: true) }
+
+ before do
+ visit project_settings_repository_path(project)
+ end
+
+ it 'shows code owner approval toggle' do
+ expect(page).to have_content("Code owner approval")
+ end
+
+ it 'displays toggle on' do
+ expect(page).to have_css('.js-code-owner-toggle.is-checked')
+ end
+ end
+
+ context 'has a protected branch with code owner approval toggled off ' do
+ let!(:protected_branch) { create(:protected_branch, project: project, code_owner_approval_required: false) }
+
+ it 'displays toggle off' do
+ visit project_settings_repository_path(project)
+
+ page.within '.qa-protected-branches-list' do
+ expect(page).not_to have_css('.js-code-owner-toggle.is-checked')
+ end
+ end
+ end
+ end
+ end
+
+ describe 'when project does not require code owner approval' do
+ before do
+ stub_licensed_features(protected_refs_for_users: true, code_owner_approval_required: false)
+
+ visit project_settings_repository_path(project)
+ end
+
+ it 'does not have code owner approval in the form' do
+ expect(page).not_to have_content("Require approval from code owners")
+ end
+
+ it 'does not have code owner approval in the table' do
+ expect(page).not_to have_content("Code owner approval")
+ end
+ end
+ end
+
describe 'access control' do
describe 'with ref permissions for users enabled' do
before do
diff --git a/ee/spec/frontend/protected_branches/protected_branch_edit_spec.js b/ee/spec/frontend/protected_branches/protected_branch_edit_spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..79f95cc6b177cd931f1ea2f6996432dff5b247f5
--- /dev/null
+++ b/ee/spec/frontend/protected_branches/protected_branch_edit_spec.js
@@ -0,0 +1,90 @@
+import $ from 'jquery';
+import MockAdapter from 'axios-mock-adapter';
+import ProtectedBranchEdit from 'ee/protected_branches/protected_branch_edit';
+import flash from '~/flash';
+import axios from '~/lib/utils/axios_utils';
+import { TEST_HOST } from 'helpers/test_constants';
+
+jest.mock('~/flash');
+
+const TEST_URL = `${TEST_HOST}/url`;
+const IS_CHECKED_CLASS = 'is-checked';
+
+describe('EE ProtectedBranchEdit', () => {
+ let mock;
+
+ beforeEach(() => {
+ setFixtures(`
+
+
`);
+
+ jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation();
+
+ mock = new MockAdapter(axios);
+ });
+
+ const findCodeOwnerToggle = () => document.querySelector('.js-code-owner-toggle');
+
+ const create = ({ isChecked = false }) => {
+ if (isChecked) {
+ findCodeOwnerToggle().classList.add(IS_CHECKED_CLASS);
+ }
+
+ return new ProtectedBranchEdit({ $wrap: $('#wrap') });
+ };
+
+ afterEach(() => {
+ mock.restore();
+ });
+
+ describe('when unchecked toggle button', () => {
+ let toggle;
+
+ beforeEach(() => {
+ create({ isChecked: false });
+
+ toggle = findCodeOwnerToggle();
+ });
+
+ it('is not changed', () => {
+ expect(toggle).not.toHaveClass(IS_CHECKED_CLASS);
+ expect(toggle).not.toBeDisabled();
+ });
+
+ describe('when clicked', () => {
+ beforeEach(() => {
+ mock
+ .onPatch(TEST_URL, { protected_branch: { code_owner_approval_required: true } })
+ .replyOnce(200, {});
+
+ toggle.click();
+ });
+
+ it('checks and disables button', () => {
+ expect(toggle).toHaveClass(IS_CHECKED_CLASS);
+ expect(toggle).toBeDisabled();
+ });
+
+ it('sends update to BE', () =>
+ axios.waitForAll().then(() => {
+ // Args are asserted in the `.onPatch` call
+ expect(mock.history.patch.length).toEqual(1);
+
+ expect(toggle).not.toBeDisabled();
+ expect(flash).not.toHaveBeenCalled();
+ }));
+ });
+
+ describe('when clikced and BE error', () => {
+ beforeEach(() => {
+ mock.onPatch(TEST_URL).replyOnce(500);
+ toggle.click();
+ });
+
+ it('flashes error', () =>
+ axios.waitForAll().then(() => {
+ expect(flash).toHaveBeenCalled();
+ }));
+ });
+ });
+});
diff --git a/ee/spec/lib/ee/gitlab/usage_data_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_spec.rb
index ec5fa90eb5967f4c66dd28d5dfbbd8254e5d668b..dd30d0868cf937abe40dbc401fd137cf8547c522 100644
--- a/ee/spec/lib/ee/gitlab/usage_data_spec.rb
+++ b/ee/spec/lib/ee/gitlab/usage_data_spec.rb
@@ -57,7 +57,7 @@
deploy_keys: 1,
keys: 1,
merge_requests: 1,
- projects_enforcing_code_owner_approval: 1,
+ projects_enforcing_code_owner_approval: 0,
projects_imported_from_github: 1,
projects_with_repositories_enabled: 1,
protected_branches: 1,
diff --git a/ee/spec/lib/gitlab/usage_data_spec.rb b/ee/spec/lib/gitlab/usage_data_spec.rb
index 7904e72cc8d3e25dee419a56458be4668531b9ea..ac22f3c9a14e1ae4d0e4d645821aefb17a75778f 100644
--- a/ee/spec/lib/gitlab/usage_data_spec.rb
+++ b/ee/spec/lib/gitlab/usage_data_spec.rb
@@ -194,9 +194,15 @@
describe 'code owner approval required' do
before do
- create(:project, :archived, :requiring_code_owner_approval)
- create(:project, :requiring_code_owner_approval, pending_delete: true)
- create(:project, :requiring_code_owner_approval)
+ create(:protected_branch, code_owner_approval_required: true)
+
+ create(:protected_branch,
+ code_owner_approval_required: true,
+ project: create(:project, :archived))
+
+ create(:protected_branch,
+ code_owner_approval_required: true,
+ project: create(:project, pending_delete: true))
end
it 'counts the projects actively requiring code owner approval' do
diff --git a/ee/spec/models/approval_wrapped_rule_spec.rb b/ee/spec/models/approval_wrapped_rule_spec.rb
index a1fbfae1ee0bf34be47956bb4ad158605754a172..a65bc507375b15b3c9af61137895603355e13704 100644
--- a/ee/spec/models/approval_wrapped_rule_spec.rb
+++ b/ee/spec/models/approval_wrapped_rule_spec.rb
@@ -196,23 +196,12 @@
.to receive(:code_owner_approval_required_available?).and_return(true)
end
- context "when no protected_branches require code owner approval" do
- it 'returns the correct number of approvals' do
- allow(subject.project)
- .to receive(:merge_requests_require_code_owner_approval?).and_return(feature_enabled)
- allow(subject.project)
- .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(false)
-
- expect(subject.approvals_required).to eq(expected_required_approvals)
- end
- end
-
context "when the project doesn't require code owner approval on all MRs" do
it 'returns the expected number of approvals for protected_branches that do require approval' do
allow(subject.project)
.to receive(:merge_requests_require_code_owner_approval?).and_return(false)
- allow(subject.project)
- .to receive(:branch_requires_code_owner_approval?).with(branch.name).and_return(feature_enabled)
+ allow(ProtectedBranch)
+ .to receive(:branch_requires_code_owner_approval?).with(subject.project, branch.name).and_return(feature_enabled)
expect(subject.approvals_required).to eq(expected_required_approvals)
end
diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb
index 49b29870d892304de61ca37b31efb1c4b75afe3d..0a9b4262cb287e63ab439a1d052b11d5e4efffaa 100644
--- a/ee/spec/models/project_spec.rb
+++ b/ee/spec/models/project_spec.rb
@@ -45,11 +45,15 @@
context 'scopes' do
describe '.requiring_code_owner_approval' do
+ let!(:project) { create(:project) }
+ let!(:expected_project) { protected_branch_needing_approval.project }
+ let!(:protected_branch_needing_approval) { create(:protected_branch, code_owner_approval_required: true) }
+
it 'only includes the right projects' do
- create(:project)
- expected_project = create(:project, merge_requests_require_code_owner_approval: true)
+ scoped_query_result = described_class.requiring_code_owner_approval
- expect(described_class.requiring_code_owner_approval).to contain_exactly(expected_project)
+ expect(described_class.count).to eq(2)
+ expect(scoped_query_result).to contain_exactly(expected_project)
end
end
@@ -1016,13 +1020,17 @@
true | true | true
false | true | false
true | false | false
- true | nil | false
end
with_them do
before do
stub_licensed_features(code_owner_approval_required: feature_available)
- project.merge_requests_require_code_owner_approval = feature_enabled
+
+ if feature_enabled
+ create(:protected_branch,
+ project: project,
+ code_owner_approval_required: true)
+ end
end
it 'requires code owner approval when needed' do
diff --git a/ee/spec/requests/api/protected_branches_spec.rb b/ee/spec/requests/api/protected_branches_spec.rb
index 9721e5b225cdabb0ad8bc4ad49fd11c716774d3e..ebddcf61b95dc2cb78ef5a06d3da95399e60492e 100644
--- a/ee/spec/requests/api/protected_branches_spec.rb
+++ b/ee/spec/requests/api/protected_branches_spec.rb
@@ -82,6 +82,57 @@
end
end
+ describe "PATCH /projects/:id/protected_branches/:branch" do
+ let(:route) { "/projects/#{project.id}/protected_branches/#{branch_name}" }
+
+ context 'when authenticated as a maintainer' do
+ before do
+ project.add_maintainer(user)
+ end
+
+ context "when the feature is enabled" do
+ before do
+ stub_licensed_features(code_owner_approval_required: true)
+ end
+
+ it "updates the protected branch" do
+ expect do
+ patch api(route, user), params: { code_owner_approval_required: true }
+ end.to change { protected_branch.reload.code_owner_approval_required }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['code_owner_approval_required']).to eq(true)
+ end
+ end
+
+ context "when the feature is disabled" do
+ before do
+ stub_licensed_features(code_owner_approval_required: false)
+ end
+
+ it "does not change the protected branch" do
+ expect do
+ patch api(route, user), params: { code_owner_approval_required: true }
+ end.not_to change { protected_branch.reload.code_owner_approval_required }
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+ end
+
+ context 'when authenticated as a guest' do
+ before do
+ project.add_guest(user)
+ end
+
+ it "returns a 403 response" do
+ patch api(route, user)
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+ end
+
describe 'POST /projects/:id/protected_branches' do
let(:branch_name) { 'new_branch' }
let(:post_endpoint) { api("/projects/#{project.id}/protected_branches", user) }
diff --git a/ee/spec/services/ee/protected_branches/create_service_spec.rb b/ee/spec/services/ee/protected_branches/create_service_spec.rb
index dedcc409284188db7b19bbd929540b7736e70f0d..2d810c536102b1d3cf71500a9bc28866cdfb5a0b 100644
--- a/ee/spec/services/ee/protected_branches/create_service_spec.rb
+++ b/ee/spec/services/ee/protected_branches/create_service_spec.rb
@@ -25,6 +25,41 @@
target_project.add_user(user, :developer)
end
+ context "code_owner_approval_required" do
+ context "when unavailable" do
+ before do
+ stub_licensed_features(code_owner_approval_required: false)
+
+ params[:code_owner_approval_required] = true
+ end
+
+ it "ignores incoming params and sets code_owner_approval_required to false" do
+ expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+ expect(ProtectedBranch.last.code_owner_approval_required).to be_falsy
+ end
+ end
+
+ context "when available" do
+ before do
+ stub_licensed_features(code_owner_approval_required: true)
+ end
+
+ it "sets code_owner_approval_required to true when param is true" do
+ params[:code_owner_approval_required] = true
+
+ expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+ expect(ProtectedBranch.last.code_owner_approval_required).to be_truthy
+ end
+
+ it "sets code_owner_approval_required to false when param is false" do
+ params[:code_owner_approval_required] = false
+
+ expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+ expect(ProtectedBranch.last.code_owner_approval_required).to be_falsy
+ end
+ end
+ end
+
context "when there are open merge requests" do
let!(:merge_request) do
create(:merge_request,
diff --git a/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb b/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb
index 068261d4196c029866326fb7f1f3a79b8c84793f..9f6497e633198c347a5551a33d61e2660db69cec 100644
--- a/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb
+++ b/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb
@@ -18,6 +18,7 @@
allow(MergeRequestApproverPresenter).to receive(:new).and_return(approver_presenter)
assign(:project, project)
assign(:target_project, project)
+ assign(:mr_presenter, merge_request.present(current_user: user))
end
context 'has no approvers' do
diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb
index ca75ee906ce0c4578fe97b4c86aff4783e600ed2..c7665c20234362bda21533280929bdeac63e03ea 100644
--- a/lib/api/protected_branches.rb
+++ b/lib/api/protected_branches.rb
@@ -42,7 +42,7 @@ class ProtectedBranches < Grape::API
end
# rubocop: enable CodeReuse/ActiveRecord
- desc 'Protect a single branch or wildcard' do
+ desc 'Protect a single branch' do
success Entities::ProtectedBranch
end
params do
@@ -93,3 +93,5 @@ class ProtectedBranches < Grape::API
end
end
end
+
+API::ProtectedBranches.prepend_if_ee('EE::API::ProtectedBranches')
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a17d0460967101bae3374ff137f9104fa1bba268..cbde6291514327bacaaec6147bf836d0178eb65f 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -12773,6 +12773,48 @@ msgstr ""
msgid "Protected branches"
msgstr ""
+msgid "ProtectedBranch|%{wildcards_link_start}Wildcards%{wildcards_link_end} such as %{code_tag_start}*-stable%{code_tag_end} or %{code_tag_start}production/*%{code_tag_end} are supported"
+msgstr ""
+
+msgid "ProtectedBranch|Allowed to merge"
+msgstr ""
+
+msgid "ProtectedBranch|Allowed to merge:"
+msgstr ""
+
+msgid "ProtectedBranch|Allowed to push"
+msgstr ""
+
+msgid "ProtectedBranch|Allowed to push:"
+msgstr ""
+
+msgid "ProtectedBranch|Code owner approval"
+msgstr ""
+
+msgid "ProtectedBranch|Last commit"
+msgstr ""
+
+msgid "ProtectedBranch|Protect"
+msgstr ""
+
+msgid "ProtectedBranch|Protect a branch"
+msgstr ""
+
+msgid "ProtectedBranch|Protected branch (%{protected_branches_count})"
+msgstr ""
+
+msgid "ProtectedBranch|Pushes that change filenames matched by the CODEOWNERS file will be rejected"
+msgstr ""
+
+msgid "ProtectedBranch|Require approval from code owners:"
+msgstr ""
+
+msgid "ProtectedBranch|There are currently no protected branches, protect a branch with the form above."
+msgstr ""
+
+msgid "ProtectedBranch|Toggle code owner approval"
+msgstr ""
+
msgid "ProtectedEnvironment|%{environment_name} will be writable for developers. Are you sure?"
msgstr ""
@@ -13420,9 +13462,6 @@ msgstr ""
msgid "Require all users to accept Terms of Service and Privacy Policy when they access GitLab."
msgstr ""
-msgid "Require approval from code owners"
-msgstr ""
-
msgid "Require user password to approve"
msgstr ""
diff --git a/qa/qa/ee.rb b/qa/qa/ee.rb
index a706e7da6dfc0f74e0641833e14d4843b0967be7..f57dc5fcdbc79674655a54b4741bfd18a8ba2601 100644
--- a/qa/qa/ee.rb
+++ b/qa/qa/ee.rb
@@ -115,7 +115,6 @@ module Settings
autoload :MirroringRepositories, 'qa/ee/page/project/settings/mirroring_repositories'
autoload :Main, 'qa/ee/page/project/settings/main'
autoload :MergeRequest, 'qa/ee/page/project/settings/merge_request'
- autoload :MergeRequestApproval, 'qa/ee/page/project/settings/merge_request_approval'
autoload :Repository, 'qa/ee/page/project/settings/repository'
autoload :PushRules, 'qa/ee/page/project/settings/push_rules'
end
diff --git a/qa/qa/ee/page/project/settings/merge_request_approval.rb b/qa/qa/ee/page/project/settings/merge_request_approval.rb
deleted file mode 100644
index 63b42c9bf9d7bec09bd5f18f76c28f92871e1359..0000000000000000000000000000000000000000
--- a/qa/qa/ee/page/project/settings/merge_request_approval.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-# frozen_string_literal: true
-
-module QA
- module EE
- module Page
- module Project
- module Settings
- class MergeRequestApproval < QA::Page::Base
- view 'ee/app/views/projects/_merge_request_approvals_settings_form.html.haml' do
- element :require_code_owners_approval_checkbox
- end
-
- view 'ee/app/views/projects/_merge_request_approvals_settings.html.haml' do
- element :save_merge_request_approval_settings_button
- end
-
- def click_require_code_owners_approval_checkbox
- check_element :require_code_owners_approval_checkbox
- end
-
- def click_save_merge_request_approval_button
- click_element :save_merge_request_approval_settings_button
- end
- end
- end
- end
- end
- end
-end
diff --git a/spec/migrations/migrate_code_owner_approval_status_to_protected_branches_in_batches_spec.rb b/spec/migrations/migrate_code_owner_approval_status_to_protected_branches_in_batches_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..67ac40d4d3909bb7813759927aeeb9594ed6e618
--- /dev/null
+++ b/spec/migrations/migrate_code_owner_approval_status_to_protected_branches_in_batches_spec.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190827102026_migrate_code_owner_approval_status_to_protected_branches_in_batches.rb')
+
+describe MigrateCodeOwnerApprovalStatusToProtectedBranchesInBatches, :migration do
+ let(:namespaces) { table(:namespaces) }
+ let(:projects) { table(:projects) }
+ let(:protected_branches) { table(:protected_branches) }
+
+ let(:namespace) do
+ namespaces.create!(
+ path: 'gitlab-instance-administrators',
+ name: 'GitLab Instance Administrators'
+ )
+ end
+
+ let(:project) do
+ projects.create!(
+ namespace_id: namespace.id,
+ name: 'GitLab Instance Administration'
+ )
+ end
+
+ let!(:protected_branch_1) do
+ protected_branches.create!(
+ name: "branch name",
+ project_id: project.id
+ )
+ end
+
+ describe '#up' do
+ context "when there's no projects needing approval" do
+ it "doesn't change any protected branch records" do
+ expect { migrate! }
+ .not_to change { ProtectedBranch.where(code_owner_approval_required: true).count }
+ end
+ end
+
+ context "when there's a project needing approval" do
+ let!(:project_needing_approval) do
+ projects.create!(
+ namespace_id: namespace.id,
+ name: 'GitLab Instance Administration',
+ merge_requests_require_code_owner_approval: true
+ )
+ end
+
+ let!(:protected_branch_2) do
+ protected_branches.create!(
+ name: "branch name",
+ project_id: project_needing_approval.id
+ )
+ end
+
+ it "changes N protected branch records" do
+ expect { migrate! }
+ .to change { ProtectedBranch.where(code_owner_approval_required: true).count }
+ .by(1)
+ end
+ end
+ end
+end