Skip to content

G
gitlab-ce
Commit 22cd7dc4 authored by Peter Hegman's avatar Peter Hegman
Browse files
Options

Merge branch '350999-invite-modal-using-invitations-api' into 'master' 

Allow invitations API to handle user invites as well as emails

See merge request gitlab-org/gitlab!80733
parents cc977bb1 3f7c9ba7
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 290 additions and 376 deletions
app/assets/javascripts/api.js
View file @ 22cd7dc4
......@@ -156,13 +156,7 @@ const Api = {
});
},
addGroupMembersByUserId(id, data) {
const url = Api.buildUrl(this.groupMembersPath).replace(':id', encodeURIComponent(id));
return axios.post(url, data);
},
inviteGroupMembersByEmail(id, data) {
inviteGroupMembers(id, data) {
const url = Api.buildUrl(this.groupInvitationsPath).replace(':id', encodeURIComponent(id));
return axios.post(url, data);
......@@ -258,13 +252,7 @@ const Api = {
.then(({ data }) => data);
},
addProjectMembersByUserId(id, data) {
const url = Api.buildUrl(this.projectMembersPath).replace(':id', encodeURIComponent(id));
return axios.post(url, data);
},
inviteProjectMembersByEmail(id, data) {
inviteProjectMembers(id, data) {
const url = Api.buildUrl(this.projectInvitationsPath).replace(':id', encodeURIComponent(id));
return axios.post(url, data);
......
app/assets/javascripts/invite_members/components/invite_members_modal.vue
View file @ 22cd7dc4
......@@ -193,46 +193,28 @@ export default {
this.invalidFeedbackMessage = '';
const [usersToInviteByEmail, usersToAddById] = this.partitionNewUsersToInvite();
const promises = [];
const baseData = {
const apiAddByInvite = this.isProject
? Api.inviteProjectMembers.bind(Api)
: Api.inviteGroupMembers.bind(Api);
const email = usersToInviteByEmail !== '' ? { email: usersToInviteByEmail } : {};
const userId = usersToAddById !== '' ? { user_id: usersToAddById } : {};
this.trackinviteMembersForTask();
apiAddByInvite(this.id, {
format: 'json',
expires_at: expiresAt,
access_level: accessLevel,
invite_source: this.source,
tasks_to_be_done: this.tasksToBeDoneForPost,
tasks_project_id: this.tasksProjectForPost,
};
if (usersToInviteByEmail !== '') {
const apiInviteByEmail = this.isProject
? Api.inviteProjectMembersByEmail.bind(Api)
: Api.inviteGroupMembersByEmail.bind(Api);
promises.push(
apiInviteByEmail(this.id, {
...baseData,
email: usersToInviteByEmail,
}),
);
}
if (usersToAddById !== '') {
const apiAddByUserId = this.isProject
? Api.addProjectMembersByUserId.bind(Api)
: Api.addGroupMembersByUserId.bind(Api);
promises.push(
apiAddByUserId(this.id, {
...baseData,
user_id: usersToAddById,
}),
);
}
this.trackinviteMembersForTask();
Promise.all(promises)
.then((responses) => {
const message = responseMessageFromSuccess(responses);
...email,
...userId,
})
.then((response) => {
const message = responseMessageFromSuccess(response);
if (message) {
this.showInvalidFeedbackMessage({
......
app/assets/javascripts/invite_members/constants.js
View file @ 22cd7dc4
import { __, s__ } from '~/locale';
import { s__ } from '~/locale';
export const SEARCH_DELAY = 200;
......@@ -14,9 +14,6 @@ export const GROUP_FILTERS = {
DESCENDANT_GROUPS: 'descendant_groups',
};
export const API_MESSAGES = {
EMAIL_ALREADY_INVITED: __('Invite email has already been taken'),
};
export const USERS_FILTER_ALL = 'all';
export const USERS_FILTER_SAML_PROVIDER_ID = 'saml_provider_id';
export const TRIGGER_ELEMENT_BUTTON = 'button';
......
app/assets/javascripts/invite_members/utils/response_message_parser.js
View file @ 22cd7dc4
import { isString } from 'lodash';
import { API_MESSAGES } from '~/invite_members/constants';
function responseKeyedMessageParsed(keyedMessage) {
try {
const keys = Object.keys(keyedMessage);
const msg = keyedMessage[keys[0]];
if (msg === API_MESSAGES.EMAIL_ALREADY_INVITED) {
return '';
}
return msg;
} catch {
return '';
}
}
function responseMessageStringForMultiple(message) {
return message.includes(':');
}
function responseMessageStringFirstPart(message) {
const firstPart = message.split(':')[1];
const firstMsg = firstPart.split(/ and [\w-]*$/)[0].trim();
return firstMsg;
}
export function responseMessageFromError(response) {
if (!response?.response?.data) {
......@@ -33,36 +20,25 @@ export function responseMessageFromError(response) {
response: { data },
} = response;
return (
data.error ||
data.message?.user?.[0] ||
data.message?.access_level?.[0] ||
data.message?.error ||
data.message ||
''
);
return data.error || data.message?.error || data.message || '';
}
export function responseMessageFromSuccess(response) {
if (!response?.[0]?.data) {
if (!response?.data) {
return '';
}
const { data } = response[0];
const { data } = response;
if (data.message && !data.message.user) {
if (data.message) {
const { message } = data;
if (isString(message)) {
if (responseMessageStringForMultiple(message)) {
return responseMessageStringFirstPart(message);
}
return message;
}
return responseKeyedMessageParsed(message);
}
return data.message || data.message?.user || data.error || '';
return data.error || '';
}
locale/gitlab.pot
View file @ 22cd7dc4
......@@ -20605,9 +20605,6 @@ msgstr ""
msgid "Invite a group"
msgstr ""
msgid "Invite email has already been taken"
msgstr ""
msgid "Invite members"
msgstr ""
......
spec/features/groups/members/manage_members_spec.rb
View file @ 22cd7dc4
......@@ -42,27 +42,6 @@ RSpec.describe 'Groups > Members > Manage members' do
end
end
it 'add user to group', :js, :snowplow, :aggregate_failures do
group.add_owner(user1)
visit group_group_members_path(group)
invite_member(user2.name, role: 'Reporter')
page.within(second_row) do
expect(page).to have_content(user2.name)
expect(page).to have_button('Reporter')
end
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'group-members-page',
property: 'existing_user',
user: user1
)
end
it 'remove user from group', :js do
group.add_owner(user1)
group.add_developer(user2)
......@@ -87,43 +66,29 @@ RSpec.describe 'Groups > Members > Manage members' do
end
end
it 'add yourself to group when already an owner', :js, :aggregate_failures do
group.add_owner(user1)
visit group_group_members_path(group)
invite_member(user1.name, role: 'Reporter')
page.within(first_row) do
expect(page).to have_content(user1.name)
expect(page).to have_content('Owner')
end
end
context 'when inviting' do
it 'add yourself to group when already an owner', :js do
group.add_owner(user1)
it 'invite user to group', :js, :snowplow do
group.add_owner(user1)
visit group_group_members_path(group)
visit group_group_members_path(group)
invite_member(user1.name, role: 'Reporter', refresh: false)
invite_member('test@example.com', role: 'Reporter')
expect(page).to have_selector(invite_modal_selector)
expect(page).to have_content("not authorized to update member")
expect(page).to have_link 'Invited'
click_link 'Invited'
page.refresh
aggregate_failures do
page.within(members_table) do
expect(page).to have_content('test@example.com')
expect(page).to have_content('Invited')
expect(page).to have_button('Reporter')
page.within find_member_row(user1) do
expect(page).to have_content('Owner')
end
end
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: 'group-members-page',
property: 'net_new_user',
user: user1
)
it_behaves_like 'inviting members', 'group-members-page' do
let_it_be(:entity) { group }
let_it_be(:members_page_path) { group_group_members_path(entity) }
let_it_be(:subentity) { create(:group, parent: group) }
let_it_be(:subentity_members_page_path) { group_group_members_path(subentity) }
end
end
......
spec/features/projects/members/manage_members_spec.rb
View file @ 22cd7dc4
......@@ -48,24 +48,6 @@ RSpec.describe 'Projects > Members > Manage members', :js do
end
end
it 'add user to project', :snowplow, :aggregate_failures do
visit_members_page
invite_member(user2.name, role: 'Reporter')
page.within find_member_row(user2) do
expect(page).to have_button('Reporter')
end
expect_snowplow_event(
category: 'Members::CreateService',
action: 'create_member',
label: 'project-members-page',
property: 'existing_user',
user: user1
)
end
it 'uses ProjectMember access_level_roles for the invite members modal access option', :aggregate_failures do
visit_members_page
......@@ -104,24 +86,11 @@ RSpec.describe 'Projects > Members > Manage members', :js do
expect(members_table).not_to have_content(other_user.name)
end
it 'invite user to project', :snowplow, :aggregate_failures do
visit_members_page
invite_member('test@example.com', role: 'Reporter')
click_link 'Invited'
page.within find_invited_member_row('test@example.com') do
expect(page).to have_button('Reporter')
end
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: 'project-members-page',
property: 'net_new_user',
user: user1
)
it_behaves_like 'inviting members', 'project-members-page' do
let_it_be(:entity) { project }
let_it_be(:members_page_path) { project_project_members_path(entity) }
let_it_be(:subentity) { project }
let_it_be(:subentity_members_page_path) { project_project_members_path(entity) }
end
describe 'member search results' do
......
spec/frontend/api_spec.js
View file @ 22cd7dc4
......@@ -187,36 +187,15 @@ describe('Api', () => {
});
});
describe('addGroupMembersByUserId', () => {
it('adds an existing User as a new Group Member by User ID', () => {
const groupId = 1;
const expectedUserId = 2;
const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/1/members`;
const params = {
user_id: expectedUserId,
access_level: 10,
expires_at: undefined,
};
mock.onPost(expectedUrl).reply(200, {
id: expectedUserId,
state: 'active',
});
return Api.addGroupMembersByUserId(groupId, params).then(({ data }) => {
expect(data.id).toBe(expectedUserId);
expect(data.state).toBe('active');
});
});
});
describe('inviteGroupMembersByEmail', () => {
describe('inviteGroupMembers', () => {
it('invites a new email address to create a new User and become a Group Member', () => {
const groupId = 1;
const email = 'email@example.com';
const userId = '1';
const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/1/invitations`;
const params = {
email,
userId,
access_level: 10,
expires_at: undefined,
};
......@@ -225,7 +204,7 @@ describe('Api', () => {
status: 'success',
});
return Api.inviteGroupMembersByEmail(groupId, params).then(({ data }) => {
return Api.inviteGroupMembers(groupId, params).then(({ data }) => {
expect(data.status).toBe('success');
});
});
......@@ -543,36 +522,15 @@ describe('Api', () => {
});
});
describe('addProjectMembersByUserId', () => {
it('adds an existing User as a new Project Member by User ID', () => {
const projectId = 1;
const expectedUserId = 2;
const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/1/members`;
const params = {
user_id: expectedUserId,
access_level: 10,
expires_at: undefined,
};
mock.onPost(expectedUrl).reply(200, {
id: expectedUserId,
state: 'active',
});
return Api.addProjectMembersByUserId(projectId, params).then(({ data }) => {
expect(data.id).toBe(expectedUserId);
expect(data.state).toBe('active');
});
});
});
describe('inviteProjectMembersByEmail', () => {
describe('inviteProjectMembers', () => {
it('invites a new email address to create a new User and become a Project Member', () => {
const projectId = 1;
const expectedEmail = 'email@example.com';
const email = 'email@example.com';
const userId = '1';
const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/1/invitations`;
const params = {
email: expectedEmail,
email,
userId,
access_level: 10,
expires_at: undefined,
};
......@@ -581,7 +539,7 @@ describe('Api', () => {
status: 'success',
});
return Api.inviteProjectMembersByEmail(projectId, params).then(({ data }) => {
return Api.inviteProjectMembers(projectId, params).then(({ data }) => {
expect(data.status).toBe('success');
});
});
......
spec/frontend/invite_members/mock_data/api_responses.js
View file @ 22cd7dc4
const INVITATIONS_API_EMAIL_INVALID = {
const EMAIL_INVALID = {
message: { error: 'email contains an invalid email address' },
};
const INVITATIONS_API_ERROR_EMAIL_INVALID = {
const ERROR_EMAIL_INVALID = {
error: 'email contains an invalid email address',
};
const INVITATIONS_API_EMAIL_RESTRICTED = {
const EMAIL_RESTRICTED = {
message: {
'email@example.com':
"The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
......@@ -14,65 +14,31 @@ const INVITATIONS_API_EMAIL_RESTRICTED = {
status: 'error',
};
const INVITATIONS_API_MULTIPLE_EMAIL_RESTRICTED = {
const MULTIPLE_RESTRICTED = {
message: {
'email@example.com':
"The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
'email4@example.com':
"The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check the Domain denylist.",
},
status: 'error',
};
const INVITATIONS_API_EMAIL_TAKEN = {
message: {
'email@example.org': 'Invite email has already been taken',
},
status: 'error',
};
const MEMBERS_API_MEMBER_ALREADY_EXISTS = {
message: 'Member already exists',
};
const MEMBERS_API_SINGLE_USER_RESTRICTED = {
message: {
user: [
root:
"The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
],
},
status: 'error',
};
const MEMBERS_API_SINGLE_USER_ACCESS_LEVEL = {
const EMAIL_TAKEN = {
message: {
access_level: [
'should be greater than or equal to Owner inherited membership from group Gitlab Org',
],
'email@example.org': "The member's email address has already been taken",
},
};
const MEMBERS_API_MULTIPLE_USERS_RESTRICTED = {
message:
"root: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups. and user18: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check the Domain denylist. and john_doe31: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Email restrictions for sign-ups.",
status: 'error',
};
export const apiPaths = {
GROUPS_MEMBERS: '/api/v4/groups/1/members',
GROUPS_INVITATIONS: '/api/v4/groups/1/invitations',
};
export const membersApiResponse = {
MEMBER_ALREADY_EXISTS: MEMBERS_API_MEMBER_ALREADY_EXISTS,
SINGLE_USER_ACCESS_LEVEL: MEMBERS_API_SINGLE_USER_ACCESS_LEVEL,
SINGLE_USER_RESTRICTED: MEMBERS_API_SINGLE_USER_RESTRICTED,
MULTIPLE_USERS_RESTRICTED: MEMBERS_API_MULTIPLE_USERS_RESTRICTED,
};
export const GROUPS_INVITATIONS_PATH = '/api/v4/groups/1/invitations';
export const invitationsApiResponse = {
EMAIL_INVALID: INVITATIONS_API_EMAIL_INVALID,
ERROR_EMAIL_INVALID: INVITATIONS_API_ERROR_EMAIL_INVALID,
EMAIL_RESTRICTED: INVITATIONS_API_EMAIL_RESTRICTED,
MULTIPLE_EMAIL_RESTRICTED: INVITATIONS_API_MULTIPLE_EMAIL_RESTRICTED,
EMAIL_TAKEN: INVITATIONS_API_EMAIL_TAKEN,
EMAIL_INVALID,
ERROR_EMAIL_INVALID,
EMAIL_RESTRICTED,
MULTIPLE_RESTRICTED,
EMAIL_TAKEN,
};
spec/frontend/invite_members/utils/response_message_parser_spec.js
View file @ 22cd7dc4
......@@ -2,23 +2,19 @@ import {
responseMessageFromSuccess,
responseMessageFromError,
} from '~/invite_members/utils/response_message_parser';
import { membersApiResponse, invitationsApiResponse } from '../mock_data/api_responses';
import { invitationsApiResponse } from '../mock_data/api_responses';
describe('Response message parser', () => {
const expectedMessage = 'expected display and message.';
describe('parse message from successful response', () => {
const exampleKeyedMsg = { 'email@example.com': expectedMessage };
const exampleFirstPartMultiple = 'username1: expected display and message.';
const exampleUserMsgMultiple =
' and username2: id not found and restricted email. and username3: email is restricted.';
it.each([
[[{ data: { message: expectedMessage } }]],
[[{ data: { message: exampleFirstPartMultiple + exampleUserMsgMultiple } }]],
[[{ data: { error: expectedMessage } }]],
[[{ data: { message: [expectedMessage] } }]],
[[{ data: { message: exampleKeyedMsg } }]],
[{ data: { message: expectedMessage } }],
[{ data: { error: expectedMessage } }],
[{ data: { message: [expectedMessage] } }],
[{ data: { message: exampleKeyedMsg } }],
])(`returns "${expectedMessage}" from success response: %j`, (successResponse) => {
expect(responseMessageFromSuccess(successResponse)).toBe(expectedMessage);
});
......@@ -27,8 +23,6 @@ describe('Response message parser', () => {
describe('message from error response', () => {
it.each([
[{ response: { data: { error: expectedMessage } } }],
[{ response: { data: { message: { user: [expectedMessage] } } } }],
[{ response: { data: { message: { access_level: [expectedMessage] } } } }],
[{ response: { data: { message: { error: expectedMessage } } } }],
[{ response: { data: { message: expectedMessage } } }],
])(`returns "${expectedMessage}" from error response: %j`, (errorResponse) => {
......@@ -41,18 +35,10 @@ describe('Response message parser', () => {
"The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.";
it.each([
[[{ data: membersApiResponse.MULTIPLE_USERS_RESTRICTED }]],
[[{ data: invitationsApiResponse.MULTIPLE_EMAIL_RESTRICTED }]],
[[{ data: invitationsApiResponse.EMAIL_RESTRICTED }]],
[{ data: invitationsApiResponse.MULTIPLE_RESTRICTED }],
[{ data: invitationsApiResponse.EMAIL_RESTRICTED }],
])(`returns "${expectedMessage}" from success response: %j`, (restrictedResponse) => {
expect(responseMessageFromSuccess(restrictedResponse)).toBe(expected);
});
it.each([[{ response: { data: membersApiResponse.SINGLE_USER_RESTRICTED } }]])(
`returns "${expectedMessage}" from error response: %j`,
(singleRestrictedResponse) => {
expect(responseMessageFromError(singleRestrictedResponse)).toBe(expected);
},
);
});
});
spec/support/helpers/features/invite_members_modal_helper.rb
View file @ 22cd7dc4
......@@ -5,19 +5,22 @@ module Spec
module Helpers
module Features
module InviteMembersModalHelper
def invite_member(name, role: 'Guest', expires_at: nil)
def invite_member(names, role: 'Guest', expires_at: nil, refresh: true)
click_on 'Invite members'
page.within invite_modal_selector do
find(member_dropdown_selector).set(name)
Array.wrap(names).each do |name|
find(member_dropdown_selector).set(name)
wait_for_requests
click_button name
end
wait_for_requests
click_button name
choose_options(role, expires_at)
click_button 'Invite'
page.refresh
page.refresh if refresh
end
end
......
spec/support/shared_examples/features/inviting_members_shared_examples.rb 0 → 100644
View file @ 22cd7dc4
# frozen_string_literal: true
RSpec.shared_examples 'inviting members' do |snowplow_invite_label|
before_all do
group.add_owner(user1)
end
it 'adds user as member', :js, :snowplow, :aggregate_failures do
visit members_page_path
invite_member(user2.name, role: 'Reporter')
page.within find_member_row(user2) do
expect(page).to have_button('Reporter')
end
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: snowplow_invite_label,
property: 'existing_user',
user: user1
)
end
it 'invites user by email', :js, :snowplow, :aggregate_failures do
visit members_page_path
invite_member('test@example.com', role: 'Reporter')
click_link 'Invited'
page.within find_invited_member_row('test@example.com') do
expect(page).to have_button('Reporter')
end
expect_snowplow_event(
category: 'Members::InviteService',
action: 'create_member',
label: snowplow_invite_label,
property: 'net_new_user',
user: user1
)
end
it 'invites user by username and invites user by email', :js, :aggregate_failures do
visit members_page_path
invite_member([user2.name, 'test@example.com'], role: 'Reporter')
page.within find_member_row(user2) do
expect(page).to have_button('Reporter')
end
click_link 'Invited'
page.within find_invited_member_row('test@example.com') do
expect(page).to have_button('Reporter')
end
end
context 'when member is already a member by username' do
it 'updates the member for that user', :js do
visit members_page_path
invite_member(user2.name, role: 'Developer')
invite_member(user2.name, role: 'Reporter', refresh: false)
expect(page).not_to have_selector(invite_modal_selector)
page.refresh
page.within find_invited_member_row(user2.name) do
expect(page).to have_button('Reporter')
end
end
end
context 'when member is already a member by email' do
it 'fails with an error', :js do
visit members_page_path
invite_member('test@example.com', role: 'Developer')
invite_member('test@example.com', role: 'Reporter', refresh: false)
expect(page).to have_selector(invite_modal_selector)
expect(page).to have_content("The member's email address has already been taken")
page.refresh
click_link 'Invited'
page.within find_invited_member_row('test@example.com') do
expect(page).to have_button('Developer')
end
end
end
context 'when inviting a parent group member to the sub-entity' do
before_all do
group.add_owner(user1)
group.add_developer(user2)
end
context 'when role is higher than parent group membership' do
let(:role) { 'Maintainer' }
it 'adds the user as a member on sub-entity with higher access level', :js do
visit subentity_members_page_path
invite_member(user2.name, role: role, refresh: false)
expect(page).not_to have_selector(invite_modal_selector)
page.refresh
page.within find_invited_member_row(user2.name) do
expect(page).to have_button(role)
end
end
end
context 'when role is lower than parent group membership' do
let(:role) { 'Reporter' }
it 'fails with an error', :js do
visit subentity_members_page_path
invite_member(user2.name, role: role, refresh: false)
expect(page).to have_selector(invite_modal_selector)
expect(page).to have_content "Access level should be greater than or equal to Developer inherited membership " \
"from group #{group.name}"
page.refresh
page.within find_invited_member_row(user2.name) do
expect(page).to have_content('Developer')
expect(page).not_to have_button('Developer')
end
end
context 'when there are multiple users invited with errors' do
let_it_be(:user3) { create(:user) }
before do
group.add_maintainer(user3)
end
it 'only shows the first user error', :js do
visit subentity_members_page_path
invite_member([user2.name, user3.name], role: role, refresh: false)
expect(page).to have_selector(invite_modal_selector)
expect(page).to have_text("Access level should be greater than or equal to", count: 1)
page.refresh
page.within find_invited_member_row(user2.name) do
expect(page).to have_content('Developer')
expect(page).not_to have_button('Developer')
end
page.within find_invited_member_row(user3.name) do
expect(page).to have_content('Maintainer')
expect(page).not_to have_button('Maintainer')
end
end
end
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7