diff --git a/app/assets/javascripts/users_select/index.js b/app/assets/javascripts/users_select/index.js index 971c99788657194dce3417ed306689c375674026..e693c3e90a4d36dd8cc25be0a367b5de3955cedc 100644 --- a/app/assets/javascripts/users_select/index.js +++ b/app/assets/javascripts/users_select/index.js @@ -49,6 +49,7 @@ function UsersSelect(currentUser, els, options = {}) { options.todoStateFilter = $dropdown.data('todoStateFilter'); options.iid = $dropdown.data('iid'); options.issuableType = $dropdown.data('issuableType'); + options.targetBranch = $dropdown.data('targetBranch'); const showNullUser = $dropdown.data('nullUser'); const defaultNullUser = $dropdown.data('nullUserDefault'); const showMenuAbove = $dropdown.data('showMenuAbove'); @@ -582,7 +583,14 @@ function UsersSelect(currentUser, els, options = {}) { img = ``; } - return userSelect.renderRow(options.issuableType, user, selected, username, img); + return userSelect.renderRow( + options.issuableType, + user, + selected, + username, + img, + elsClassName, + ); }, }); }); @@ -746,8 +754,17 @@ UsersSelect.prototype.users = function(query, options, callback) { ...getAjaxUsersSelectParams(options, AJAX_USERS_SELECT_PARAMS_MAP), }; - if (options.issuableType === 'merge_request') { + const isMergeRequest = options.issuableType === 'merge_request'; + const isEditMergeRequest = !options.issuableType && (options.iid && options.targetBranch); + const isNewMergeRequest = !options.issuableType && (!options.iid && options.targetBranch); + + if (isMergeRequest || isEditMergeRequest || isNewMergeRequest) { params.merge_request_iid = options.iid || null; + params.approval_rules = true; + } + + if (isNewMergeRequest) { + params.target_branch = options.targetBranch || null; } return axios.get(url, { params }).then(({ data }) => { @@ -762,7 +779,14 @@ UsersSelect.prototype.buildUrl = function(url) { return url; }; -UsersSelect.prototype.renderRow = function(issuableType, user, selected, username, img) { +UsersSelect.prototype.renderRow = function( + issuableType, + user, + selected, + username, + img, + elsClassName, +) { const tooltip = issuableType === 'merge_request' && !user.can_merge ? __('Cannot merge') : ''; const tooltipClass = tooltip ? `has-tooltip` : ''; const selectedClass = selected === true ? 'is-active' : ''; @@ -776,10 +800,15 @@ UsersSelect.prototype.renderRow = function(issuableType, user, selected, usernam ${this.renderRowAvatar(issuableType, user, img)} - + ${escape(user.name)} - ${username ? `${username}` : ''} + ${ + username + ? `${username}` + : '' + } + ${this.renderApprovalRules(elsClassName, user.applicable_approval_rules)} @@ -802,4 +831,22 @@ UsersSelect.prototype.renderRowAvatar = function(issuableType, user, img) { `; }; +UsersSelect.prototype.renderApprovalRules = function(elsClassName, approvalRules = []) { + if (!gon.features?.reviewerApprovalRules || !elsClassName?.includes('reviewer')) { + return ''; + } + + const count = approvalRules.length; + const [rule] = approvalRules; + const countText = sprintf(__('(+%{count} rules)'), { count }); + const renderApprovalRulesCount = count > 1 ? `${countText}` : ''; + + return count + ? `
+ ${rule.name} + ${renderApprovalRulesCount} +
` + : ''; +}; + export default UsersSelect; diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 453928e251f424e0085e589b36b8b14083bf3a63..14fd26f1c4372b0fda1646b41c4f855570c58444 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -14,6 +14,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap before_action do push_frontend_feature_flag(:merge_request_reviewers, @project) push_frontend_feature_flag(:mr_collapsed_approval_rules, @project) + push_frontend_feature_flag(:reviewer_approval_rules, @project) end def new diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index ee25a2006ad810c0d9271b7b774dc52471f62f20..6d0dbc655852af3b6d3bbdc2a33fed675921e103 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -53,6 +53,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo 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) + push_frontend_feature_flag(:reviewer_approval_rules, @project) end around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index 8a8d708b0b24dc9f55d2a65a44aa5cdb0a6cbcde..d0276c91316af37e18bce8deff5460909c6a81cc 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -55,7 +55,7 @@ def assignees_dropdown_options(issuable_type) dropdown_data end - def reviewers_dropdown_options(issuable_type) + def reviewers_dropdown_options(issuable_type, iid = nil, target_branch = nil) dropdown_data = { toggle_class: 'js-reviewer-search js-multiselect js-save-user-data', title: 'Request review from', @@ -78,6 +78,14 @@ def reviewers_dropdown_options(issuable_type) } } + if iid + dropdown_data[:data][:iid] = iid + end + + if target_branch + dropdown_data[:data][:target_branch] = target_branch + end + if merge_request_supports_multiple_reviewers? dropdown_data = multiple_reviewers_dropdown_options(dropdown_data) end 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 a4e2ce035cc79aa5b824ded66edb6cbd5aaf1093..a0df007f8ca07cbadbd3a57a4314d10eb20b91f6 100644 --- a/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml +++ b/app/views/shared/issuable/form/_metadata_issuable_reviewer.html.haml @@ -7,6 +7,6 @@ - if issuable.reviewers.empty? = 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)) + = dropdown_tag(users_dropdown_label(issuable.reviewers), options: reviewers_dropdown_options(issuable.to_ability_name, issuable.iid, issuable.target_branch)) - 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/reviewer_approval_rules.yml b/config/feature_flags/development/reviewer_approval_rules.yml new file mode 100644 index 0000000000000000000000000000000000000000..97181ef2a36467b8d18174125bb69fe7566e84ca --- /dev/null +++ b/config/feature_flags/development/reviewer_approval_rules.yml @@ -0,0 +1,8 @@ +--- +name: reviewer_approval_rules +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46738 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/293742 +milestone: '13.7' +type: development +group: group::code review +default_enabled: false diff --git a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb index 2ebb76b01a29d962d13cf5e36bc41ee6a4c37af7..53f001034b2e44a796fcf6d29188dc32d7467f8f 100644 --- a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb +++ b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb @@ -10,4 +10,45 @@ end it_behaves_like 'multiple reviewers merge request', 'updates', 'Save changes' + + context 'user approval rules', :js do + let(:rule_name) { 'some-custom-rule' } + let!(:mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: [user], name: rule_name, approvals_required: 1 )} + + it 'is not shown in assignee dropdown' do + find('.js-assignee-search').click + wait_for_requests + + page.within '.dropdown-menu-assignee' do + expect(page).not_to have_content(rule_name) + end + end + + it 'is shown in reviewer dropdown' do + find('.js-reviewer-search').click + wait_for_requests + + page.within '.dropdown-menu-reviewer' do + expect(page).to have_content(rule_name) + end + end + end + + context 'when reviewer_approval_rules feature flag off' do + let(:rule_name) { 'some-custom-rule' } + let!(:mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: [user], name: rule_name, approvals_required: 1 )} + + before do + stub_feature_flags(reviewer_approval_rules: false) + end + + it 'is not shown in reviewer dropdown' do + find('.js-reviewer-search').click + wait_for_requests + + page.within '.dropdown-menu-reviewer' do + expect(page).not_to have_content(rule_name) + end + end + end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f0336bb3af631dcf7fd38d85c0726b2fc1050b96..50cb411f38e5122f8ef31a4a1ca24103098ffd89 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -947,6 +947,9 @@ msgstr "" msgid "(%{value}) has already been taken" msgstr "" +msgid "(+%{count} rules)" +msgstr "" + msgid "(No changes)" msgstr ""