Skip to content

Commit f26371a

Browse files
fix: add/remove group member with non-lowercase email (#1464)
* fix: add/remove group member with non-lowercase email * fix: preserve original cased validated emails * docs: comment on removeStringsFromListCaseInsensitive implementation * fix: bug in removeStringsFromListCaseInsensitive * fix: LearnerDetailPage unit test * chore: use lodash without
1 parent c8258f4 commit f26371a

File tree

14 files changed

+128
-45
lines changed

14 files changed

+128
-45
lines changed

src/components/PeopleManagement/AddMembersModal/AddMembersModal.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const AddMembersModal = ({
2424
groupUuid,
2525
}) => {
2626
const intl = useIntl();
27-
const { lowerCasedEmails: learnerEmails, canInvite: canInviteMembers } = useValidatedEmailsContext();
27+
const { validatedEmails: learnerEmails, canInvite: canInviteMembers } = useValidatedEmailsContext();
2828
const [addButtonState, setAddButtonState] = useState('default');
2929
const [isSystemErrorModalOpen, openSystemErrorModal, closeSystemErrorModal] = useToggle(false);
3030
const handleCloseAddMembersModal = () => {

src/components/PeopleManagement/AddMembersModal/AddMembersModalContent.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import FileUpload from '../../learner-credit-management/invite-modal/FileUpload'
1313
import EnterpriseCustomerUserDataTable from '../EnterpriseCustomerUserDataTable';
1414
import { useEnterpriseLearners } from '../../learner-credit-management/data';
1515
import { HELP_CENTER_URL } from '../constants';
16-
import { removeStringsFromList, splitAndTrim } from '../../../utils';
16+
import { removeStringsFromListCaseInsensitive, splitAndTrim } from '../../../utils';
1717
import { addEmailsAction, initializeEnterpriseEmailsAction } from '../data/actions';
1818
import { useValidatedEmailsContext } from '../data/ValidatedEmailsContext';
1919

@@ -23,14 +23,14 @@ const AddMembersModalContent = ({
2323
enterpriseGroupLearners,
2424
}) => {
2525
const memberInviteMetadata = useValidatedEmailsContext();
26-
const { dispatch, lowerCasedEmails } = memberInviteMetadata;
26+
const { dispatch, validatedEmails } = memberInviteMetadata;
2727
const { allEnterpriseLearners } = useEnterpriseLearners({ enterpriseUUID });
2828

2929
const handleCsvUpload = useCallback((csv) => {
3030
let emails = splitAndTrim('\n', csv);
31-
emails = removeStringsFromList(emails, lowerCasedEmails);
31+
emails = removeStringsFromListCaseInsensitive(emails, validatedEmails);
3232
dispatch(addEmailsAction({ emails, clearErroredEmails: true, actionType: 'UPLOAD_CSV_ACTION' }));
33-
}, [dispatch, lowerCasedEmails]);
33+
}, [dispatch, validatedEmails]);
3434

3535
useEffect(() => {
3636
const groupEnterpriseLearners = enterpriseGroupLearners.map((learner) => learner?.memberDetails?.userEmail);

src/components/PeopleManagement/CreateGroupModal.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const CreateGroupModal = ({
2525
}) => {
2626
const intl = useIntl();
2727
const {
28-
lowerCasedEmails: learnerEmails,
28+
validatedEmails: learnerEmails,
2929
canInvite: canInviteMembers,
3030
isCreateGroupFileUpload,
3131
isCreateGroupListSelection,

src/components/PeopleManagement/CreateGroupModalContent.jsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import InviteSummaryCount from '../learner-credit-management/invite-modal/Invite
1212
import FileUpload from '../learner-credit-management/invite-modal/FileUpload';
1313
import { HELP_CENTER_URL, MAX_LENGTH_GROUP_NAME } from './constants';
1414
import EnterpriseCustomerUserDataTable from './EnterpriseCustomerUserDataTable';
15-
import { removeStringsFromList, splitAndTrim } from '../../utils';
15+
import { removeStringsFromListCaseInsensitive, splitAndTrim } from '../../utils';
1616
import { useValidatedEmailsContext } from './data/ValidatedEmailsContext';
1717
import { addEmailsAction, initializeEnterpriseEmailsAction } from './data/actions';
1818

@@ -21,7 +21,7 @@ const CreateGroupModalContent = ({
2121
onSetGroupName,
2222
}) => {
2323
const memberInviteMetadata = useValidatedEmailsContext();
24-
const { dispatch, lowerCasedEmails } = memberInviteMetadata;
24+
const { dispatch, validatedEmails } = memberInviteMetadata;
2525
const [groupNameLength, setGroupNameLength] = useState(0);
2626
const [groupName, setGroupName] = useState('');
2727

@@ -41,9 +41,9 @@ const CreateGroupModalContent = ({
4141

4242
const handleCsvUpload = useCallback((csv) => {
4343
let emails = splitAndTrim('\n', csv);
44-
emails = removeStringsFromList(emails, lowerCasedEmails);
44+
emails = removeStringsFromListCaseInsensitive(emails, validatedEmails);
4545
dispatch(addEmailsAction({ emails, clearErroredEmails: true, actionType: 'UPLOAD_CSV_ACTION' }));
46-
}, [dispatch, lowerCasedEmails]);
46+
}, [dispatch, validatedEmails]);
4747

4848
useEffect(() => {
4949
// Initialize upon first entry

src/components/PeopleManagement/EnterpriseCustomerUserDataTable.jsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,11 @@ const CustomSelectColumnCell = ({ row }) => {
4646
itemCount,
4747
controlledTableSelections: [, dataTableDispatch],
4848
} = dataTableContext;
49-
const { dispatch: validateEmailsDispatch, lowerCasedEmails, groupEnterpriseLearners } = useValidatedEmailsContext();
49+
const {
50+
dispatch: validateEmailsDispatch, lowerCasedEmails, groupEnterpriseLearners,
51+
} = useValidatedEmailsContext();
5052
const isAddedMember = groupEnterpriseLearners.includes(selectedEmail);
51-
const isValidated = lowerCasedEmails.includes(selectedEmail);
53+
const isValidated = lowerCasedEmails.includes(selectedEmail.toLowerCase());
5254

5355
const toggleSelected = useCallback(
5456
() => {

src/components/PeopleManagement/LearnerDetailPage/CourseEnrollments.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ const CourseEnrollments = ({ userEmail, lmsUserId, enterpriseUuid }) => {
1717
) : (
1818
<>
1919
<h3>Enrollments</h3>
20-
{enrollments.completed.map((enrollment) => (
20+
{enrollments?.completed?.map((enrollment) => (
2121
<EnrollmentCard enrollment={enrollment} />
2222
))}
23-
{enrollments.inProgress.map((enrollment) => (
23+
{enrollments?.inProgress?.map((enrollment) => (
2424
<EnrollmentCard enrollment={enrollment} />
2525
))}
26-
{enrollments.upcoming.map((enrollment) => (
26+
{enrollments?.upcoming?.map((enrollment) => (
2727
<EnrollmentCard enrollment={enrollment} />
2828
))}
2929
</>

src/components/PeopleManagement/data/ValidatedEmailsContext.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export const initialContext: ValidatedEmailsContext = {
1818
isValidInput: true,
1919
isCreateGroupFileUpload: false,
2020
isCreateGroupListSelection: false,
21+
validatedEmails: [],
2122
lowerCasedEmails: [],
2223
duplicateEmails: [],
2324
invalidEmails: [],

src/components/PeopleManagement/data/reducer.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,19 @@ export type ValidatedEmailsReducerType = (
2121
const allEmails = (state: ValidatedEmailsContext) => {
2222
// Return all emails from the context regardless of error status
2323
const {
24-
lowerCasedEmails, duplicateEmails, invalidEmails,
24+
validatedEmails, duplicateEmails, invalidEmails,
2525
} = state;
26-
return [lowerCasedEmails, duplicateEmails, invalidEmails].flatMap((list) => list || []);
26+
return [validatedEmails, duplicateEmails, invalidEmails].flatMap((list) => list || []);
27+
};
28+
29+
const removeEmailsFromState = (
30+
state: ValidatedEmailsContext,
31+
removedValidatedEmails: string[],
32+
): ValidatedEmailsContext => {
33+
const validatedEmails = removeStringsFromList(state.validatedEmails as string[], removedValidatedEmails);
34+
const lowerCasedEmailsToRemove = removedValidatedEmails.map((str) => str.toLowerCase());
35+
const lowerCasedEmails = removeStringsFromList(state.lowerCasedEmails as string[], lowerCasedEmailsToRemove);
36+
return { ...state, validatedEmails, lowerCasedEmails };
2737
};
2838

2939
const getUpdatedEmailsAndState = (
@@ -36,7 +46,7 @@ const getUpdatedEmailsAndState = (
3646
const newState = {
3747
...state, duplicateEmails: [], invalidEmails: [],
3848
};
39-
return [newState, [...(newState.lowerCasedEmails || []), ...addedEmails]];
49+
return [newState, [...(newState.validatedEmails || []), ...addedEmails]];
4050
}
4151

4252
return [state, [...allEmails(state), ...addedEmails]];
@@ -69,10 +79,7 @@ export const ValidatedEmailsReducer: ValidatedEmailsReducerType = (
6979
return { ...newState, ...emailValidation };
7080
} case REMOVE_EMAILS: {
7181
const { emails: removedEmails } = action.arguments as RemoveEmailsArguments;
72-
const newState = { ...state };
73-
const emails = newState.lowerCasedEmails as string[];
74-
newState.lowerCasedEmails = removeStringsFromList(emails, removedEmails);
75-
return { ...newState };
82+
return removeEmailsFromState(state, removedEmails);
7683
} default: {
7784
logError(`Unexpected action: ${action?.type}`);
7885
return state;

src/components/PeopleManagement/tests/CreateGroupModal.test.jsx

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ const mockTabledata = {
9898
{
9999
enterpriseCustomerUser: {
100100
userId: 4,
101-
name: 'Test User 4',
102-
email: 'testuser-4@2u.com',
101+
name: 'Non-lowercased User Email',
102+
email: 'testUser-NonLowercase@2u.com',
103103
joinedOrg: 'July 4, 2024',
104104
},
105105
},
@@ -415,4 +415,32 @@ describe('<CreateGroupModal />', () => {
415415
});
416416
expect(screen.queryByText('Only 1 invite per email address will be sent.')).not.toBeInTheDocument();
417417
});
418+
it('can add/remove members with non-lowercased emails', async () => {
419+
const mockGroupData = { uuid: 'test-uuid' };
420+
LmsApiService.createEnterpriseGroup.mockResolvedValue({ status: 201, data: mockGroupData });
421+
422+
const mockInviteData = { records_processed: 1, new_learners: 1, existing_learners: 0 };
423+
LmsApiService.inviteEnterpriseLearnersToGroup.mockResolvedValue(mockInviteData);
424+
425+
render(<CreateGroupModalWrapper />);
426+
const groupNameInput = screen.getByTestId('group-name');
427+
userEvent.type(groupNameInput, 'test group name');
428+
429+
// Add non-lowercased member
430+
const membersCheckbox = screen.getAllByTitle('Toggle Row Selected');
431+
userEvent.click(membersCheckbox[3]);
432+
433+
await waitFor(() => {
434+
expect(screen.getByText('Summary (1)')).toBeInTheDocument();
435+
expect(screen.getAllByText('[email protected]')).toHaveLength(2);
436+
}, { timeout: EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY + 1000 });
437+
438+
// Remove non-lowercased member
439+
userEvent.click(membersCheckbox[3]);
440+
441+
await waitFor(() => {
442+
expect(screen.queryByText('Summary (1)')).not.toBeInTheDocument();
443+
expect(screen.getAllByText('[email protected]')).toHaveLength(1);
444+
}, { timeout: EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY + 1000 });
445+
});
418446
});

src/components/learner-credit-management/cards/data/utils.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ export type LearnerEmailsValidityReport = {
118118
canInvite: boolean,
119119
/* If there are no invalid emails and the count of emails is under the limit */
120120
isValidInput: boolean,
121+
/* List of valid emails in their original casing */
122+
validatedEmails: string[],
121123
/* List of valid lower-cased emails */
122124
lowerCasedEmails: string[],
123125
/* List of duplicate emails */
@@ -143,6 +145,7 @@ export const isInviteEmailAddressesInputValueValid = ({
143145
}: LearnerEmailsValidityArgs): LearnerEmailsValidityReport => {
144146
let validationError;
145147
const learnerEmailsCount = learnerEmails.length;
148+
const lowerCasedEmails: string[] = [];
146149
const validatedEmails: string[] = [];
147150
const invalidEmails: string[] = [];
148151
const duplicateEmails: string[] = [];
@@ -153,12 +156,13 @@ export const isInviteEmailAddressesInputValueValid = ({
153156
// Validate the email address
154157
if (!isEmail(email)) {
155158
invalidEmails.push(email);
156-
} else if (validatedEmails.includes(lowerCasedEmail)) {
159+
} else if (lowerCasedEmails.includes(lowerCasedEmail)) {
157160
// Check for duplicates (case-insensitive)
158161
duplicateEmails.push(email);
159162
} else {
160-
// Add to list of lower-cased emails already handled
161-
validatedEmails.push(lowerCasedEmail);
163+
// Add to list of emailss already handled
164+
lowerCasedEmails.push(lowerCasedEmail);
165+
validatedEmails.push(email);
162166
}
163167
});
164168

@@ -199,7 +203,8 @@ export const isInviteEmailAddressesInputValueValid = ({
199203

200204
return {
201205
canInvite,
202-
lowerCasedEmails: validatedEmails,
206+
lowerCasedEmails,
207+
validatedEmails,
203208
duplicateEmails,
204209
invalidEmails,
205210
isValidInput,

0 commit comments

Comments
 (0)