Merge branch 'peterhegman/add-ldap-related-properties-to-group-members' into 'master'

Add LDAP scaffolding to group members Vue app [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!44386

app/assets/javascripts/groups/members/index.js

 import Vue from 'vue';
 import Vuex from 'vuex';
+import { parseDataAttributes } from 'ee_else_ce/groups/members/utils';
 import App from './components/app.vue';
 import membersModule from '~/vuex_shared/modules/members';
-import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 
 export const initGroupMembersApp = (el, tableFields) => {
   if (!el) {
@@ -11,15 +11,11 @@ export const initGroupMembersApp = (el, tableFields) => {
 
   Vue.use(Vuex);
 
-  const { members, groupId, memberPath } = el.dataset;
-
   const store = new Vuex.Store({
     ...membersModule({
-      members: convertObjectPropsToCamelCase(JSON.parse(members), { deep: true }),
-      sourceId: parseInt(groupId, 10),
+      ...parseDataAttributes(el),
       currentUserId: gon.current_user_id || null,
       tableFields,
-      memberPath,
     }),
   });
 

app/assets/javascripts/groups/members/utils.js

+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+
+export const parseDataAttributes = el => {
+  const { members, groupId, memberPath } = el.dataset;
+
+  return {
+    members: convertObjectPropsToCamelCase(JSON.parse(members), { deep: true }),
+    sourceId: parseInt(groupId, 10),
+    memberPath,
+  };
+};

app/assets/javascripts/vuex_shared/modules/members/index.js

-import createState from './state';
+import createState from 'ee_else_ce/vuex_shared/modules/members/state';
 
 export default initialState => ({
   namespaced: true,

app/helpers/groups/group_members_helper.rb

@@ -21,6 +21,22 @@ module Groups::GroupMembersHelper
     members_data(group, members).to_json
   end
 
+  # Overridden in `ee/app/helpers/ee/groups/group_members_helper.rb`
+  def group_members_list_data_attributes(group, members)
+    {
+      members: members_data_json(group, members),
+      member_path: group_group_member_path(group, ':id'),
+      group_id: group.id
+    }
+  end
+
+  def linked_groups_list_data_attributes(group)
+    {
+      members: linked_groups_data_json(group.shared_with_group_links),
+      group_id: group.id
+    }
+  end
+
   private
 
   def members_data(group, members)
@@ -35,7 +51,6 @@ module Groups::GroupMembersHelper
         requested_at: member.requested_at,
         can_update: member.can_update?,
         can_remove: member.can_remove?,
-        can_override: member.can_override?,
         access_level: {
           string_value: member.human_access,
           integer_value: member.access_level

app/views/groups/group_members/index.html.haml

@@ -4,7 +4,6 @@
 - show_access_requests = can_manage_members && @requesters.exists?
 - invited_active = params[:search_invited].present? || params[:invited_members_page].present?
 - vue_members_list_enabled = Feature.enabled?(:vue_group_members_list, @group)
-- data_attributes = { group_id: @group.id }
 
 - form_item_label_css_class = 'label-bold gl-mr-2 gl-mb-0 gl-py-2 align-self-md-center'
 
@@ -69,7 +68,7 @@
               = label_tag :sort_by, _('Sort by'), class: form_item_label_css_class
               = render 'shared/members/sort_dropdown'
         - if vue_members_list_enabled
-          .js-group-members-list{ data: { members: members_data_json(@group, @members), member_path: group_group_member_path(id: ':id'), **data_attributes } }
+          .js-group-members-list{ data: group_members_list_data_attributes(@group, @members) }
         - else
           %ul.content-list.members-list{ data: { qa_selector: 'members_list' } }
             = render partial: 'shared/members/member', collection: @members, as: :member
@@ -81,7 +80,7 @@
             = render 'groups/group_members/tab_pane/title' do
               = html_escape(_('Groups with access to %{strong_start}%{group_name}%{strong_end}')) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
           - if vue_members_list_enabled
-            .js-group-linked-list{ data: { members: linked_groups_data_json(@group.shared_with_group_links), **data_attributes } }
+            .js-group-linked-list{ data: linked_groups_list_data_attributes(@group) }
           - else
             %ul.content-list.members-list{ data: { qa_selector: 'groups_list' } }
               - @group.shared_with_group_links.each do |group_link|
@@ -95,7 +94,7 @@
             = form_tag group_group_members_path(@group), method: :get, class: 'user-search-form', data: { testid: 'user-search-form' } do
               = render 'shared/members/search_field', name: 'search_invited'
           - if vue_members_list_enabled
-            .js-group-invited-members-list{ data: { members: members_data_json(@group, @invited_members), member_path: group_group_member_path(id: ':id'), **data_attributes } }
+            .js-group-invited-members-list{ data: group_members_list_data_attributes(@group, @invited_members) }
           - else
             %ul.content-list.members-list
               = render partial: 'shared/members/member', collection: @invited_members, as: :member
@@ -107,7 +106,7 @@
             = render 'groups/group_members/tab_pane/title' do
               = html_escape(_('Users requesting access to %{strong_start}%{group_name}%{strong_end}')) % { group_name: @group.name, strong_start: '<strong>'.html_safe, strong_end: '</strong>'.html_safe }
           - if vue_members_list_enabled
-            .js-group-access-requests-list{ data: { members: members_data_json(@group, @requesters), member_path: group_group_member_path(id: ':id'), **data_attributes } }
+            .js-group-access-requests-list{ data: group_members_list_data_attributes(@group, @requesters) }
           - else
             %ul.content-list.members-list
               = render partial: 'shared/members/member', collection: @requesters, as: :member

ee/app/assets/javascripts/groups/members/utils.js

+import { parseDataAttributes as CEParseDataAttributes } from '~/groups/members/utils';
+
+export const parseDataAttributes = el => {
+  const { ldapOverridePath } = el.dataset;
+
+  return {
+    ...CEParseDataAttributes(el),
+    ldapOverridePath,
+  };
+};

ee/app/assets/javascripts/vuex_shared/modules/members/state.js

+import createState from '~/vuex_shared/modules/members/state';
+
+export default initialState => {
+  const { ldapOverridePath } = initialState;
+
+  return {
+    ldapOverridePath,
+    ...createState(initialState),
+  };
+};

ee/app/helpers/ee/groups/group_members_helper.rb

@@ -8,6 +8,13 @@ module EE::Groups::GroupMembersHelper
     super.merge(skip_ldap: @group.ldap_synced?)
   end
 
+  override :group_members_list_data_attributes
+  def group_members_list_data_attributes(group, _members)
+    super.merge!({
+      ldap_override_path: override_group_group_member_path(group, ':id')
+    })
+  end
+
   private
 
   override :members_data
@@ -18,7 +25,9 @@ module EE::Groups::GroupMembersHelper
       ce_members[index].merge({
         using_license: can?(current_user, :owner_access, group) && member.user&.using_gitlab_com_seat?(group),
         group_sso: member.user&.group_sso?(group),
-        group_managed_account: member.user&.group_managed_account?
+        group_managed_account: member.user&.group_managed_account?,
+        can_override: member.can_override?,
+        is_overridden: member.override
       })
     end
   end

ee/spec/fixtures/api/schemas/group_members.json

@@ -4,11 +4,13 @@
     "allOf": [
       { "$ref": "../../../../../spec/fixtures/api/schemas/group_member.json" },
       {
-        "required": ["using_license", "group_sso", "group_managed_account"],
+        "required": ["using_license", "group_sso", "group_managed_account", "can_override", "is_overridden"],
         "properties": {
           "using_license": { "type": ["boolean", "null"] },
           "group_sso": { "type": ["boolean", "null"] },
-          "group_managed_account": { "type": ["boolean", "null"] }
+          "group_managed_account": { "type": ["boolean", "null"] },
+          "can_override": { "type": ["boolean"] },
+          "is_overridden": { "type": ["boolean"] }
         }
       }
     ] 

ee/spec/frontend/groups/members/index_spec.js

+import { membersJsonString } from 'jest/groups/members/mock_data';
+import { initGroupMembersApp } from '~/groups/members';
+
+describe('initGroupMembersApp', () => {
+  let el;
+  let vm;
+
+  const createVm = () => {
+    vm = initGroupMembersApp(el, ['account']);
+  };
+
+  beforeEach(() => {
+    el = document.createElement('div');
+    el.setAttribute('data-members', membersJsonString);
+    el.setAttribute('data-group-id', '234');
+    el.setAttribute('data-ldap-override-path', '/groups/ldap-group/-/group_members/:id/override');
+  });
+
+  afterEach(() => {
+    el = null;
+  });
+
+  it('sets `ldapOverridePath` in Vuex store', () => {
+    createVm();
+
+    expect(vm.$store.state.ldapOverridePath).toBe(
+      '/groups/ldap-group/-/group_members/:id/override',
+    );
+  });
+});

ee/spec/frontend/groups/members/utils_spec.js

+import { membersJsonString, membersParsed } from 'jest/groups/members/mock_data';
+import { parseDataAttributes } from 'ee/groups/members/utils';
+
+describe('group member utils', () => {
+  describe('parseDataAttributes', () => {
+    let el;
+
+    beforeEach(() => {
+      el = document.createElement('div');
+      el.setAttribute('data-members', membersJsonString);
+      el.setAttribute('data-group-id', '234');
+      el.setAttribute('data-ldap-override-path', '/groups/ldap-group/-/group_members/:id/override');
+    });
+
+    afterEach(() => {
+      el = null;
+    });
+
+    it('correctly parses the data attributes', () => {
+      expect(parseDataAttributes(el)).toEqual({
+        members: membersParsed,
+        sourceId: 234,
+        ldapOverridePath: '/groups/ldap-group/-/group_members/:id/override',
+      });
+    });
+  });
+});

ee/spec/helpers/ee/groups/group_members_helper_spec.rb

@@ -5,9 +5,9 @@ require "spec_helper"
 RSpec.describe Groups::GroupMembersHelper do
   include MembersPresentation
 
-  describe '.group_member_select_options' do
-    let(:group) { create(:group) }
+  let_it_be(:group) { create(:group) }
 
+  describe '.group_member_select_options' do
     before do
       helper.instance_variable_set(:@group, group)
     end
@@ -19,7 +19,6 @@ RSpec.describe Groups::GroupMembersHelper do
 
   describe '#members_data' do
     let(:current_user) { create(:user) }
-    let(:group) { create(:group) }
     let(:group_member) { create(:group_member, group: group, created_by: current_user) }
 
     subject { helper.send('members_data', group, present_members([group_member])) }
@@ -47,4 +46,15 @@ RSpec.describe Groups::GroupMembersHelper do
       expect(subject.first).to include(group_managed_account: true)
     end
   end
+
+  describe '#group_members_list_data_attributes' do
+    before do
+      allow(helper).to receive(:override_group_group_member_path).with(group, ':id').and_return('/groups/foo-bar/-/group_members/:id/override')
+      allow(helper).to receive(:group_group_member_path).with(group, ':id').and_return('/groups/foo-bar/-/group_members/:id')
+    end
+
+    it 'adds `ldap_override_path` to returned hash' do
+      expect(helper.group_members_list_data_attributes(group, {})).to include(ldap_override_path: '/groups/foo-bar/-/group_members/:id/override')
+    end
+  end
 end

spec/fixtures/api/schemas/group_member.json

@@ -9,8 +9,7 @@
     "source",
     "valid_roles",
     "can_update",
-    "can_remove",
-    "can_override"
+    "can_remove"
   ],
   "properties": {
     "id": { "type": "integer" },
@@ -19,7 +18,6 @@
     "requested_at": { "type": ["date-time", "null"] },
     "can_update": { "type": "boolean" },
     "can_remove": { "type": "boolean" },
-    "can_override": { "type": "boolean" },
     "access_level": {
       "type": "object",
       "required": ["integer_value", "string_value"],

spec/frontend/groups/members/index_spec.js

@@ -20,12 +20,9 @@ describe('initGroupMembersApp', () => {
     el.setAttribute('data-member-path', '/groups/foo-bar/-/group_members/:id');
 
     window.gon = { current_user_id: 123 };
-
-    document.body.appendChild(el);
   });
 
   afterEach(() => {
-    document.body.innerHTML = '';
     el = null;
 
     wrapper.destroy();

spec/frontend/groups/members/utils_spec.js

+import { membersJsonString, membersParsed } from './mock_data';
+import { parseDataAttributes } from '~/groups/members/utils';
+
+describe('group member utils', () => {
+  describe('parseDataAttributes', () => {
+    let el;
+
+    beforeEach(() => {
+      el = document.createElement('div');
+      el.setAttribute('data-members', membersJsonString);
+      el.setAttribute('data-group-id', '234');
+    });
+
+    afterEach(() => {
+      el = null;
+    });
+
+    it('correctly parses the data attributes', () => {
+      expect(parseDataAttributes(el)).toEqual({
+        members: membersParsed,
+        sourceId: 234,
+      });
+    });
+  });
+});

spec/helpers/groups/group_members_helper_spec.rb

@@ -6,10 +6,14 @@ RSpec.describe Groups::GroupMembersHelper do
   include MembersPresentation
 
   let_it_be(:current_user) { create(:user) }
+  let_it_be(:group) { create(:group) }
 
-  describe '.group_member_select_options' do
-    let(:group) { create(:group) }
+  before do
+    allow(helper).to receive(:can?).with(current_user, :owner_access, group).and_return(true)
+    allow(helper).to receive(:current_user).and_return(current_user)
+  end
 
+  describe '.group_member_select_options' do
     before do
       helper.instance_variable_set(:@group, group)
     end
@@ -22,10 +26,6 @@ RSpec.describe Groups::GroupMembersHelper do
   describe '#linked_groups_data_json' do
     include_context 'group_group_link'
 
-    before do
-      allow(helper).to receive(:current_user).and_return(current_user)
-    end
-
     it 'matches json schema' do
       json = helper.linked_groups_data_json(shared_group.shared_with_group_links)
 
@@ -34,13 +34,6 @@ RSpec.describe Groups::GroupMembersHelper do
   end
 
   describe '#members_data_json' do
-    let(:group) { create(:group) }
-
-    before do
-      allow(helper).to receive(:can?).with(current_user, :owner_access, group).and_return(true)
-      allow(helper).to receive(:current_user).and_return(current_user)
-    end
-
     shared_examples 'group_members.json' do
       it 'matches json schema' do
         json = helper.members_data_json(group, present_members([group_member]))
@@ -75,4 +68,31 @@ RSpec.describe Groups::GroupMembersHelper do
       it_behaves_like 'group_members.json'
     end
   end
+
+  describe '#group_members_list_data_attributes' do
+    let(:group_member) { create(:group_member, group: group, created_by: current_user) }
+
+    before do
+      allow(helper).to receive(:group_group_member_path).with(group, ':id').and_return('/groups/foo-bar/-/group_members/:id')
+    end
+
+    it 'returns expected hash' do
+      expect(helper.group_members_list_data_attributes(group, present_members([group_member]))).to include({
+        members: helper.members_data_json(group, present_members([group_member])),
+        member_path: '/groups/foo-bar/-/group_members/:id',
+        group_id: group.id
+      })
+    end
+  end
+
+  describe '#linked_groups_list_data_attributes' do
+    include_context 'group_group_link'
+
+    it 'returns expected hash' do
+      expect(helper.linked_groups_list_data_attributes(shared_group)).to include({
+        members: helper.linked_groups_data_json(shared_group.shared_with_group_links),
+        group_id: shared_group.id
+      })
+    end
+  end
 end