diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 3e077c1af3772c33d642fd098bc0e57f240bc68e..453928e251f424e0085e589b36b8b14083bf3a63 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -11,6 +11,11 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path]
before_action :build_merge_request, except: [:create]
+ before_action do
+ push_frontend_feature_flag(:merge_request_reviewers, @project)
+ push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
+ end
+
def new
define_new_vars
end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 627d578643f31e0a1bf3476f46b8e771ecc3611f..c2bb441d640e1882ae368fb4cdbfe9e81fba5c9f 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -49,6 +49,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action do
push_frontend_feature_flag(:vue_issuable_sidebar, @project.group)
+ push_frontend_feature_flag(:merge_request_reviewers, @project)
+ push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index c0aba0eef7f98aa254d5a18f5b13729d53bd2857..16d9d7dae5a7e4f9accd08d3193bb7f71e5ea9dc 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -32,7 +32,7 @@
= form.label :confidential, class: 'form-check-label' do
This issue is confidential and should only be visible to team members with at least Reporter access.
-= render 'shared/issuable/form/metadata', issuable: issuable, form: form, project: project
+= render 'shared/issuable/form/metadata', issuable: issuable, form: form, project: project, presenter: presenter
= render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
diff --git a/app/views/shared/issuable/form/_metadata.html.haml b/app/views/shared/issuable/form/_metadata.html.haml
index 459eb112e4fd6b8befbad632883495e44179ca84..366e819d252c4cef524ed7f53112f360eb1678a7 100644
--- a/app/views/shared/issuable/form/_metadata.html.haml
+++ b/app/views/shared/issuable/form/_metadata.html.haml
@@ -1,5 +1,6 @@
- project = local_assigns.fetch(:project)
- issuable = local_assigns.fetch(:issuable)
+- presenter = local_assigns.fetch(:presenter)
- return unless can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
@@ -14,7 +15,7 @@
- if issuable.allows_reviewers?
.form-group.row.merge-request-reviewer
- = render "shared/issuable/form/metadata_issuable_reviewer", issuable: issuable, form: form, has_due_date: has_due_date
+ = render "shared/issuable/form/metadata_issuable_reviewer", issuable: issuable, form: form, has_due_date: has_due_date, presenter: presenter
= render_if_exists "shared/issuable/form/epic", issuable: issuable, form: form, project: project
diff --git a/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml b/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml
index 5ff3a6781ad823d29611c0581f7f861c45f2d2ba..a4e2ce035cc79aa5b824ded66edb6cbd5aaf1093 100644
--- a/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml
+++ b/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml
@@ -1,5 +1,5 @@
= form.label :reviewer_id, issuable.allows_multiple_reviewers? ? _('Reviewers') : _('Reviewer'), class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}"
-.col-sm-10{ class: ("col-md-8" if has_due_date) }
+.col-sm-10.gl-mb-2{ class: ("col-md-8" if has_due_date) }
.issuable-form-select-holder.selectbox
- issuable.reviewers.each do |reviewer|
= hidden_field_tag "#{issuable.to_ability_name}[reviewer_ids][]", reviewer.id, id: nil, data: { meta: reviewer.name, avatar_url: reviewer.avatar_url, name: reviewer.name, username: reviewer.username }
@@ -8,3 +8,5 @@
= hidden_field_tag "#{issuable.to_ability_name}[reviewer_ids][]", 0, id: nil, data: { meta: '' }
= dropdown_tag(users_dropdown_label(issuable.reviewers), options: reviewers_dropdown_options(issuable.to_ability_name))
+ - if Feature.enabled?(:mr_collapsed_approval_rules, @project)
+ = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter
diff --git a/config/feature_flags/development/mr_collapsed_approval_rules.yml b/config/feature_flags/development/mr_collapsed_approval_rules.yml
new file mode 100644
index 0000000000000000000000000000000000000000..5fca48ddc3ab869a9762dcd040791a1de81b51a4
--- /dev/null
+++ b/config/feature_flags/development/mr_collapsed_approval_rules.yml
@@ -0,0 +1,8 @@
+---
+name: mr_collapsed_approval_rules
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47475
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284052
+milestone: '13.6'
+type: development
+group: group::source code
+default_enabled: false
diff --git a/ee/app/assets/javascripts/approvals/components/mr_edit/app.vue b/ee/app/assets/javascripts/approvals/components/mr_edit/app.vue
index 68c9db7d3c4c57df76210b494fa73e4abd3fe301..dd5fff61b0f95e48d210527e306ae98187482f84 100644
--- a/ee/app/assets/javascripts/approvals/components/mr_edit/app.vue
+++ b/ee/app/assets/javascripts/approvals/components/mr_edit/app.vue
@@ -1,19 +1,58 @@
-
+
+
+
+ {{ s__('ApprovalRule|Approval rules') }}
+
+
+
+
+
+
+
+
+
+
diff --git a/ee/app/views/shared/issuable/_approvals.html.haml b/ee/app/views/shared/issuable/_approvals.html.haml
index 40b8ed64561d7247a364c382b33ffecb7639f5ff..127f50848ab8f4425188d04869d4d8bf170f227f 100644
--- a/ee/app/views/shared/issuable/_approvals.html.haml
+++ b/ee/app/views/shared/issuable/_approvals.html.haml
@@ -3,8 +3,9 @@
- return unless issuable.is_a?(MergeRequest)
- return unless issuable.approval_feature_available?
-.form-group.row
- .col-sm-2.col-form-label
- = form.label :approver_ids, "Approval rules"
- .col-sm-10
- = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter
+- if !Feature.enabled?(:merge_request_reviewers, @project) || !Feature.enabled?(:mr_collapsed_approval_rules, @project)
+ .form-group.row
+ .col-sm-2.col-form-label
+ = form.label :approver_ids, "Approval rules"
+ .col-sm-10
+ = render_if_exists 'shared/issuable/approver_suggestion', issuable: issuable, presenter: presenter
diff --git a/ee/spec/features/merge_request/user_creates_merge_request_spec.rb b/ee/spec/features/merge_request/user_creates_merge_request_spec.rb
index c11aab1c45e0adff7e32318b1f895c279076bb5b..299afcbae9de1382a3018b061ca47fabe373077f 100644
--- a/ee/spec/features/merge_request/user_creates_merge_request_spec.rb
+++ b/ee/spec/features/merge_request/user_creates_merge_request_spec.rb
@@ -39,6 +39,8 @@
expect(find_field("merge_request_description").value).to eq(template_text)
+ click_button 'Approval rules'
+
page.within('.js-approval-rules') do
expect(page).to have_css("img[alt=\"#{approver.name}\"]")
end
diff --git a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
index 65520a8a65b2d129173b818a2d881b9954fc2fdc..6fcb94e22e05bf053945f27e665861c59f5fd70a 100644
--- a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
+++ b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
@@ -41,12 +41,16 @@ def add_approval_rule_member(type, name)
end
it "shows approval rules" do
+ click_button 'Approval rules'
+
names = page_rule_names.map(&:text)
expect(names).to eq(mr_rule_names)
end
it "allows user to create approval rule" do
+ click_button 'Approval rules'
+
rule_name = "Custom Approval Rule"
click_button "Add approval rule"
@@ -67,6 +71,7 @@ def add_approval_rule_member(type, name)
before do
group.add_developer create(:user)
+ click_button 'Approval rules'
click_button "Add approval rule"
end
diff --git a/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb b/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb
index 34c7d1c9948cdaa150eb85b7a7b6faacebcb261c..a95ae19d49466fa5d21fc09072f82d5f71fa6a70 100644
--- a/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb
+++ b/ee/spec/features/merge_request/user_sets_approval_rules_spec.rb
@@ -25,6 +25,8 @@ def page_rule_names
end
it "shows approval rules from target project", :sidekiq_might_not_need_inline do
+ click_button 'Approval rules'
+
names = page_rule_names
regular_rules.each_with_index do |rule, idx|
expect(names[idx]).to have_text(rule.name)
@@ -46,6 +48,8 @@ def page_rule_names
end
it "shows approval rules" do
+ click_button 'Approval rules'
+
names = page_rule_names
rules.each.with_index do |rule, idx|
expect(names[idx]).to have_text(rule.name)
diff --git a/ee/spec/features/merge_request/user_sets_approvers_spec.rb b/ee/spec/features/merge_request/user_sets_approvers_spec.rb
index 12df752bef31a96337c8c56171e723309ea0780f..5470a0abb0aeb1769382783a60052bacba51aa5d 100644
--- a/ee/spec/features/merge_request/user_sets_approvers_spec.rb
+++ b/ee/spec/features/merge_request/user_sets_approvers_spec.rb
@@ -11,6 +11,36 @@
let!(:config_selector) { '.js-approval-rules' }
let!(:modal_selector) { '#mr-edit-approvals-create-modal' }
+ context 'with feature flag off' do
+ let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
+ def visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: false)
+ stub_feature_flags(merge_request_reviewers: merge_request_reviewers, mr_collapsed_approval_rules: mr_collapsed_approval_rules)
+ project.add_developer(user)
+ sign_in(user)
+ visit edit_project_merge_request_path(project, merge_request)
+ end
+
+ def non_collapse_approval_rules
+ expect(page).to have_button('Add approval rule')
+ end
+
+ it 'does not hide approval rules inside collapse when only merge_request_reviewers is off' do
+ visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: true)
+ non_collapse_approval_rules
+ end
+
+ it 'does not hide approval rules inside collapse when mr_collapsed_approval_rules is off' do
+ visit_mr(merge_request_reviewers: true, mr_collapsed_approval_rules: false)
+ non_collapse_approval_rules
+ end
+
+ it 'does not hide approval rules inside collapse when merge_request_reviewers and mr_collapsed_approval_rules are off' do
+ visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: false)
+ non_collapse_approval_rules
+ end
+ end
+
context 'when editing an MR with a different author' do
let(:author) { create(:user) }
let(:merge_request) { create(:merge_request, author: author, source_project: project) }
diff --git a/ee/spec/features/merge_requests/user_resets_approvers_spec.rb b/ee/spec/features/merge_requests/user_resets_approvers_spec.rb
index f787d9fc3b5c618ae35c0ddc610d8ab959ca3204..15e872f3a233b50419bddd5ae1eb0cdaea10b250 100644
--- a/ee/spec/features/merge_requests/user_resets_approvers_spec.rb
+++ b/ee/spec/features/merge_requests/user_resets_approvers_spec.rb
@@ -32,6 +32,8 @@
end
it 'resets approvers for merge requests' do
+ click_button 'Approval rules'
+
expect_avatar(find('.js-members'), first_user)
click_button 'Reset to project defaults'
diff --git a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
index 701edce2240b38e6f2535d151383cc5656b1cf5f..4efb445239f831fb1414bd9ded3753270f38828d 100644
--- a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
+++ b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
@@ -23,7 +23,7 @@
it 'adds approver' do
visit edit_project_path(project)
- open_modal(text: 'Add approval rule')
+ open_modal(text: 'Add approval rule', expand: false)
open_approver_select
expect(find('.select2-results')).to have_content(user.name)
@@ -50,7 +50,7 @@
it 'adds approver group' do
visit edit_project_path(project)
- open_modal(text: 'Add approval rule')
+ open_modal(text: 'Add approval rule', expand: false)
open_approver_select
expect(find('.select2-results')).to have_content(group.name)
@@ -81,7 +81,7 @@
expect_avatar(find('.js-members'), rule.approvers)
- open_modal
+ open_modal(text: 'Edit', expand: false)
remove_approver(group.name)
click_button "Update approval rule"
wait_for_requests
diff --git a/ee/spec/support/helpers/feature_approval_helper.rb b/ee/spec/support/helpers/feature_approval_helper.rb
index 1cc1830cd90dedfa676ca465ad84256b3d437e12..ae8f24be496e21688b304398e9069261d8d31653 100644
--- a/ee/spec/support/helpers/feature_approval_helper.rb
+++ b/ee/spec/support/helpers/feature_approval_helper.rb
@@ -1,8 +1,13 @@
# frozen_string_literal: true
module FeatureApprovalHelper
- def open_modal(text: 'Edit')
+ def open_modal(text: 'Edit', expand: true)
page.execute_script "document.querySelector('#{config_selector}').scrollIntoView()"
+
+ if expand
+ click_button 'Approval rules'
+ end
+
within(config_selector) do
click_on(text)
end
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 7c7eb619838401aedcbde6fa41f405b0c8140d0a..73558401720bb3ef74a0d74aab454bc1864f8891 100644
--- a/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb
+++ b/ee/spec/views/shared/issuable/_approvals.html.haml_spec.rb
@@ -22,8 +22,9 @@
end
context 'has no approvers' do
- context 'can override approvers' do
+ context 'when mr_collapsed_approval_rules feature flag is off' do
before do
+ stub_feature_flags(mr_collapsed_approval_rules: false)
render 'shared/issuable/approvals', form: form, issuable: merge_request, presenter: presenter
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d93d2dbb8a31babb2a4cc705d02d16ae36ead8da..7284bc9d8b4047e6f2d349eeb84ce1eeea1894e4 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3511,6 +3511,9 @@ msgid_plural "ApprovalRuleSummary|%{count} approvals required from %{membersCoun
msgstr[0] ""
msgstr[1] ""
+msgid "ApprovalRule|Approval rules"
+msgstr ""
+
msgid "ApprovalRule|Approvers"
msgstr ""