Commit 3537786f authored by peterhegman's avatar peterhegman

Refactor passed members data to one JSON data attribute

Instead of multiple data attributes
parent faee533c
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';
......
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 { data } = 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(data), {
deep: true,
ignoreKeyNames: ['params'],
});
};
export const baseRequestFormatter = (basePropertyName, accessLevelPropertyName) => ({
......
......@@ -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
}
......
......@@ -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
......@@ -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
......@@ -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: { 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: { 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: { 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: { data: group_members_list_data_json(@group, @requesters) } }
.loading
.gl-spinner.gl-spinner-md
......@@ -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: { 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: { 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: { 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: { data: project_members_list_data_json(@project, @requesters) } }
.loading
.gl-spinner.gl-spinner-md
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,
};
};
......@@ -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')
})
......
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-data', dataAttribute);
});
afterEach(() => {
......
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',
});
});
});
});
});
......@@ -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
......@@ -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-data', dataAttribute);
window.gon = { current_user_id: 123 };
});
......
......@@ -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',
});
......@@ -20,10 +20,9 @@ import {
member2faEnabled,
group,
invite,
membersJsonString,
members,
paginationJsonString,
pagination,
dataAttribute,
} from './mock_data';
const IS_CURRENT_USER_ID = 123;
......@@ -260,23 +259,23 @@ 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-data', dataAttribute);
});
afterEach(() => {
el = null;
});
it('correctly parses the data attributes', () => {
expect(parseDataAttributes(el)).toEqual({
members,
pagination,
sourceId: 234,
canManageMembers: true,
});
it('correctly parses the data attribute', () => {
expect(parseDataAttributes(el)).toEqual(
expect.objectContaining({
members,
pagination,
sourceId: 234,
canManageMembers: true,
memberPath: '/groups/foo-bar/-/group_members/:id',
}),
);
});
});
......
......@@ -23,20 +23,20 @@ RSpec.describe Groups::GroupMembersHelper do
end
end
describe '#group_group_links_data_json' do
describe '#group_group_links_serialized' do
include_context 'group_group_link'
it 'matches json schema' do
json = helper.group_group_links_data_json(shared_group.shared_with_group_links)
json = helper.send(:group_group_links_serialized, shared_group.shared_with_group_links).to_json
expect(json).to match_schema('group_link/group_group_links')
end
end
describe '#members_data_json' do
describe '#group_members_serialized' do
shared_examples 'members.json' do
it 'matches json schema' do
json = helper.members_data_json(group, present_members([group_member]))
json = helper.send(:group_members_serialized, group, present_members([group_member])).to_json
expect(json).to match_schema('members')
end
......@@ -69,76 +69,87 @@ RSpec.describe Groups::GroupMembersHelper do
end
end
describe '#group_members_list_data_attributes' do
describe '#group_members_list_data_json' do
let_it_be(:group_members) { create_list(:group_member, 2, group: group, created_by: current_user) }
let(:pagination) { {} }
let(:collection) { group_members }
let(:presented_members) { present_members(collection) }
subject { Gitlab::Json.parse(helper.group_members_list_data_json(group, presented_members, pagination)) }
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)),
it 'returns expected json' do
expected = {
members: helper.send(:group_members_serialized, group, presented_members),
member_path: '/groups/foo-bar/-/group_members/:id',
source_id: group.id,
can_manage_members: 'true'
})
can_manage_members: true
}.as_json
expect(subject).to include(expected)
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),
},
members: helper.send(:group_group_links_serialized, 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
end
end
......@@ -149,57 +149,64 @@ 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
let(:collection) { project_members }
let(:presented_members) { present_members(collection) }
describe '#project_members_serialized' do
it 'matches json schema' do
expect(helper.project_members_data_json(project, present_members(project_members))).to match_schema('members')
expect(helper.send(:project_members_serialized, project, presented_members).to_json).to match_schema('members')
end
end
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 = {
members: helper.send(:project_members_serialized, project, presented_members),
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
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
......@@ -212,30 +219,34 @@ RSpec.describe Projects::ProjectMembersHelper do
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')
expect(helper.send(:project_group_links_serialized, project_group_links).to_json).to match_schema('group_link/project_group_links')
end
end
describe '#project_group_links_list_data_attributes' do
describe '#project_group_links_list_data_json' do
subject { Gitlab::Json.parse(helper.project_group_links_list_data_json(project, project_group_links)) }
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 = {
members: helper.send(:project_group_links_serialized, project_group_links),
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
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment