Skip to content

Commit 4d62030

Browse files
committed
feat: enhance Add New Team Member functionality with user validation and error handling
1 parent 75602b8 commit 4d62030

4 files changed

Lines changed: 113 additions & 18 deletions

File tree

src/authz-module/data/api.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('validateUsers', () => {
112112
const result = await validateUsers({ users: ['jdoe', 'unknown'] });
113113

114114
expect(mockPost).toHaveBeenCalledWith(
115-
'http://localhost:8000/api/authz/v1/users/validate',
115+
'http://localhost:8000/api/authz/v1/users/validate/',
116116
{ users: ['jdoe', 'unknown'] },
117117
);
118118
expect(result).toEqual(mockResponse);

src/authz-module/data/api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export const validateUsers = async (
9191
data: ValidateUsersRequest,
9292
): Promise<ValidateUsersResponse> => {
9393
const res = await getAuthenticatedHttpClient().post(
94-
getApiUrl('/api/authz/v1/users/validate'),
94+
getApiUrl('/api/authz/v1/users/validate/'),
9595
data,
9696
);
9797
return camelCaseObject(res.data);

src/authz-module/libraries-manager/components/AddNewTeamMemberModal/AddNewTeamMemberTrigger.test.tsx

Lines changed: 75 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
1-
import React, { act } from 'react';
1+
import { act } from 'react';
22
import { screen, waitFor } from '@testing-library/react';
33
import userEvent from '@testing-library/user-event';
44
import { renderWrapper } from '@src/setupTest';
5-
import { useAssignTeamMembersRole } from '@src/authz-module/data/hooks';
5+
import { useAssignTeamMembersRole, useValidateUsers } from '@src/authz-module/data/hooks';
66
import { ToastManagerProvider } from '@src/authz-module/libraries-manager/ToastManagerContext';
77
import AddNewTeamMemberTrigger from './AddNewTeamMemberTrigger';
88

99
jest.mock('@edx/frontend-platform/logging');
1010

1111
const mockMutate = jest.fn();
12+
const mockMutateAsync = jest.fn();
1213

1314
// Mock the hooks module
1415
jest.mock('@src/authz-module/data/hooks', () => ({
1516
useAssignTeamMembersRole: jest.fn(),
17+
useValidateUsers: jest.fn(),
1618
}));
1719

1820
jest.mock('./AddNewTeamMemberModal', () => {
@@ -54,12 +56,17 @@ describe('AddNewTeamMemberTrigger', () => {
5456

5557
beforeEach(() => {
5658
jest.clearAllMocks();
59+
mockMutateAsync.mockResolvedValue({ validUsers: [], invalidUsers: [] });
5760
(useAssignTeamMembersRole as jest.Mock).mockReturnValue({
5861
mutate: mockMutate,
5962
isPending: false,
6063
isError: false,
6164
isSuccess: false,
6265
} as any);
66+
(useValidateUsers as jest.Mock).mockReturnValue({
67+
mutateAsync: mockMutateAsync,
68+
isPending: false,
69+
} as any);
6370
});
6471

6572
it('renders the trigger button', () => {
@@ -109,7 +116,7 @@ describe('AddNewTeamMemberTrigger', () => {
109116
await user.selectOptions(roleSelect, 'editor');
110117
await user.click(saveButton);
111118

112-
expect(mockMutate).toHaveBeenCalledWith(
119+
await waitFor(() => expect(mockMutate).toHaveBeenCalledWith(
113120
{
114121
data: {
115122
@@ -120,7 +127,7 @@ describe('AddNewTeamMemberTrigger', () => {
120127
expect.objectContaining({
121128
onSuccess: expect.any(Function),
122129
}),
123-
);
130+
));
124131
});
125132

126133
it('displays success toast and closes modal on successful addition with no errors', async () => {
@@ -130,10 +137,14 @@ describe('AddNewTeamMemberTrigger', () => {
130137
const triggerButton = screen.getByRole('button', { name: /assign role/i });
131138
await user.click(triggerButton);
132139

140+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
141+
await user.type(usersInput, '[email protected], [email protected]');
142+
133143
const saveButton = screen.getByRole('button', { name: 'Save team member' });
134144
await user.click(saveButton);
135145

136-
// Simulate successful response with no errors
146+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
147+
137148
const [, { onSuccess }] = mockMutate.mock.calls[0];
138149
onSuccess({
139150
completed: [
@@ -157,10 +168,14 @@ describe('AddNewTeamMemberTrigger', () => {
157168
const triggerButton = screen.getByRole('button', { name: /assign role/i });
158169
await user.click(triggerButton);
159170

171+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
172+
await user.type(usersInput, '[email protected], [email protected]');
173+
160174
const saveButton = screen.getByRole('button', { name: 'Save team member' });
161175
await user.click(saveButton);
162176

163-
// Simulate partial success response
177+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
178+
164179
const [, { onSuccess }] = mockMutate.mock.calls[0];
165180
onSuccess({
166181
completed: [
@@ -231,10 +246,14 @@ describe('AddNewTeamMemberTrigger', () => {
231246
const triggerButton = screen.getByRole('button', { name: /assign role/i });
232247
await user.click(triggerButton);
233248

249+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
250+
await user.type(usersInput, '[email protected], [email protected]');
251+
234252
const saveButton = screen.getByRole('button', { name: 'Save team member' });
235253
await user.click(saveButton);
236254

237-
// Simulate all failed response
255+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
256+
238257
const [, { onSuccess }] = mockMutate.mock.calls[0];
239258
onSuccess({
240259
completed: [],
@@ -259,9 +278,14 @@ describe('AddNewTeamMemberTrigger', () => {
259278
const triggerButton = screen.getByRole('button', { name: /assign role/i });
260279
await user.click(triggerButton);
261280

281+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
282+
await user.type(usersInput, '[email protected], [email protected]');
283+
262284
const saveButton = screen.getByRole('button', { name: 'Save team member' });
263285
await user.click(saveButton);
264286

287+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
288+
265289
const [, { onSuccess }] = mockMutate.mock.calls[0];
266290
onSuccess({
267291
completed: [],
@@ -294,7 +318,8 @@ describe('AddNewTeamMemberTrigger', () => {
294318
await user.selectOptions(roleSelect, 'editor');
295319
await user.click(saveButton);
296320

297-
// Simulate successful response with no errors
321+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
322+
298323
const [, { onSuccess }] = mockMutate.mock.calls[0];
299324
onSuccess({
300325
completed: [{ userIdentifier: '[email protected]', status: 'role_added' }],
@@ -318,10 +343,14 @@ describe('AddNewTeamMemberTrigger', () => {
318343
const triggerButton = screen.getByRole('button', { name: /assign role/i });
319344
await user.click(triggerButton);
320345

346+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
347+
await user.type(usersInput, '[email protected]');
348+
321349
const saveButton = screen.getByRole('button', { name: 'Save team member' });
322350
await user.click(saveButton);
323351

324-
// Simulate successful response
352+
await waitFor(() => expect(mockMutate).toHaveBeenCalled());
353+
325354
const [, { onSuccess }] = mockMutate.mock.calls[0];
326355
onSuccess({
327356
completed: [{ userIdentifier: '[email protected]', status: 'role_added' }],
@@ -361,6 +390,9 @@ describe('AddNewTeamMemberTrigger', () => {
361390
const triggerButton = screen.getByRole('button', { name: /assign role/i });
362391
await user.click(triggerButton);
363392

393+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
394+
await user.type(usersInput, '[email protected]');
395+
364396
const saveButton = screen.getByRole('button', { name: 'Save team member' });
365397
await user.click(saveButton);
366398

@@ -384,6 +416,38 @@ describe('AddNewTeamMemberTrigger', () => {
384416
expect(mockMutate).toHaveBeenCalledTimes(2);
385417
});
386418

419+
it('shows error toast and highlights invalid users when validation finds unknown users', async () => {
420+
const user = userEvent.setup();
421+
422+
mockMutateAsync.mockResolvedValue({ validUsers: [], invalidUsers: ['[email protected]'] });
423+
424+
renderWrapper(<ToastManagerProvider><AddNewTeamMemberTrigger libraryId={mockLibraryId} /></ToastManagerProvider>);
425+
426+
const triggerButton = screen.getByRole('button', { name: /assign role/i });
427+
await user.click(triggerButton);
428+
429+
const usersInput = screen.getByRole('textbox', { name: 'Enter user emails or usernames' });
430+
const roleSelect = screen.getByRole('combobox', { name: 'Select role' });
431+
const saveButton = screen.getByRole('button', { name: 'Save team member' });
432+
433+
await user.type(usersInput, '[email protected]');
434+
await user.selectOptions(roleSelect, 'editor');
435+
await user.click(saveButton);
436+
437+
await waitFor(() => {
438+
expect(screen.getByText(/We couldn't find a user for 1 email address or username/)).toBeInTheDocument();
439+
});
440+
441+
// assignTeamMembersRole should NOT have been called
442+
expect(mockMutate).not.toHaveBeenCalled();
443+
444+
// Modal should remain open
445+
expect(screen.getByRole('dialog', { name: 'assign role' })).toBeInTheDocument();
446+
447+
// Input should be updated to show only invalid users
448+
expect(usersInput).toHaveValue('[email protected]');
449+
});
450+
387451
it('displays loading state when adding team member', async () => {
388452
const user = userEvent.setup();
389453

@@ -438,7 +502,7 @@ describe('AddNewTeamMemberTrigger', () => {
438502
expect(loadingIndicator).toHaveTextContent('Loading...');
439503
});
440504

441-
expect(mutateMock).toHaveBeenCalledWith(
505+
await waitFor(() => expect(mutateMock).toHaveBeenCalledWith(
442506
{
443507
data: {
444508
users: ['[email protected]'],
@@ -447,6 +511,6 @@ describe('AddNewTeamMemberTrigger', () => {
447511
},
448512
},
449513
expect.any(Object),
450-
);
514+
));
451515
});
452516
});

src/authz-module/libraries-manager/components/AddNewTeamMemberModal/AddNewTeamMemberTrigger.tsx

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Button, useToggle } from '@openedx/paragon';
44
import { Plus } from '@openedx/paragon/icons';
55

66
import { PutAssignTeamMembersRoleResponse } from 'authz-module/data/api';
7-
import { useAssignTeamMembersRole } from '@src/authz-module/data/hooks';
7+
import { useAssignTeamMembersRole, useValidateUsers } from '@src/authz-module/data/hooks';
88
import { RoleOperationErrorStatus } from '@src/authz-module/constants';
99
import { AppToast, useToastManager } from '@src/authz-module/libraries-manager/ToastManagerContext';
1010
import { DEFAULT_TOAST_DELAY } from '@src/authz-module/libraries-manager/constants';
@@ -28,7 +28,9 @@ const AddNewTeamMemberTrigger = ({ libraryId }: AddNewTeamMemberTriggerProps) =>
2828
const [isError, setIsError] = useState(false);
2929
const [errorUsers, setErrorUsers] = useState<string[]>([]);
3030

31-
const { mutate: assignTeamMembersRole, isPending } = useAssignTeamMembersRole();
31+
const { mutate: assignTeamMembersRole, isPending: isAssigning } = useAssignTeamMembersRole();
32+
const { mutateAsync: validateUsersAsync, isPending: isValidating } = useValidateUsers();
33+
const isPending = isAssigning || isValidating;
3234
const {
3335
showToast, showErrorToast, Bold, Br,
3436
} = useToastManager();
@@ -128,16 +130,45 @@ const AddNewTeamMemberTrigger = ({ libraryId }: AddNewTeamMemberTriggerProps) =>
128130
};
129131
};
130132

131-
const handleAddTeamMember = () => {
133+
const handleAddTeamMember = async () => {
132134
const normalizedUsers = [...new Set(
133135
formValues.users
134136
.split(',')
135137
.map((u) => u.trim())
136138
.filter(Boolean),
137139
)];
138140

141+
let usersToAssign = normalizedUsers;
142+
143+
try {
144+
const { invalidUsers } = await validateUsersAsync({ data: { users: normalizedUsers } });
145+
146+
if (invalidUsers.length > 0) {
147+
const notFoundMessage = intl.formatMessage(
148+
messages['libraries.authz.manage.add.member.failure.not.found'],
149+
{
150+
count: invalidUsers.length,
151+
userIds: invalidUsers.join(', '),
152+
Bold,
153+
Br,
154+
},
155+
);
156+
showToast({ message: notFoundMessage, type: 'error', delay: DEFAULT_TOAST_DELAY });
157+
setErrorUsers(invalidUsers);
158+
setIsError(true);
159+
setFormValues((prev) => ({ ...prev, users: invalidUsers.join(', ') }));
160+
return;
161+
}
162+
163+
usersToAssign = normalizedUsers.filter((u) => !invalidUsers.includes(u));
164+
} catch (validationError) {
165+
// If validation endpoint fails, fall through and let assignTeamMembersRole handle errors
166+
}
167+
168+
if (usersToAssign.length === 0) { return; }
169+
139170
const payload = {
140-
users: normalizedUsers,
171+
users: usersToAssign,
141172
role: formValues.role,
142173
scope: libraryId,
143174
};
@@ -167,7 +198,7 @@ const AddNewTeamMemberTrigger = ({ libraryId }: AddNewTeamMemberTriggerProps) =>
167198
...roleAlreadyAssignedUsers.map(r => r.userIdentifier),
168199
];
169200

170-
const errorUserIds = normalizedUsers.filter((user) => !successUserIds.includes(user));
201+
const errorUserIds = usersToAssign.filter((user) => !successUserIds.includes(user));
171202
setErrorUsers(errorUserIds);
172203
setIsError(true);
173204
setFormValues((prev) => ({

0 commit comments

Comments
 (0)