Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/blob/suggest_gitlab_ci_yml/components/popover.vue

@@ -108,7 +108,7 @@ export default {
     show
     :target="target"
     placement="right"
-    trigger="manual"
+    triggers="manual"
     container="viewport"
     :css-classes="['suggest-gitlab-ci-yml', 'ml-4']"
   >

app/assets/javascripts/boards/components/board_add_new_column.vue

@@ -5,7 +5,6 @@ import BoardAddNewColumnForm from '~/boards/components/board_add_new_column_form
 import { ListType } from '~/boards/constants';
 import boardsStore from '~/boards/stores/boards_store';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
-import { isScopedLabel } from '~/lib/utils/common_utils';
 
 export default {
   components: {
@@ -20,17 +19,12 @@ export default {
   data() {
     return {
       selectedId: null,
+      selectedLabel: null,
     };
   },
   computed: {
     ...mapState(['labels', 'labelsLoading']),
     ...mapGetters(['getListByLabelId', 'shouldUseGraphQL']),
-    selectedLabel() {
-      if (!this.selectedId) {
-        return null;
-      }
-      return this.labels.find(({ id }) => id === this.selectedId);
-    },
     columnForSelected() {
       return this.getListByLabelId(this.selectedId);
     },
@@ -83,8 +77,13 @@ export default {
       this.fetchLabels(searchTerm);
     },
 
-    showScopedLabels(label) {
-      return this.scopedLabelsAvailable && isScopedLabel(label);
+    setSelectedItem(selectedId) {
+      const label = this.labels.find(({ id }) => id === selectedId);
+      if (!selectedId || !label) {
+        this.selectedLabel = null;
+      } else {
+        this.selectedLabel = { ...label };
+      }
     },
   },
 };
@@ -116,6 +115,7 @@ export default {
         v-if="labels.length > 0"
         v-model="selectedId"
         class="gl-overflow-y-auto gl-px-5 gl-pt-3"
+        @change="setSelectedItem"
       >
         <label
           v-for="label in labels"

app/finders/group_members_finder.rb

@@ -21,28 +21,13 @@ class GroupMembersFinder < UnionFinder
   end
 
   def execute(include_relations: DEFAULT_RELATIONS)
-    group_members = group_members_list
-    relations = []
+    return filter_members(group_members_list) if include_relations == [:direct]
 
-    return filter_members(group_members) if include_relations == [:direct]
+    groups = groups_by_relations(include_relations)
+    return GroupMember.none unless groups
 
-    relations << group_members if include_relations.include?(:direct)
+    members = all_group_members(groups).distinct_on_user_with_max_access_level
 
-    if include_relations.include?(:inherited) && group.parent
-      parents_members = relation_group_members(group.ancestors)
-
-      relations << parents_members
-    end
-
-    if include_relations.include?(:descendants)
-      descendant_members = relation_group_members(group.descendants)
-
-      relations << descendant_members
-    end
-
-    return GroupMember.none if relations.empty?
-
-    members = find_union(relations, GroupMember)
     filter_members(members)
   end
 
@@ -50,6 +35,25 @@ class GroupMembersFinder < UnionFinder
 
   attr_reader :user, :group
 
+  def groups_by_relations(include_relations)
+    case include_relations.sort
+    when [:inherited]
+      group.ancestors
+    when [:descendants]
+      group.descendants
+    when [:direct, :inherited]
+      group.self_and_ancestors
+    when [:descendants, :direct]
+      group.self_and_descendants
+    when [:descendants, :inherited]
+      find_union([group.ancestors, group.descendants], Group)
+    when [:descendants, :direct, :inherited]
+      group.self_and_hierarchy
+    else
+      nil
+    end
+  end
+
   def filter_members(members)
     members = members.search(params[:search]) if params[:search].present?
     members = members.sort_by_attribute(params[:sort]) if params[:sort].present?
@@ -69,17 +73,13 @@ class GroupMembersFinder < UnionFinder
     group.members
   end
 
-  def relation_group_members(relation)
-    all_group_members(relation).non_minimal_access
+  def all_group_members(groups)
+    members_of_groups(groups).non_minimal_access
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
-  def all_group_members(relation)
-    GroupMember.non_request
-      .where(source_id: relation.select(:id))
-      .where.not(user_id: group.users.select(:id))
+  def members_of_groups(groups)
+    GroupMember.non_request.of_groups(groups)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end
 
 GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder')

app/models/member.rb

@@ -137,6 +137,12 @@ class Member < ApplicationRecord
   scope :with_source_id, ->(source_id) { where(source_id: source_id) }
   scope :including_source, -> { includes(:source) }
 
+  scope :distinct_on_user_with_max_access_level, -> do
+    distinct_members = select('DISTINCT ON (user_id, invite_email) *')
+                       .order('user_id, invite_email, access_level DESC, expires_at DESC, created_at ASC')
+    Member.from(distinct_members, :members)
+  end
+
   scope :order_name_asc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'ASC')) }
   scope :order_name_desc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'DESC')) }
   scope :order_recent_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'DESC')) }

changelogs/unreleased/fix-group_members_max_access_level.yml

+---
+title: Fix derivation of effective permissions (access level) of group members
+merge_request: 56677
+author: Jonas Wälter @wwwjon
+type: fixed

config/feature_flags/development/ci_needs_optional.yml

@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323891
 milestone: '13.10'
 type: development
 group: group::pipeline authoring
-default_enabled: false
+default_enabled: true

doc/api/members.md

@@ -90,8 +90,8 @@ Example response:
 
 Gets a list of group or project members viewable by the authenticated user, including inherited members and permissions through ancestor groups.
 
-WARNING:
-Due to [an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/249523), the users effective `access_level` may actually be higher than returned value when listing group members.
+If a user is a member of this group or project and also of one or more ancestor groups, only its membership with the highest `access_level` is returned.
+This represents the effective permission of the user.
 
 This function takes pagination parameters `page` and `per_page` to restrict the list of users.
 

doc/ci/yaml/README.md

@@ -2149,10 +2149,11 @@ To download artifacts from a job in the current pipeline, use the basic form of
 #### Optional `needs`
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/30680) in GitLab 13.10.
-> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
-> - It's disabled on GitLab.com.
-> - It's not recommended for production use.
-> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-optional-needs). **(FREE SELF)**
+> - [Deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
+> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/323891) in GitLab 13.11.
+> - Enabled on GitLab.com.
+> - Recommended for production use.
+> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-optional-needs). **(FREE SELF)**
 
 WARNING:
 This feature might not be available to you. Check the **version history** note above for details.
@@ -2191,10 +2192,10 @@ rspec:
 
 #### Enable or disable optional needs **(FREE SELF)**
 
-Optional needs is under development and not ready for production use. It is
-deployed behind a feature flag that is **disabled by default**.
+Optional needs is under development but ready for production use.
+It is deployed behind a feature flag that is **enabled by default**.
 [GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
-can enable it.
+can opt to disable it.
 
 To enable it:
 

ee/app/assets/javascripts/boards/components/board_add_new_column.vue

@@ -72,6 +72,7 @@ export default {
   data() {
     return {
       selectedId: null,
+      selectedItem: null,
       columnType: ListType.label,
     };
   },
@@ -113,10 +114,6 @@ export default {
       return this.columnType === ListType.iteration;
     },
 
-    selectedItem() {
-      return this.items.find(({ id }) => id === this.selectedId);
-    },
-
     hasLabelSelection() {
       return this.labelTypeSelected && this.selectedItem;
     },
@@ -262,11 +259,17 @@ export default {
     setColumnType(type) {
       this.columnType = type;
       this.selectedId = null;
+      this.setSelectedItem(null);
       this.filterItems();
     },
 
-    hideDropdown() {
-      this.$root.$emit('bv::dropdown::hide');
+    setSelectedItem(selectedId) {
+      const item = this.items.find(({ id }) => id === selectedId);
+      if (!selectedId || !item) {
+        this.selectedItem = null;
+      } else {
+        this.selectedItem = { ...item };
+      }
     },
   },
 };
@@ -337,7 +340,8 @@ export default {
       <gl-form-radio-group
         v-model="selectedId"
         class="gl-overflow-y-auto gl-px-5"
-        @change="hideDropdown"
+        data-testid="selectItem"
+        @change="setSelectedItem"
       >
         <label
           v-for="item in items"

ee/app/assets/javascripts/security_dashboard/components/csv_export_button.vue

 <script>
 import { GlPopover, GlIcon, GlLink, GlButton, GlTooltipDirective } from '@gitlab/ui';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
+import AccessorUtils from '~/lib/utils/accessor';
 import axios from '~/lib/utils/axios_utils';
 import { formatDate } from '~/lib/utils/datetime_utility';
 import download from '~/lib/utils/downloader';
@@ -19,12 +20,7 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
-  props: {
-    vulnerabilitiesExportEndpoint: {
-      type: String,
-      required: true,
-    },
-  },
+  inject: ['vulnerabilitiesExportEndpoint'],
   data() {
     return {
       isPreparingCsvExport: false,
@@ -45,10 +41,8 @@ export default {
     closePopover() {
       this.showPopover = false;
 
-      try {
+      if (AccessorUtils.isLocalStorageAccessSafe()) {
         localStorage.setItem(STORAGE_KEY, 'true');
-      } catch (e) {
-        // Ignore the error - this is just a safety measure.
       }
     },
     initiateCsvExport() {
@@ -87,9 +81,10 @@ export default {
   >
     {{ __('Export') }}
     <gl-popover
+      v-if="showPopover"
       ref="popover"
       :target="() => $refs.csvExportButton.$el"
-      :show="showPopover"
+      show
       placement="left"
       triggers="manual"
     >

ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard.vue

@@ -20,12 +20,6 @@ export default {
     VulnerabilitiesCountList,
   },
   inject: ['groupFullPath'],
-  props: {
-    vulnerabilitiesExportEndpoint: {
-      type: String,
-      required: true,
-    },
-  },
   apollo: {
     projects: {
       query: vulnerableProjectsQuery,
@@ -74,7 +68,7 @@ export default {
           <h2 class="gl-flex-grow-1 gl-my-0">
             {{ s__('SecurityReports|Vulnerability Report') }}
           </h2>
-          <csv-export-button :vulnerabilities-export-endpoint="vulnerabilitiesExportEndpoint" />
+          <csv-export-button />
         </header>
         <vulnerabilities-count-list
           :scope="$options.vulnerabilitiesSeverityCountScopes.group"

ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard.vue

@@ -19,12 +19,6 @@ export default {
     DashboardNotConfigured,
     VulnerabilitiesCountList,
   },
-  props: {
-    vulnerabilitiesExportEndpoint: {
-      type: String,
-      required: true,
-    },
-  },
   apollo: {
     projects: {
       query: projectsQuery,
@@ -75,7 +69,7 @@ export default {
           <h2 class="gl-flex-grow-1 gl-my-0">
             {{ s__('SecurityReports|Vulnerability Report') }}
           </h2>
-          <csv-export-button :vulnerabilities-export-endpoint="vulnerabilitiesExportEndpoint" />
+          <csv-export-button />
         </header>
         <vulnerabilities-count-list
           :scope="$options.vulnerabilitiesSeverityCountScopes.instance"

ee/app/assets/javascripts/security_dashboard/components/first_class_project_security_dashboard.vue

@@ -34,11 +34,6 @@ export default {
       required: false,
       default: () => ({}),
     },
-    vulnerabilitiesExportEndpoint: {
-      type: String,
-      required: false,
-      default: '',
-    },
   },
   data() {
     const shouldShowAutoFixUserCallout =
@@ -73,7 +68,7 @@ export default {
         <template #header>
           <div class="gl-mt-6 gl-display-flex">
             <h4 class="gl-flex-grow-1 gl-my-0">{{ __('Vulnerability Report') }}</h4>
-            <csv-export-button :vulnerabilities-export-endpoint="vulnerabilitiesExportEndpoint" />
+            <csv-export-button />
           </div>
           <project-pipeline-status :pipeline="pipeline" />
           <vulnerabilities-count-list

ee/app/assets/javascripts/security_dashboard/vulnerability_report_init.js

@@ -65,6 +65,7 @@ export default (el, dashboardType) => {
     groupFullPath,
     securityConfigurationPath,
     surveyRequestSvgPath,
+    vulnerabilitiesExportEndpoint,
     hasVulnerabilities: parseBoolean(hasVulnerabilities),
     scanners: scanners ? JSON.parse(scanners) : [],
     hasJiraVulnerabilitiesIntegrationEnabled: parseBoolean(
@@ -76,7 +77,6 @@ export default (el, dashboardType) => {
     securityDashboardHelpPath,
     projectAddEndpoint,
     projectListEndpoint,
-    vulnerabilitiesExportEndpoint,
   };
 
   let component;

ee/app/assets/javascripts/vue_shared/security_reports/components/modal_footer.vue

 <script>
-import { GlButton, GlIcon } from '@gitlab/ui';
+import { GlButton } from '@gitlab/ui';
 import DismissButton from 'ee/vue_shared/security_reports/components/dismiss_button.vue';
 import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
 import { s__ } from '~/locale';
@@ -9,7 +9,6 @@ export default {
     DismissButton,
     GlButton,
     SplitButton,
-    GlIcon,
   },
   props: {
     modal: {
@@ -75,15 +74,16 @@ export default {
       const issueButton = {
         name: this.createIssueButtonText,
         tagline: s__('ciReport|Investigate this vulnerability by creating an issue'),
-        isLoading: this.isCreatingIssue,
+        icon: this.vulnerability.create_jira_issue_url ? 'external-link' : undefined,
+        loading: this.isCreatingIssue,
+        target: this.vulnerability.create_jira_issue_url ? '_blank' : undefined,
         action: this.vulnerability.create_jira_issue_url ? undefined : 'createNewIssue',
         href: this.vulnerability.create_jira_issue_url,
-        icon: this.vulnerability.create_jira_issue_url ? 'external-link' : undefined,
       };
       const MRButton = {
         name: s__('ciReport|Resolve with merge request'),
         tagline: s__('ciReport|Automatically apply the patch in a new branch'),
-        isLoading: this.isCreatingMergeRequest,
+        loading: this.isCreatingMergeRequest,
         action: 'createMergeRequest',
       };
       const DownloadButton = {
@@ -139,21 +139,14 @@ export default {
 
     <gl-button
       v-else-if="actionButtons.length > 0"
-      :loading="actionButtons[0].isLoading"
+      v-bind="actionButtons[0]"
       :disabled="disabled"
       variant="success"
       category="secondary"
       data-testid="create-issue-button"
       data-qa-selector="create_issue_button"
-      :target="actionButtons[0].href ? '_blank' : undefined"
-      :href="actionButtons[0].href"
       @click="$emit(actionButtons[0].action)"
     >
-      <gl-icon
-        v-if="actionButtons[0].icon"
-        :name="actionButtons[0].icon"
-        class="gl-vertical-align-middle"
-      />
       {{ __(actionButtons[0].name) }}
     </gl-button>
   </div>

ee/app/finders/ee/group_members_finder.rb

@@ -21,9 +21,9 @@ module EE::GroupMembersFinder
     super
   end
 
-  override :relation_group_members
-  def relation_group_members(relation)
-    return all_group_members(relation) if group.minimal_access_role_allowed?
+  override :all_group_members
+  def all_group_members(groups)
+    return members_of_groups(groups) if group.minimal_access_role_allowed?
 
     super
   end

ee/changelogs/unreleased/324246-use-inject-for-vulnerabilities-export-endpoint.yml

+---
+title: Use provide/inject for vulnerabilities export endpoint
+merge_request: 57639
+author:
+type: other

ee/spec/frontend/boards/components/board_add_new_column_spec.js

@@ -17,6 +17,10 @@ describe('BoardAddNewColumn', () => {
   let wrapper;
   let shouldUseGraphQL;
 
+  const selectItem = (id) => {
+    wrapper.findByTestId('selectItem').vm.$emit('change', id);
+  };
+
   const createStore = ({ actions = {}, getters = {}, state = {} } = {}) => {
     return new Vuex.Store({
       state: {
@@ -76,6 +80,11 @@ describe('BoardAddNewColumn', () => {
         },
       }),
     );
+
+    // trigger change event
+    if (selectedId) {
+      selectItem(selectedId);
+    }
   };
 
   afterEach(() => {

ee/spec/frontend/security_dashboard/components/csv_export_button_spec.js

@@ -4,8 +4,10 @@ import MockAdapter from 'axios-mock-adapter';
 import CsvExportButton, {
   STORAGE_KEY,
 } from 'ee/security_dashboard/components/csv_export_button.vue';
+import { useLocalStorageSpy } from 'helpers/local_storage_helper';
 import { TEST_HOST } from 'helpers/test_constants';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
+import AccessorUtils from '~/lib/utils/accessor';
 import axios from '~/lib/utils/axios_utils';
 import { formatDate } from '~/lib/utils/datetime_utility';
 import downloader from '~/lib/utils/downloader';
@@ -14,6 +16,8 @@ import statusCodes from '~/lib/utils/http_status';
 jest.mock('~/flash');
 jest.mock('~/lib/utils/downloader');
 
+useLocalStorageSpy();
+
 const mockReportDate = formatDate(new Date(), 'isoDateTime');
 const vulnerabilitiesExportEndpoint = `${TEST_HOST}/vulnerability_findings.csv`;
 
@@ -29,7 +33,7 @@ describe('Csv Button Export', () => {
 
   const createComponent = () => {
     return shallowMount(CsvExportButton, {
-      propsData: {
+      provide: {
         vulnerabilitiesExportEndpoint,
       },
       stubs: {
@@ -49,7 +53,7 @@ describe('Csv Button Export', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    localStorage.removeItem(STORAGE_KEY);
+    localStorage.clear();
   });
 
   describe('when the user sees the button for the first time', () => {
@@ -117,16 +121,33 @@ describe('Csv Button Export', () => {
     });
 
     it('displays the popover by default', () => {
-      expect(findPopover().attributes('show')).toBeTruthy();
+      expect(findPopover().exists()).toBe(true);
     });
 
-    it('closes the popover when the button is clicked', async () => {
-      const button = findPopoverButton();
-      expect(button.text().trim()).toBe('Got it!');
-      button.vm.$emit('click');
-      await wrapper.vm.$nextTick();
+    describe('closing the popover', () => {
+      it('closes the popover when the button is clicked', async () => {
+        expect(findPopoverButton().text()).toBe('Got it!');
+        findPopoverButton().vm.$emit('click');
+        await wrapper.vm.$nextTick();
+
+        expect(findPopover().exists()).toBe(false);
+      });
+
+      it('sets localStorage', async () => {
+        jest.spyOn(AccessorUtils, 'isLocalStorageAccessSafe').mockImplementation(() => true);
+        findPopoverButton().vm.$emit('click');
+        await wrapper.vm.$nextTick();
 
-      expect(findPopover().attributes('show')).toBeUndefined();
+        expect(localStorage.setItem).toHaveBeenCalledTimes(1);
+      });
+
+      it(`does not set localStorage if it's not available`, async () => {
+        jest.spyOn(AccessorUtils, 'isLocalStorageAccessSafe').mockImplementation(() => false);
+        findPopoverButton().vm.$emit('click');
+        await wrapper.vm.$nextTick();
+
+        expect(localStorage.setItem).toHaveBeenCalledTimes(0);
+      });
     });
   });
 
@@ -137,7 +158,7 @@ describe('Csv Button Export', () => {
     });
 
     it('does not display the popover anymore', () => {
-      expect(findPopover().attributes('show')).toBeFalsy();
+      expect(findPopover().exists()).toBe(false);
     });
   });
 });

ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_spec.js

@@ -14,7 +14,6 @@ describe('First Class Group Dashboard Component', () => {
   const dashboardDocumentation = 'dashboard-documentation';
   const emptyStateSvgPath = 'empty-state-path';
   const groupFullPath = 'group-full-path';
-  const vulnerabilitiesExportEndpoint = '/vulnerabilities/exports';
 
   const findDashboardLayout = () => wrapper.find(SecurityDashboardLayout);
   const findGroupVulnerabilities = () => wrapper.find(FirstClassGroupVulnerabilities);
@@ -29,7 +28,6 @@ describe('First Class Group Dashboard Component', () => {
       propsData: {
         dashboardDocumentation,
         emptyStateSvgPath,
-        vulnerabilitiesExportEndpoint,
       },
       provide: { groupFullPath },
       data,
@@ -89,9 +87,7 @@ describe('First Class Group Dashboard Component', () => {
     });
 
     it('displays the csv export button', () => {
-      expect(findCsvExportButton().props('vulnerabilitiesExportEndpoint')).toBe(
-        vulnerabilitiesExportEndpoint,
-      );
+      expect(findCsvExportButton().exists()).toBe(true);
     });
 
     it('loading button should not be rendered', () => {

ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_spec.js

@@ -14,8 +14,6 @@ describe('First Class Instance Dashboard Component', () => {
     $apollo: { queries: { projects: { loading } } },
   });
 
-  const vulnerabilitiesExportEndpoint = '/vulnerabilities/exports';
-
   const findInstanceVulnerabilities = () => wrapper.find(FirstClassInstanceVulnerabilities);
   const findCsvExportButton = () => wrapper.find(CsvExportButton);
   const findEmptyState = () => wrapper.find(DashboardNotConfigured);
@@ -29,9 +27,6 @@ describe('First Class Instance Dashboard Component', () => {
         return { ...data };
       },
       mocks,
-      propsData: {
-        vulnerabilitiesExportEndpoint,
-      },
       stubs: {
         ...stubs,
         SecurityDashboardLayout,
@@ -77,9 +72,7 @@ describe('First Class Instance Dashboard Component', () => {
     });
 
     it('displays the csv export button', () => {
-      expect(findCsvExportButton().props('vulnerabilitiesExportEndpoint')).toBe(
-        vulnerabilitiesExportEndpoint,
-      );
+      expect(findCsvExportButton().exists()).toBe(true);
     });
 
     it('displays the vulnerability count list with the correct data', () => {

ee/spec/frontend/security_dashboard/components/first_class_project_security_dashboard_spec.js

@@ -21,7 +21,6 @@ const props = {
     path: '/mixed-vulnerabilities/dependency-list-test-01/-/pipelines/214',
   },
   securityDashboardHelpPath: '/security/dashboard/help-path',
-  vulnerabilitiesExportEndpoint: '/vulnerabilities/exports',
 };
 
 const provide = {

spec/finders/group_members_finder_spec.rb

@@ -3,174 +3,180 @@
 require 'spec_helper'
 
 RSpec.describe GroupMembersFinder, '#execute' do
-  let(:group) { create(:group) }
-  let(:nested_group) { create(:group, parent: group) }
-  let(:deeper_nested_group) { create(:group, parent: nested_group) }
-  let(:user1) { create(:user) }
-  let(:user2) { create(:user) }
-  let(:user3) { create(:user) }
-  let(:user4) { create(:user) }
-  let(:user5) { create(:user, :two_factor_via_otp) }
-
-  it 'returns members for top-level group' do
-    member1 = group.add_maintainer(user1)
-    member2 = group.add_maintainer(user2)
-    member3 = group.add_maintainer(user3)
-    create(:group_member, :minimal_access, user: create(:user), source: group)
-
-    result = described_class.new(group).execute
-
-    expect(result.to_a).to match_array([member3, member2, member1])
+  let(:group)         { create(:group) }
+  let(:sub_group)     { create(:group, parent: group) }
+  let(:sub_sub_group) { create(:group, parent: sub_group) }
+  let(:user1)         { create(:user) }
+  let(:user2)         { create(:user) }
+  let(:user3)         { create(:user) }
+  let(:user4)         { create(:user) }
+  let(:user5)         { create(:user, :two_factor_via_otp) }
+
+  let(:groups) do
+    {
+      group:         group,
+      sub_group:     sub_group,
+      sub_sub_group: sub_sub_group
+    }
   end
 
-  it 'returns members & inherited members for nested group by default' do
-    group.add_developer(user2)
-    nested_group.request_access(user4)
-    member1 = group.add_maintainer(user1)
-    member3 = nested_group.add_maintainer(user2)
-    member4 = nested_group.add_maintainer(user3)
-
-    result = described_class.new(nested_group).execute
-
-    expect(result.to_a).to match_array([member1, member3, member4])
+  context 'relations' do
+    let!(:members) do
+      {
+        user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1),
+        user1_sub_group:     create(:group_member, :developer,  group: sub_group,     user: user1),
+        user1_group:         create(:group_member, :reporter,   group: group,         user: user1),
+        user2_sub_sub_group: create(:group_member, :reporter,   group: sub_sub_group, user: user2),
+        user2_sub_group:     create(:group_member, :developer,  group: sub_group,     user: user2),
+        user2_group:         create(:group_member, :maintainer, group: group,         user: user2),
+        user3_sub_sub_group: create(:group_member, :developer,  group: sub_sub_group, user: user3, expires_at: 1.day.from_now),
+        user3_sub_group:     create(:group_member, :developer,  group: sub_group,     user: user3, expires_at: 2.days.from_now),
+        user3_group:         create(:group_member, :reporter,   group: group,         user: user3),
+        user4_sub_sub_group: create(:group_member, :reporter,   group: sub_sub_group, user: user4),
+        user4_sub_group:     create(:group_member, :developer,  group: sub_group,     user: user4, expires_at: 1.day.from_now),
+        user4_group:         create(:group_member, :developer,  group: group,         user: user4, expires_at: 2.days.from_now)
+      }
+    end
+
+    using RSpec::Parameterized::TableSyntax
+
+    where(:subject_relations, :subject_group, :expected_members) do
+      nil                                 | :group         | [:user1_group,         :user2_group,         :user3_group,         :user4_group]
+      [:direct]                           | :group         | [:user1_group,         :user2_group,         :user3_group,         :user4_group]
+      [:inherited]                        | :group         | []
+      [:descendants]                      | :group         | [:user1_sub_sub_group, :user2_sub_group,     :user3_sub_group,     :user4_sub_group]
+      [:direct, :inherited]               | :group         | [:user1_group,         :user2_group,         :user3_group,         :user4_group]
+      [:direct, :descendants]             | :group         | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+      [:descendants, :inherited]          | :group         | [:user1_sub_sub_group, :user2_sub_group,     :user3_sub_group,     :user4_sub_group]
+      [:direct, :descendants, :inherited] | :group         | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+      nil                                 | :sub_group     | [:user1_sub_group,     :user2_group,         :user3_sub_group,     :user4_group]
+      [:direct]                           | :sub_group     | [:user1_sub_group,     :user2_sub_group,     :user3_sub_group,     :user4_sub_group]
+      [:inherited]                        | :sub_group     | [:user1_group,         :user2_group,         :user3_group,         :user4_group]
+      [:descendants]                      | :sub_group     | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group]
+      [:direct, :inherited]               | :sub_group     | [:user1_sub_group,     :user2_group,         :user3_sub_group,     :user4_group]
+      [:direct, :descendants]             | :sub_group     | [:user1_sub_sub_group, :user2_sub_group,     :user3_sub_group,     :user4_sub_group]
+      [:descendants, :inherited]          | :sub_group     | [:user1_sub_sub_group, :user2_group,         :user3_sub_sub_group, :user4_group]
+      [:direct, :descendants, :inherited] | :sub_group     | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+      nil                                 | :sub_sub_group | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+      [:direct]                           | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group]
+      [:inherited]                        | :sub_sub_group | [:user1_sub_group,     :user2_group,         :user3_sub_group,     :user4_group]
+      [:descendants]                      | :sub_sub_group | []
+      [:direct, :inherited]               | :sub_sub_group | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+      [:direct, :descendants]             | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group]
+      [:descendants, :inherited]          | :sub_sub_group | [:user1_sub_group,     :user2_group,         :user3_sub_group,     :user4_group]
+      [:direct, :descendants, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group,         :user3_sub_group,     :user4_group]
+    end
+
+    with_them do
+      it 'returns correct members' do
+        result = if subject_relations
+                   described_class.new(groups[subject_group]).execute(include_relations: subject_relations)
+                 else
+                   described_class.new(groups[subject_group]).execute
+                 end
+
+        expect(result.to_a).to match_array(expected_members.map { |name| members[name] })
+      end
+    end
   end
 
-  it 'does not return inherited members for nested group if requested' do
-    group.add_maintainer(user1)
-    group.add_developer(user2)
-    member2 = nested_group.add_maintainer(user2)
-    member3 = nested_group.add_maintainer(user3)
+  context 'search' do
+    it 'returns searched members if requested' do
+      group.add_maintainer(user2)
+      group.add_developer(user3)
+      member = group.add_maintainer(user1)
 
-    result = described_class.new(nested_group).execute(include_relations: [:direct])
+      result = described_class.new(group, params: { search: user1.name }).execute
 
-    expect(result.to_a).to match_array([member2, member3])
-  end
+      expect(result.to_a).to match_array([member])
+    end
 
-  it 'returns only inherited members for nested group if requested' do
-    group.add_developer(user2)
-    nested_group.request_access(user4)
-    member1 = group.add_maintainer(user1)
-    nested_group.add_maintainer(user2)
-    nested_group.add_maintainer(user3)
+    it 'returns nothing if search only in inherited relation' do
+      group.add_maintainer(user2)
+      group.add_developer(user3)
+      group.add_maintainer(user1)
 
-    result = described_class.new(nested_group).execute(include_relations: [:inherited])
+      result = described_class.new(group, params: { search: user1.name }).execute(include_relations: [:inherited])
 
-    expect(result.to_a).to match_array([member1])
-  end
+      expect(result.to_a).to match_array([])
+    end
 
-  it 'does not return nil if `inherited only` relation is requested on root group' do
-    group.add_developer(user2)
+    it 'returns searched member only from sub_group if search only in inherited relation' do
+      group.add_maintainer(user2)
+      group.add_developer(user3)
+      sub_group.add_maintainer(create(:user, name: user1.name))
+      member = group.add_maintainer(user1)
 
-    result = described_class.new(group).execute(include_relations: [:inherited])
+      result = described_class.new(sub_group, params: { search: member.user.name }).execute(include_relations: [:inherited])
 
-    expect(result).not_to be_nil
+      expect(result.to_a).to contain_exactly(member)
+    end
   end
 
-  it 'returns members for descendant groups if requested' do
-    member1 = group.add_maintainer(user2)
-    member2 = group.add_maintainer(user1)
-    nested_group.add_maintainer(user2)
-    member3 = nested_group.add_maintainer(user3)
-    member4 = nested_group.add_maintainer(user4)
+  context 'filter by two-factor' do
+    it 'returns members with two-factor auth if requested by owner' do
+      group.add_owner(user2)
+      group.add_maintainer(user1)
+      member = group.add_maintainer(user5)
 
-    result = described_class.new(group).execute(include_relations: [:direct, :descendants])
+      result = described_class.new(group, user2, params: { two_factor: 'enabled' }).execute
 
-    expect(result.to_a).to match_array([member1, member2, member3, member4])
-  end
+      expect(result.to_a).to contain_exactly(member)
+    end
 
-  it 'returns searched members if requested' do
-    group.add_maintainer(user2)
-    group.add_developer(user3)
-    member = group.add_maintainer(user1)
+    it 'returns members without two-factor auth if requested by owner' do
+      member1 = group.add_owner(user2)
+      member2 = group.add_maintainer(user1)
+      member_with_2fa = group.add_maintainer(user5)
 
-    result = described_class.new(group, params: { search: user1.name }).execute
+      result = described_class.new(group, user2, params: { two_factor: 'disabled' }).execute
 
-    expect(result.to_a).to match_array([member])
-  end
+      expect(result.to_a).not_to include(member_with_2fa)
+      expect(result.to_a).to match_array([member1, member2])
+    end
 
-  it 'returns nothing if search only in inherited relation' do
-    group.add_maintainer(user2)
-    group.add_developer(user3)
-    group.add_maintainer(user1)
+    it 'returns direct members with two-factor auth if requested by owner' do
+      group.add_owner(user1)
+      group.add_maintainer(user2)
+      sub_group.add_maintainer(user3)
+      member_with_2fa = sub_group.add_maintainer(user5)
 
-    result = described_class.new(group, params: { search: user1.name }).execute(include_relations: [:inherited])
+      result = described_class.new(sub_group, user1, params: { two_factor: 'enabled' }).execute(include_relations: [:direct])
 
-    expect(result.to_a).to match_array([])
-  end
+      expect(result.to_a).to match_array([member_with_2fa])
+    end
 
-  it 'returns searched member only from nested_group if search only in inherited relation' do
-    group.add_maintainer(user2)
-    group.add_developer(user3)
-    nested_group.add_maintainer(create(:user, name: user1.name))
-    member = group.add_maintainer(user1)
+    it 'returns inherited members with two-factor auth if requested by owner' do
+      group.add_owner(user1)
+      member_with_2fa = group.add_maintainer(user5)
+      sub_group.add_maintainer(user2)
+      sub_group.add_maintainer(user3)
 
-    result = described_class.new(nested_group, params: { search: member.user.name }).execute(include_relations: [:inherited])
+      result = described_class.new(sub_group, user1, params: { two_factor: 'enabled' }).execute(include_relations: [:inherited])
 
-    expect(result.to_a).to contain_exactly(member)
-  end
-
-  it 'returns members with two-factor auth if requested by owner' do
-    group.add_owner(user2)
-    group.add_maintainer(user1)
-    member = group.add_maintainer(user5)
-
-    result = described_class.new(group, user2, params: { two_factor: 'enabled' }).execute
+      expect(result.to_a).to match_array([member_with_2fa])
+    end
 
-    expect(result.to_a).to contain_exactly(member)
-  end
-
-  it 'returns members without two-factor auth if requested by owner' do
-    member1 = group.add_owner(user2)
-    member2 = group.add_maintainer(user1)
-    member_with_2fa = group.add_maintainer(user5)
+    it 'returns direct members without two-factor auth if requested by owner' do
+      group.add_owner(user1)
+      group.add_maintainer(user2)
+      member3 = sub_group.add_maintainer(user3)
+      sub_group.add_maintainer(user5)
 
-    result = described_class.new(group, user2, params: { two_factor: 'disabled' }).execute
+      result = described_class.new(sub_group, user1, params: { two_factor: 'disabled' }).execute(include_relations: [:direct])
 
-    expect(result.to_a).not_to include(member_with_2fa)
-    expect(result.to_a).to match_array([member1, member2])
-  end
-
-  it 'returns direct members with two-factor auth if requested by owner' do
-    group.add_owner(user1)
-    group.add_maintainer(user2)
-    nested_group.add_maintainer(user3)
-    member_with_2fa = nested_group.add_maintainer(user5)
-
-    result = described_class.new(nested_group, user1, params: { two_factor: 'enabled' }).execute(include_relations: [:direct])
-
-    expect(result.to_a).to match_array([member_with_2fa])
-  end
-
-  it 'returns inherited members with two-factor auth if requested by owner' do
-    group.add_owner(user1)
-    member_with_2fa = group.add_maintainer(user5)
-    nested_group.add_maintainer(user2)
-    nested_group.add_maintainer(user3)
-
-    result = described_class.new(nested_group, user1, params: { two_factor: 'enabled' }).execute(include_relations: [:inherited])
-
-    expect(result.to_a).to match_array([member_with_2fa])
-  end
-
-  it 'returns direct members without two-factor auth if requested by owner' do
-    group.add_owner(user1)
-    group.add_maintainer(user2)
-    member3 = nested_group.add_maintainer(user3)
-    nested_group.add_maintainer(user5)
-
-    result = described_class.new(nested_group, user1, params: { two_factor: 'disabled' }).execute(include_relations: [:direct])
-
-    expect(result.to_a).to match_array([member3])
-  end
+      expect(result.to_a).to match_array([member3])
+    end
 
-  it 'returns inherited members without two-factor auth if requested by owner' do
-    member1 = group.add_owner(user1)
-    group.add_maintainer(user5)
-    nested_group.add_maintainer(user2)
-    nested_group.add_maintainer(user3)
+    it 'returns inherited members without two-factor auth if requested by owner' do
+      member1 = group.add_owner(user1)
+      group.add_maintainer(user5)
+      sub_group.add_maintainer(user2)
+      sub_group.add_maintainer(user3)
 
-    result = described_class.new(nested_group, user1, params: { two_factor: 'disabled' }).execute(include_relations: [:inherited])
+      result = described_class.new(sub_group, user1, params: { two_factor: 'disabled' }).execute(include_relations: [:inherited])
 
-    expect(result.to_a).to match_array([member1])
+      expect(result.to_a).to match_array([member1])
+    end
   end
 end

spec/frontend/boards/components/board_add_new_column_spec.js

+import { GlFormRadioGroup } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue, { nextTick } from 'vue';
 import Vuex from 'vuex';
@@ -12,6 +13,10 @@ Vue.use(Vuex);
 describe('Board card layout', () => {
   let wrapper;
 
+  const selectLabel = (id) => {
+    wrapper.findComponent(GlFormRadioGroup).vm.$emit('change', id);
+  };
+
   const createStore = ({ actions = {}, getters = {}, state = {} } = {}) => {
     return new Vuex.Store({
       state: {
@@ -57,6 +62,11 @@ describe('Board card layout', () => {
         },
       }),
     );
+
+    // trigger change event
+    if (selectedId) {
+      selectLabel(selectedId);
+    }
   };
 
   afterEach(() => {

spec/frontend/error_tracking/components/error_tracking_list_spec.js

@@ -52,7 +52,6 @@ describe('ErrorTrackingList', () => {
 
   beforeEach(() => {
     actions = {
-      getErrorList: () => {},
       startPolling: jest.fn(),
       restartPolling: jest.fn().mockName('restartPolling'),
       addRecentSearch: jest.fn(),

spec/models/member_spec.rb

@@ -413,6 +413,24 @@ RSpec.describe Member do
       it { is_expected.not_to include @blocked_developer }
       it { is_expected.not_to include @member_with_minimal_access }
     end
+
+    describe '.distinct_on_user_with_max_access_level' do
+      let_it_be(:other_group) { create(:group) }
+      let_it_be(:member_with_lower_access_level) { create(:group_member, :developer, group: other_group, user: @owner_user) }
+
+      subject { described_class.default_scoped.distinct_on_user_with_max_access_level.to_a }
+
+      it { is_expected.not_to include member_with_lower_access_level }
+      it { is_expected.to include @owner }
+      it { is_expected.to include @maintainer }
+      it { is_expected.to include @invited_member }
+      it { is_expected.to include @accepted_invite_member }
+      it { is_expected.to include @requested_member }
+      it { is_expected.to include @accepted_request_member }
+      it { is_expected.to include @blocked_maintainer }
+      it { is_expected.to include @blocked_developer }
+      it { is_expected.to include @member_with_minimal_access }
+    end
   end
 
   describe "Delegate methods" do