Merge branch '324681-group-project-members-migrate-to-one-vue-app-and-gltabs' into 'master'

Refactor how data is passed to group/project members Vue apps [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!61049

app/assets/javascripts/members/index.js

 import { GlToast } from '@gitlab/ui';
 import Vue from 'vue';
 import Vuex from 'vuex';
-import { parseDataAttributes } from 'ee_else_ce/members/utils';
+import { parseDataAttributes } from '~/members/utils';
 import App from './components/app.vue';
 import membersStore from './store';
 

app/assets/javascripts/members/utils.js

 import { isUndefined } from 'lodash';
-import {
-  getParameterByName,
-  convertObjectPropsToCamelCase,
-  parseBoolean,
-} from '~/lib/utils/common_utils';
+import { getParameterByName, convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { setUrlParams } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
 import {
@@ -105,18 +101,12 @@ export const buildSortHref = ({
 export const canOverride = () => false;
 
 export const parseDataAttributes = (el) => {
-  const { members, pagination, sourceId, memberPath, canManageMembers } = el.dataset;
+  const { membersData } = el.dataset;
 
-  return {
-    members: convertObjectPropsToCamelCase(JSON.parse(members), { deep: true }),
-    pagination: convertObjectPropsToCamelCase(JSON.parse(pagination || '{}'), {
-      deep: true,
-      ignoreKeyNames: ['params'],
-    }),
-    sourceId: parseInt(sourceId, 10),
-    memberPath,
-    canManageMembers: parseBoolean(canManageMembers),
-  };
+  return convertObjectPropsToCamelCase(JSON.parse(membersData), {
+    deep: true,
+    ignoreKeyNames: ['params'],
+  });
 };
 
 export const baseRequestFormatter = (basePropertyName, accessLevelPropertyName) => ({

app/helpers/groups/group_members_helper.rb

@@ -13,31 +13,41 @@ module Groups::GroupMembersHelper
     render 'shared/members/invite_member', submit_url: group_group_members_path(group), access_levels: group.access_level_roles, default_access_level: default_access_level
   end
 
-  def group_group_links_data_json(group_links)
-    GroupLink::GroupGroupLinkSerializer.new.represent(group_links, { current_user: current_user }).to_json
+  def group_members_list_data_json(group, members, pagination = {})
+    group_members_list_data(group, members, pagination).to_json
   end
 
-  def members_data_json(group, members)
-    MemberSerializer.new.represent(members, { current_user: current_user, group: group, source: group }).to_json
+  def group_group_links_list_data_json(group)
+    group_group_links_list_data(group).to_json
+  end
+
+  private
+
+  def group_members_serialized(group, members)
+    MemberSerializer.new.represent(members, { current_user: current_user, group: group, source: group })
+  end
+
+  def group_group_links_serialized(group_links)
+    GroupLink::GroupGroupLinkSerializer.new.represent(group_links, { current_user: current_user })
   end
 
   # Overridden in `ee/app/helpers/ee/groups/group_members_helper.rb`
-  def group_members_list_data_attributes(group, members, pagination = {})
+  def group_members_list_data(group, members, pagination)
     {
-      members: members_data_json(group, members),
-      pagination: members_pagination_data_json(members, pagination),
+      members: group_members_serialized(group, members),
+      pagination: members_pagination_data(members, pagination),
       member_path: group_group_member_path(group, ':id'),
       source_id: group.id,
-      can_manage_members: can?(current_user, :admin_group_member, group).to_s
+      can_manage_members: can?(current_user, :admin_group_member, group)
     }
   end
 
-  def group_group_links_list_data_attributes(group)
+  def group_group_links_list_data(group)
     group_links = group.shared_with_group_links
 
     {
-      members: group_group_links_data_json(group_links),
-      pagination: members_pagination_data_json(group_links),
+      members: group_group_links_serialized(group_links),
+      pagination: members_pagination_data(group_links),
       member_path: group_group_link_path(group, ':id'),
       source_id: group.id
     }

app/helpers/members_helper.rb

@@ -66,13 +66,13 @@ module MembersHelper
     'group and any subresources'
   end
 
-  def members_pagination_data_json(members, pagination = {})
+  def members_pagination_data(members, pagination = {})
     {
       current_page: members.respond_to?(:current_page) ? members.current_page : nil,
       per_page: members.respond_to?(:limit_value) ? members.limit_value : nil,
       total_items: members.respond_to?(:total_count) ? members.total_count : members.count,
       param_name: pagination[:param_name] || nil,
       params: pagination[:params] || {}
-    }.to_json
+    }
   end
 end

app/helpers/projects/project_members_helper.rb

@@ -27,31 +27,41 @@ module Projects::ProjectMembersHelper
     project.group.has_owner?(current_user)
   end
 
-  def project_group_links_data_json(group_links)
-    GroupLink::ProjectGroupLinkSerializer.new.represent(group_links, { current_user: current_user }).to_json
+  def project_members_list_data_json(project, members, pagination = {})
+    project_members_list_data(project, members, pagination).to_json
   end
 
-  def project_members_data_json(project, members)
-    MemberSerializer.new.represent(members, { current_user: current_user, group: project.group, source: project }).to_json
+  def project_group_links_list_data_json(project, group_links)
+    project_group_links_list_data(project, group_links).to_json
   end
 
-  def project_members_list_data_attributes(project, members, pagination = {})
+  private
+
+  def project_members_serialized(project, members)
+    MemberSerializer.new.represent(members, { current_user: current_user, group: project.group, source: project })
+  end
+
+  def project_group_links_serialized(group_links)
+    GroupLink::ProjectGroupLinkSerializer.new.represent(group_links, { current_user: current_user })
+  end
+
+  def project_members_list_data(project, members, pagination)
     {
-      members: project_members_data_json(project, members),
-      pagination: members_pagination_data_json(members, pagination),
+      members: project_members_serialized(project, members),
+      pagination: members_pagination_data(members, pagination),
       member_path: project_project_member_path(project, ':id'),
       source_id: project.id,
-      can_manage_members: can_manage_project_members?(project).to_s
+      can_manage_members: can_manage_project_members?(project)
     }
   end
 
-  def project_group_links_list_data_attributes(project, group_links)
+  def project_group_links_list_data(project, group_links)
     {
-      members: project_group_links_data_json(group_links),
-      pagination: members_pagination_data_json(group_links),
+      members: project_group_links_serialized(group_links),
+      pagination: members_pagination_data(group_links),
       member_path: project_group_link_path(project, ':id'),
       source_id: project.id,
-      can_manage_members: can_manage_project_members?(project).to_s
+      can_manage_members: can_manage_project_members?(project)
     }
   end
 end

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

@@ -61,21 +61,21 @@
             %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm= @requesters.count
     .tab-content
       #tab-members.tab-pane{ class: ('active' unless invited_active) }
-        .js-group-members-list{ data: group_members_list_data_attributes(@group, @members, { param_name: :page, params: { invited_members_page: nil, search_invited: nil } }) }
+        .js-group-members-list{ data: { members_data: group_members_list_data_json(@group, @members, { param_name: :page, params: { invited_members_page: nil, search_invited: nil } }) } }
           .loading
             .gl-spinner.gl-spinner-md
       - if @group.shared_with_group_links.present?
         #tab-groups.tab-pane
-          .js-group-group-links-list{ data: group_group_links_list_data_attributes(@group) }
+          .js-group-group-links-list{ data: { members_data: group_group_links_list_data_json(@group) } }
             .loading
               .gl-spinner.gl-spinner-md
       - if show_invited_members
         #tab-invited-members.tab-pane{ class: ('active' if invited_active) }
-          .js-group-invited-members-list{ data: group_members_list_data_attributes(@group, @invited_members, { param_name: :invited_members_page, params: { page: nil } }) }
+          .js-group-invited-members-list{ data: { members_data: group_members_list_data_json(@group, @invited_members, { param_name: :invited_members_page, params: { page: nil } }) } }
             .loading
               .gl-spinner.gl-spinner-md
       - if show_access_requests
         #tab-access-requests.tab-pane
-          .js-group-access-requests-list{ data: group_members_list_data_attributes(@group, @requesters) }
+          .js-group-access-requests-list{ data: { members_data: group_members_list_data_json(@group, @requesters) } }
             .loading
               .gl-spinner.gl-spinner-md

app/views/projects/project_members/index.html.haml

@@ -82,21 +82,21 @@
             %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm= @requesters.count
     .tab-content
       #tab-members.tab-pane{ class: ('active' unless groups_tab_active?) }
-        .js-project-members-list{ data: project_members_list_data_attributes(@project, @project_members, { param_name: :page, params: { search_groups: nil } }) }
+        .js-project-members-list{ data: { members_data: project_members_list_data_json(@project, @project_members, { param_name: :page, params: { search_groups: nil } }) } }
           .loading
             .gl-spinner.gl-spinner-md
       - if show_groups?(@group_links)
         #tab-groups.tab-pane{ class: ('active' if groups_tab_active?) }
-          .js-project-group-links-list{ data: project_group_links_list_data_attributes(@project, @group_links) }
+          .js-project-group-links-list{ data: { members_data: project_group_links_list_data_json(@project, @group_links) } }
             .loading
               .gl-spinner.gl-spinner-md
       - if show_invited_members?(@project, @invited_members)
         #tab-invited-members.tab-pane
-          .js-project-invited-members-list{ data: project_members_list_data_attributes(@project, @invited_members) }
+          .js-project-invited-members-list{ data: { members_data: project_members_list_data_json(@project, @invited_members) } }
             .loading
               .gl-spinner.gl-spinner-md
       - if show_access_requests?(@project, @requesters)
         #tab-access-requests.tab-pane
-          .js-project-access-requests-list{ data: project_members_list_data_attributes(@project, @requesters) }
+          .js-project-access-requests-list{ data: { members_data: project_members_list_data_json(@project, @requesters) } }
             .loading
               .gl-spinner.gl-spinner-md

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

 import { __ } from '~/locale';
-import {
-  generateBadges as CEGenerateBadges,
-  parseDataAttributes as CEParseDataAttributes,
-  isDirectMember,
-} from '~/members/utils';
+import { generateBadges as CEGenerateBadges, isDirectMember } from '~/members/utils';
 
 export {
   isGroup,
@@ -39,12 +35,3 @@ export const generateBadges = ({ member, isCurrentUser, canManageMembers }) => [
 ];
 
 export const canOverride = (member) => member.canOverride && isDirectMember(member);
-
-export const parseDataAttributes = (el) => {
-  const { ldapOverridePath } = el.dataset;
-
-  return {
-    ...CEParseDataAttributes(el),
-    ldapOverridePath,
-  };
-};

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

@@ -8,8 +8,8 @@ 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, _pagination = {})
+  override :group_members_list_data
+  def group_members_list_data(group, _members, _pagination = {})
     super.merge!({
       ldap_override_path: override_group_group_member_path(group, ':id')
     })

ee/spec/frontend/members/index_spec.js

-import { membersJsonString } from 'jest/members/mock_data';
+import { dataAttribute } from 'jest/members/mock_data';
 import { MEMBER_TYPES } from '~/members/constants';
 import { initMembersApp } from '~/members/index';
 
@@ -14,10 +14,7 @@ describe('initMembersApp', () => {
 
   beforeEach(() => {
     el = document.createElement('div');
-    el.setAttribute('data-members', membersJsonString);
-    el.setAttribute('data-source-id', '234');
-    el.setAttribute('data-member-path', '/groups/foo-bar/-/group_members/:id');
-    el.setAttribute('data-ldap-override-path', '/groups/ldap-group/-/group_members/:id/override');
+    el.setAttribute('data-members-data', dataAttribute);
   });
 
   afterEach(() => {

ee/spec/frontend/members/utils_spec.js

-import { generateBadges, canOverride, parseDataAttributes } from 'ee/members/utils';
-import {
-  member as memberMock,
-  directMember,
-  inheritedMember,
-  membersJsonString,
-  members,
-  paginationJsonString,
-  pagination,
-} from 'jest/members/mock_data';
+import { generateBadges, canOverride } from 'ee/members/utils';
+import { member as memberMock, directMember, inheritedMember } from 'jest/members/mock_data';
 
 describe('Members Utils', () => {
   describe('generateBadges', () => {
@@ -53,36 +45,4 @@ describe('Members Utils', () => {
       expect(canOverride(member)).toBe(expected);
     });
   });
-
-  describe('group member utils', () => {
-    describe('parseDataAttributes', () => {
-      let el;
-
-      beforeEach(() => {
-        el = document.createElement('div');
-        el.setAttribute('data-members', membersJsonString);
-        el.setAttribute('data-pagination', paginationJsonString);
-        el.setAttribute('data-source-id', '234');
-        el.setAttribute('data-can-manage-members', 'true');
-        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,
-          pagination,
-          sourceId: 234,
-          canManageMembers: true,
-          ldapOverridePath: '/groups/ldap-group/-/group_members/:id/override',
-        });
-      });
-    });
-  });
 });

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

@@ -22,15 +22,17 @@ RSpec.describe Groups::GroupMembersHelper do
     end
   end
 
-  describe '#group_members_list_data_attributes' do
+  describe '#group_members_list_data_json' do
+    subject { Gitlab::Json.parse(helper.group_members_list_data_json(group, [])) }
+
     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')
       allow(helper).to receive(:can?).with(current_user, :admin_group_member, group).and_return(true)
     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')
+    it 'adds `ldap_override_path` to returned json' do
+      expect(subject['ldap_override_path']).to eq('/groups/foo-bar/-/group_members/:id/override')
     end
   end
 end

spec/frontend/members/index_spec.js

@@ -2,7 +2,7 @@ import { createWrapper } from '@vue/test-utils';
 import MembersApp from '~/members/components/app.vue';
 import { MEMBER_TYPES } from '~/members/constants';
 import { initMembersApp } from '~/members/index';
-import { membersJsonString, members, paginationJsonString, pagination } from './mock_data';
+import { members, pagination, dataAttribute } from './mock_data';
 
 describe('initMembersApp', () => {
   let el;
@@ -23,11 +23,7 @@ describe('initMembersApp', () => {
 
   beforeEach(() => {
     el = document.createElement('div');
-    el.setAttribute('data-members', membersJsonString);
-    el.setAttribute('data-pagination', paginationJsonString);
-    el.setAttribute('data-source-id', '234');
-    el.setAttribute('data-can-manage-members', 'true');
-    el.setAttribute('data-member-path', '/groups/foo-bar/-/group_members/:id');
+    el.setAttribute('data-members-data', dataAttribute);
 
     window.gon = { current_user_id: 123 };
   });

spec/frontend/members/mock_data.js

@@ -80,13 +80,13 @@ export const inheritedMember = { ...member, isDirectMember: false };
 
 export const member2faEnabled = { ...member, user: { ...member.user, twoFactorEnabled: true } };
 
-export const paginationJsonString = JSON.stringify({
+export const paginationData = {
   current_page: 1,
   per_page: 5,
   total_items: 10,
   param_name: 'page',
   params: { search_groups: null },
-});
+};
 
 export const pagination = {
   currentPage: 1,
@@ -95,3 +95,12 @@ export const pagination = {
   paramName: 'page',
   params: { search_groups: null },
 };
+
+export const dataAttribute = JSON.stringify({
+  members,
+  pagination: paginationData,
+  source_id: 234,
+  can_manage_members: true,
+  member_path: '/groups/foo-bar/-/group_members/:id',
+  ldap_override_path: '/groups/ldap-group/-/group_members/:id/override',
+});

spec/frontend/members/utils_spec.js

@@ -20,10 +20,9 @@ import {
   member2faEnabled,
   group,
   invite,
-  membersJsonString,
   members,
-  paginationJsonString,
   pagination,
+  dataAttribute,
 } from './mock_data';
 
 const IS_CURRENT_USER_ID = 123;
@@ -260,22 +259,20 @@ describe('Members Utils', () => {
 
     beforeEach(() => {
       el = document.createElement('div');
-      el.setAttribute('data-members', membersJsonString);
-      el.setAttribute('data-pagination', paginationJsonString);
-      el.setAttribute('data-source-id', '234');
-      el.setAttribute('data-can-manage-members', 'true');
+      el.setAttribute('data-members-data', dataAttribute);
     });
 
     afterEach(() => {
       el = null;
     });
 
-    it('correctly parses the data attributes', () => {
-      expect(parseDataAttributes(el)).toEqual({
+    it('correctly parses the data attribute', () => {
+      expect(parseDataAttributes(el)).toMatchObject({
         members,
         pagination,
         sourceId: 234,
         canManageMembers: true,
+        memberPath: '/groups/foo-bar/-/group_members/:id',
       });
     });
   });

spec/helpers/groups/group_members_helper_spec.rb

@@ -23,122 +23,119 @@ RSpec.describe Groups::GroupMembersHelper do
     end
   end
 
-  describe '#group_group_links_data_json' do
-    include_context 'group_group_link'
+  describe '#group_members_list_data_json' do
+    let(:group_members) { create_list(:group_member, 2, group: group, created_by: current_user) }
 
-    it 'matches json schema' do
-      json = helper.group_group_links_data_json(shared_group.shared_with_group_links)
+    let(:pagination) { {} }
+    let(:collection) { group_members }
+    let(:presented_members) { present_members(collection) }
 
-      expect(json).to match_schema('group_link/group_group_links')
-    end
-  end
+    subject { Gitlab::Json.parse(helper.group_members_list_data_json(group, presented_members, pagination)) }
 
-  describe '#members_data_json' do
     shared_examples 'members.json' do
-      it 'matches json schema' do
-        json = helper.members_data_json(group, present_members([group_member]))
-
-        expect(json).to match_schema('members')
+      it 'returns `members` property that matches json schema' do
+        expect(subject['members'].to_json).to match_schema('members')
       end
     end
 
-    context 'for a group member' 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')
+      allow(helper).to receive(:can?).with(current_user, :admin_group_member, group).and_return(true)
+    end
+
+    it 'returns expected json' do
+      expected = {
+        member_path: '/groups/foo-bar/-/group_members/:id',
+        source_id: group.id,
+        can_manage_members: true
+      }.as_json
 
+      expect(subject).to include(expected)
+    end
+
+    context 'for a group member' do
       it_behaves_like 'members.json'
 
       context 'with user status set' do
         let(:user) { create(:user) }
         let!(:status) { create(:user_status, user: user) }
-        let(:group_member) { create(:group_member, group: group, user: user, created_by: current_user) }
+        let(:group_members) { [create(:group_member, group: group, user: user, created_by: current_user)] }
 
         it_behaves_like 'members.json'
       end
     end
 
     context 'for an invited group member' do
-      let(:group_member) { create(:group_member, :invited, group: group, created_by: current_user) }
+      let(:group_members) { create_list(:group_member, 2, :invited, group: group, created_by: current_user) }
 
       it_behaves_like 'members.json'
     end
 
     context 'for an access request' do
-      let(:group_member) { create(:group_member, :access_request, group: group, created_by: current_user) }
+      let(:group_members) { create_list(:group_member, 2, :access_request, group: group, created_by: current_user) }
 
       it_behaves_like 'members.json'
     end
-  end
-
-  describe '#group_members_list_data_attributes' do
-    let_it_be(:group_members) { create_list(:group_member, 2, 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')
-      allow(helper).to receive(:can?).with(current_user, :admin_group_member, group).and_return(true)
-    end
-
-    it 'returns expected hash' do
-      expect(helper.group_members_list_data_attributes(group, present_members(group_members))).to include({
-        members: helper.members_data_json(group, present_members(group_members)),
-        member_path: '/groups/foo-bar/-/group_members/:id',
-        source_id: group.id,
-        can_manage_members: 'true'
-      })
-    end
 
     context 'when pagination is not available' do
       it 'sets `pagination` attribute to expected json' do
-        expect(helper.group_members_list_data_attributes(group, present_members(group_members))[:pagination]).to match({
+        expected = {
           current_page: nil,
           per_page: nil,
           total_items: 2,
           param_name: nil,
           params: {}
-        }.to_json)
+        }.as_json
+
+        expect(subject['pagination']).to include(expected)
       end
     end
 
     context 'when pagination is available' do
       let(:collection) { Kaminari.paginate_array(group_members).page(1).per(1) }
+      let(:pagination) { { param_name: :page, params: { search_groups: nil } } }
 
       it 'sets `pagination` attribute to expected json' do
-        expect(
-          helper.group_members_list_data_attributes(
-            group,
-            present_members(collection),
-            { param_name: :page, params: { search_groups: nil } }
-          )[:pagination]
-        ).to match({
+        expected = {
           current_page: 1,
           per_page: 1,
           total_items: 2,
           param_name: :page,
           params: { search_groups: nil }
-        }.to_json)
+        }.as_json
+
+        expect(subject['pagination']).to include(expected)
       end
     end
   end
 
-  describe '#group_group_links_list_data_attributes' do
+  describe '#group_group_links_list_data_json' do
     include_context 'group_group_link'
 
+    subject { Gitlab::Json.parse(helper.group_group_links_list_data_json(shared_group)) }
+
     before do
       allow(helper).to receive(:group_group_link_path).with(shared_group, ':id').and_return('/groups/foo-bar/-/group_links/:id')
     end
 
-    it 'returns expected hash' do
-      expect(helper.group_group_links_list_data_attributes(shared_group)).to include({
+    it 'returns expected json' do
+      expected = {
         pagination: {
           current_page: nil,
           per_page: nil,
           total_items: 1,
           param_name: nil,
           params: {}
-        }.to_json,
-        members: helper.group_group_links_data_json(shared_group.shared_with_group_links),
+        },
         member_path: '/groups/foo-bar/-/group_links/:id',
         source_id: shared_group.id
-      })
+      }.as_json
+
+      expect(subject).to include(expected)
+    end
+
+    it 'returns `members` property that matches json schema' do
+      expect(subject['members'].to_json).to match_schema('group_link/group_group_links')
     end
   end
 end

spec/helpers/projects/project_members_helper_spec.rb

@@ -149,57 +149,61 @@ RSpec.describe Projects::ProjectMembersHelper do
   describe 'project members' do
     let_it_be(:project_members) { create_list(:project_member, 2, project: project) }
 
-    describe '#project_members_data_json' do
-      it 'matches json schema' do
-        expect(helper.project_members_data_json(project, present_members(project_members))).to match_schema('members')
-      end
-    end
+    let(:collection) { project_members }
+    let(:presented_members) { present_members(collection) }
 
-    describe '#project_members_list_data_attributes' do
+    describe '#project_members_list_data_json' do
       let(:allow_admin_project) { true }
+      let(:pagination) { {} }
+
+      subject { Gitlab::Json.parse(helper.project_members_list_data_json(project, presented_members, pagination)) }
 
       before do
         allow(helper).to receive(:project_project_member_path).with(project, ':id').and_return('/foo-bar/-/project_members/:id')
       end
 
-      it 'returns expected hash' do
-        expect(helper.project_members_list_data_attributes(project, present_members(project_members))).to include({
-          members: helper.project_members_data_json(project, present_members(project_members)),
+      it 'returns expected json' do
+        expected = {
           member_path: '/foo-bar/-/project_members/:id',
           source_id: project.id,
-          can_manage_members: 'true'
-        })
+          can_manage_members: true
+        }.as_json
+
+        expect(subject).to include(expected)
+      end
+
+      it 'returns `members` property that matches json schema' do
+        expect(subject['members'].to_json).to match_schema('members')
       end
 
       context 'when pagination is not available' do
         it 'sets `pagination` attribute to expected json' do
-          expect(helper.project_members_list_data_attributes(project, present_members(project_members))[:pagination]).to match({
+          expected = {
             current_page: nil,
             per_page: nil,
             total_items: 2,
             param_name: nil,
             params: {}
-          }.to_json)
+          }.as_json
+
+          expect(subject['pagination']).to include(expected)
         end
       end
 
       context 'when pagination is available' do
         let(:collection) { Kaminari.paginate_array(project_members).page(1).per(1) }
+        let(:pagination) { { param_name: :page, params: { search_groups: nil } } }
 
         it 'sets `pagination` attribute to expected json' do
-          expect(
-            helper.project_members_list_data_attributes(
-              project,
-              present_members(collection),
-              { param_name: :page, params: { search_groups: nil } }
-            )[:pagination]
-          ).to match({
+          expected = {
             current_page: 1,
             per_page: 1,
             total_items: 2,
             param_name: :page,
             params: { search_groups: nil }
-          }.to_json)
+          }.as_json
+
+          expect(subject['pagination']).to match(expected)
         end
       end
     end
@@ -210,32 +214,33 @@ RSpec.describe Projects::ProjectMembersHelper do
 
     let(:allow_admin_project) { true }
 
-    describe '#project_group_links_data_json' do
-      it 'matches json schema' do
-        expect(helper.project_group_links_data_json(project_group_links)).to match_schema('group_link/project_group_links')
-      end
-    end
+    describe '#project_group_links_list_data_json' do
+      subject { Gitlab::Json.parse(helper.project_group_links_list_data_json(project, project_group_links)) }
 
-    describe '#project_group_links_list_data_attributes' do
       before do
         allow(helper).to receive(:project_group_link_path).with(project, ':id').and_return('/foo-bar/-/group_links/:id')
         allow(helper).to receive(:can?).with(current_user, :admin_project_member, project).and_return(true)
       end
 
-      it 'returns expected hash' do
-        expect(helper.project_group_links_list_data_attributes(project, project_group_links)).to include({
-          members: helper.project_group_links_data_json(project_group_links),
+      it 'returns expected json' do
+        expected = {
           pagination: {
             current_page: nil,
             per_page: nil,
             total_items: 1,
             param_name: nil,
             params: {}
-          }.to_json,
+          },
           member_path: '/foo-bar/-/group_links/:id',
           source_id: project.id,
-          can_manage_members: 'true'
-        })
+          can_manage_members: true
+        }.as_json
+
+        expect(subject).to include(expected)
+      end
+
+      it 'returns `members` property that matches json schema' do
+        expect(subject['members'].to_json).to match_schema('group_link/project_group_links')
       end
     end
   end