Merge branch 'retry-approvers-dropdown-use-new-endpoint' into 'master'

Show possible groups approvers [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60339

app/controllers/projects/merge_requests_controller.rb

@@ -59,6 +59,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
   before_action do
     push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
+    push_frontend_feature_flag(:show_relevant_approval_rule_approvers, @project, default_enabled: :yaml)
   end
 
   around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]

config/feature_flags/development/show_relevant_approval_rule_approvers.yml

+---
+name: show_relevant_approval_rule_approvers
+introduced_by_url:
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329153
+milestone: '13.12'
+type: development
+group: group::source code
+default_enabled: false

ee/app/assets/javascripts/api.js

@@ -47,6 +47,7 @@ export default {
   descendantGroupsPath: '/api/:version/groups/:group_id/descendant_groups',
   projectDeploymentFrequencyAnalyticsPath:
     '/api/:version/projects/:id/analytics/deployment_frequency',
+  projectGroupsPath: '/api/:version/projects/:id/groups.json',
   issueMetricImagesPath: '/api/:version/projects/:id/issues/:issue_iid/metric_images',
   issueMetricSingleImagePath:
     '/api/:version/projects/:id/issues/:issue_iid/metric_images/:image_id',
@@ -440,4 +441,18 @@ export default {
 
     return axios.get(url);
   },
+
+  projectGroups(id, options) {
+    const url = Api.buildUrl(this.projectGroupsPath).replace(':id', encodeURIComponent(id));
+
+    return axios
+      .get(url, {
+        params: {
+          ...options,
+        },
+      })
+      .then(({ data }) => {
+        return data;
+      });
+  },
 };

ee/app/assets/javascripts/approvals/components/approvers_select.vue

@@ -80,6 +80,11 @@ export default {
       default: false,
     },
   },
+  computed: {
+    isFeatureEnabled() {
+      return Boolean(gon.features?.showRelevantApprovalRuleApprovers);
+    },
+  },
   watch: {
     value(val) {
       if (val.length > 0) {
@@ -135,6 +140,18 @@ export default {
         .then((results) => ({ results }));
     },
     fetchGroups(term) {
+      if (this.isFeatureEnabled) {
+        const hasTerm = term.trim().length > 0;
+        const DEVELOPER_ACCESS_LEVEL = 30;
+
+        return Api.projectGroups(this.projectId, {
+          skip_groups: this.skipGroupIds,
+          ...(hasTerm ? { search: term } : {}),
+          with_shared: true,
+          shared_min_access_level: DEVELOPER_ACCESS_LEVEL,
+        });
+      }
+
       // Don't includeAll when search is empty. Otherwise, the user could get a lot of garbage choices.
       // https://gitlab.com/gitlab-org/gitlab/issues/11566
       const includeAll = term.trim().length > 0;

ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb

@@ -74,11 +74,41 @@ RSpec.describe 'Merge request > User edits MR with approval rules', :js do
     expect(page_rule_names.last).to have_text(rule_name)
   end
 
-  context "with public group" do
-    let_it_be(:group) { create(:group, :public) }
+  context 'with show_relevant_approval_rule_approvers feature flag disabled' do
+    before do
+      stub_feature_flags(show_relevant_approval_rule_approvers: false)
+    end
+
+    context "with public group" do
+      let(:group) { create(:group, :public) }
+
+      before do
+        group.add_developer create(:user)
+
+        click_button 'Approval rules'
+        click_button "Add approval rule"
+      end
+
+      it "with empty search, does not show public group" do
+        open_select2 members_selector
+        wait_for_requests
+
+        expect(page).not_to have_selector('.select2-result-label .group-result', text: group.name)
+      end
+    end
+  end
+
+  context 'with public group' do
+    let(:group) { create(:group, :public) }
 
     before do
-      group.add_developer create(:user)
+      group_project = create(:project, :public, :repository, namespace: group)
+      group_project_merge_request = create(:merge_request, source_project: group_project)
+      group.add_developer(author)
+
+      visit(edit_project_merge_request_path(group_project, group_project_merge_request))
+
+      wait_for_requests
 
       click_button 'Approval rules'
       click_button "Add approval rule"

ee/spec/features/merge_request/user_sets_approvers_spec.rb

@@ -154,21 +154,54 @@ RSpec.describe 'Merge request > User sets approvers', :js do
         sign_in(user)
       end
 
-      it 'allows setting groups as approvers' do
+      context 'with show_relevant_approval_rule_approvers feature flag disabled' do
+        before do
+          stub_feature_flags(show_relevant_approval_rule_approvers: false)
+        end
+
+        it 'allows setting groups as approvers' do
+          group = create :group
+          group.add_developer(other_user)
+
+          visit edit_project_merge_request_path(project, merge_request)
+
+          open_modal(text: 'Add approval rule')
+          open_approver_select
+
+          expect(find('.select2-results')).not_to have_content(group.name)
+
+          close_approver_select
+          group.add_developer(user) # only display groups that user has access to
+          open_approver_select
+
+          expect(find('.select2-results')).to have_content(group.name)
+
+          find('.select2-results .user-result', text: group.name).click
+          close_approver_select
+          within('.modal-content') do
+            click_button 'Add approval rule'
+          end
+
+          click_on("Save changes")
+          wait_for_all_requests
+
+          expect(page).to have_content("Requires approval.")
+          expect(page).to have_selector("img[alt='#{other_user.name}']")
+        end
+      end
+
+      it 'allows setting groups as approvers when there is possible group approvers' do
         group = create :group
+        group_project = create(:project, :public, :repository, namespace: group)
+        group_project_merge_request = create(:merge_request, source_project: group_project)
+        group.add_developer(user)
         group.add_developer(other_user)
 
-        visit edit_project_merge_request_path(project, merge_request)
+        visit edit_project_merge_request_path(group_project, group_project_merge_request)
 
         open_modal(text: 'Add approval rule')
         open_approver_select
 
-        expect(find('.select2-results')).not_to have_content(group.name)
-
-        close_approver_select
-        group.add_developer(user) # only display groups that user has access to
-        open_approver_select
-
         expect(find('.select2-results')).to have_content(group.name)
 
         find('.select2-results .user-result', text: group.name).click
@@ -181,6 +214,7 @@ RSpec.describe 'Merge request > User sets approvers', :js do
         wait_for_all_requests
 
         expect(page).to have_content("Requires approval.")
+        expect(page).to have_selector("img[alt='#{user.name}']")
         expect(page).to have_selector("img[alt='#{other_user.name}']")
       end