From 26b87653106e8ba781af368a0a8dd44a7b45a721 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Fri, 22 Sep 2023 11:34:03 +0500 Subject: [PATCH] feat: refactor to make code human understandable --- src/components/InviteLearnersModal/index.jsx | 18 +-- src/data/validation/email.js | 118 +++++++++---------- src/data/validation/email.test.js | 40 +++++-- 3 files changed, 90 insertions(+), 86 deletions(-) diff --git a/src/components/InviteLearnersModal/index.jsx b/src/components/InviteLearnersModal/index.jsx index 6446bf7302..bab9dc5a67 100644 --- a/src/components/InviteLearnersModal/index.jsx +++ b/src/components/InviteLearnersModal/index.jsx @@ -10,7 +10,7 @@ import { camelCaseObject } from '@edx/frontend-platform/utils'; import emailTemplate from './emailTemplate'; import TextAreaAutoSize from '../TextAreaAutoSize'; import FileInput from '../FileInput'; -import { extractEmailAndIds, returnValidatedEmails, validateEmailAddrTemplateForm } from '../../data/validation/email'; +import { extractEmailIds, returnValidatedEmails, validateEmailAddrTemplateForm } from '../../data/validation/email'; import { normalizeFileUpload } from '../../utils'; class InviteLearnersModal extends React.Component { @@ -64,18 +64,6 @@ class InviteLearnersModal extends React.Component { } = this.props; const emailTemplateKey = 'email-template-body'; - - // Data can contain - // 1. emails only, OR - // 2. emails + Salesforce IDs (every email will have a corresponding sfid) - // In the 2nd case, remove the Salesforce IDs from formData and - // handle the Salesforce IDs independently, this way emails - // validation will remain as it is. - const data = extractEmailAndIds(formData); - if (data.haveSFIDs) { - formData['csv-email-addresses'] = data.emails; // eslint-disable-line no-param-reassign - } - // Validate form data validateEmailAddrTemplateForm(formData, emailTemplateKey); @@ -86,9 +74,7 @@ class InviteLearnersModal extends React.Component { }; options.user_emails = returnValidatedEmails(formData); - if (data.haveSFIDs) { - options.user_sfids = data.ids; - } + options.user_sfids = extractEmailIds(formData, options.user_emails); /* eslint-disable no-underscore-dangle */ return addLicensesForUsers(options, subscriptionUUID) diff --git a/src/data/validation/email.js b/src/data/validation/email.js index b6a4faf663..70d2f44adf 100644 --- a/src/data/validation/email.js +++ b/src/data/validation/email.js @@ -82,6 +82,33 @@ const validateEmailAddresses = (emails) => { return result; }; +// Each row in textarea or csv can contain email plus an optional salesforce id +// Email and salesforce id will be separated by comma. This function will read +// each row, split it by comma and then return an object with three properties: +// textEmails: All emails extracted from textarea +// CSVEmails: All emails extracted from CSV +// allEmails: Concatenation of `textEmails` and `CSVEmails` +const extractEmails = (formData) => { + let textEmails = []; + let CSVEmails = []; + let allEmails = []; + + if (formData[EMAIL_ADDRESS_TEXT_FORM_DATA] && formData[EMAIL_ADDRESS_TEXT_FORM_DATA].length) { + textEmails = formData[EMAIL_ADDRESS_TEXT_FORM_DATA].split(/\r\n|\n/).map(item => item.split(',')[0]); + } + if (formData[EMAIL_ADDRESS_CSV_FORM_DATA] && formData[EMAIL_ADDRESS_CSV_FORM_DATA].length) { + CSVEmails = formData[EMAIL_ADDRESS_CSV_FORM_DATA].map(item => item.split(',')[0]); + } + + allEmails = [...textEmails, ...CSVEmails]; + + return { + textEmails: textEmails.length ? textEmails : undefined, + CSVEmails: CSVEmails.length ? CSVEmails : undefined, + allEmails, + }; +}; + const validateEmailAddressesFields = (formData) => { // Validate that email address fields contain valid-looking emails. // Expects Redux form data @@ -90,8 +117,9 @@ const validateEmailAddressesFields = (formData) => { _error: [], }; - const textAreaEmails = formData[EMAIL_ADDRESS_TEXT_FORM_DATA] && formData[EMAIL_ADDRESS_TEXT_FORM_DATA].split(/\r\n|\n/); - const csvEmails = formData[EMAIL_ADDRESS_CSV_FORM_DATA]; + const extractedEmails = extractEmails(formData); + const textAreaEmails = extractedEmails.textEmails; + const csvEmails = extractedEmails.CSVEmails; let { invalidEmailIndices, } = validateEmailAddresses(textAreaEmails || csvEmails); @@ -155,80 +183,48 @@ const returnValidatedEmails = (formData) => { if (errorsDict._error.length > 0) { throw new SubmissionError(errorsDict); } - let emails = []; - if (formData[EMAIL_ADDRESS_TEXT_FORM_DATA] && formData[EMAIL_ADDRESS_TEXT_FORM_DATA].length) { - emails.push(...formData[EMAIL_ADDRESS_TEXT_FORM_DATA].split(/\r\n|\n/)); - } - if (formData[EMAIL_ADDRESS_CSV_FORM_DATA] && formData[EMAIL_ADDRESS_CSV_FORM_DATA].length) { - emails.push(...formData[EMAIL_ADDRESS_CSV_FORM_DATA]); - } + let emails = extractEmails(formData).allEmails; emails = _.union(emails); // Dedup emails return validateEmailAddresses(emails).validEmails; }; -// Return an object of email and id if non-empty email and id are present -const sanitize = (row) => { - const data = row.split(','); - // only consider those rows where we have 2 columns - if (data.length === 2) { - const email = data[0].trim(); - const id = data[1].trim(); - // only consider email and id if both are non-empty - if (email && id) { - return { - id, - email: email.toLowerCase(), - }; - } - } - return null; -}; - -/** -* Return an object containing the below properties -* emails (array): Learner emails -* ids (array): Salesforce ids corresponding to each email -* haveSFIDs (bool): Whether the formData contains Salesforce ids or not -*/ -const extractEmailAndIds = (formData) => { - const emails = []; - const ids = []; +// Combine all the rows from textarea and CSV and then make a map of email to salesforce id +const parseData = (formData) => { + const rows = []; + const all = {}; - // TODO: TBD: Most probably we will remove this and will not handle emails + salesforce ids in textarea if (formData[EMAIL_ADDRESS_TEXT_FORM_DATA] && formData[EMAIL_ADDRESS_TEXT_FORM_DATA].length) { - const rows = formData[EMAIL_ADDRESS_TEXT_FORM_DATA].split(/\r\n|\n/); - rows.forEach((row) => { - const data = sanitize(row); - // do not add duplicate emails - if (data && emails.includes(data.email) === false) { - emails.push(data.email); - ids.push(data.id); - } - }); + rows.push(...formData[EMAIL_ADDRESS_TEXT_FORM_DATA].split(/\r\n|\n/)); } - // TBD: We will only handle emails + salesforce ids in CSV if (formData[EMAIL_ADDRESS_CSV_FORM_DATA] && formData[EMAIL_ADDRESS_CSV_FORM_DATA].length) { - formData[EMAIL_ADDRESS_CSV_FORM_DATA].forEach((row) => { - const data = sanitize(row); - // do not add duplicate emails - if (data && emails.includes(data.email) === false) { - emails.push(data.email); - ids.push(data.id); - } - }); + rows.push(...formData[EMAIL_ADDRESS_CSV_FORM_DATA]); } - return { - emails, - ids, - haveSFIDs: !!(emails.length > 0 && ids.length > 0 && emails.length === ids.length), - }; + rows.forEach((row) => { + const [email, id] = row.split(',').map(item => item.trim()); + all[email] = id; + }); + + return all; +}; + +// Extract salesforce ids for all validated emails +const extractEmailIds = (formData, userEmails) => { + const parsedData = parseData(formData); + const ids = []; + + userEmails.forEach((email) => { + ids.push(parsedData[email]); + }); + + const allFalse = ids.every(item => !item); + return allFalse ? [] : ids; }; /* eslint-enable no-underscore-dangle */ export { - extractEmailAndIds, + extractEmailIds, validateEmailAddresses, validateEmailAddressesFields, validateEmailTemplateForm, diff --git a/src/data/validation/email.test.js b/src/data/validation/email.test.js index b52f0705ed..6f2caa5669 100644 --- a/src/data/validation/email.test.js +++ b/src/data/validation/email.test.js @@ -1,6 +1,6 @@ import _ from 'lodash'; import { - extractEmailAndIds, + extractEmailIds, validateEmailAddresses, validateEmailAddressesFields, validateEmailTemplateFields, @@ -238,17 +238,39 @@ describe('email validation', () => { describe('validate emails and ids extraction', () => { it('extracted correct emails and ids', () => { const formData = new FormData(); + formData[EMAIL_ADDRESS_TEXT_FORM_DATA] = [ + 'abc@example.com,000000000000ABCABC', + 'asdf@example.com,', + 'zzz@example.com,000000000000XYZXYZ', + ].join('\n'); formData[EMAIL_ADDRESS_CSV_FORM_DATA] = [ - 'cthulu@lkerwjrwlke.com,000000000000ABCABC', - 'bobbyb@test.com,000000000000XYZXYZ', - 'cthulu@lkerwjrwlke.com,000000000000ABCDDD', - 'abc@example.com,', + 'one@example.com,000000000000YYYYYY', + 'two@example.com,000000000000ZZZZZZ', + 'three@example.com,000000000000ABCDDD', + 'wow@example.com,', + 'abc@example.com,000000000000ABCABC', + ]; + const userEmails = [ + 'abc@example.com', + 'asdf@example.com', + 'zzz@example.com', + 'one@example.com', + 'two@example.com', + 'three@example.com', + 'wow@example.com', ]; - const data = extractEmailAndIds(formData); - expect(data.emails.sort()).toEqual(['bobbyb@test.com', 'cthulu@lkerwjrwlke.com'].sort()); - expect(data.ids).toEqual(['000000000000ABCABC', '000000000000XYZXYZ']); - expect(data.haveSFIDs).toBeTruthy(); + const ids = extractEmailIds(formData, userEmails); + expect(ids).toEqual([ + '000000000000ABCABC', + '', + '000000000000XYZXYZ', + '000000000000YYYYYY', + '000000000000ZZZZZZ', + '000000000000ABCDDD', + '', + ]); + expect(userEmails.length).toEqual(ids.length); }); }); });