Merge branch 'master' into mkeating/ENT-9742

.env

@@ -1,2 +1 @@
 NODE_ENV='production'
-USE_API_CACHE=true

.env.development

@@ -52,7 +52,6 @@ IS_MAINTENANCE_ALERT_ENABLED=''
 MAINTENANCE_ALERT_MESSAGE=''
 MAINTENANCE_ALERT_START_TIMESTAMP=''
 MAINTENANCE_ALERT_END_TIMESTAMP=''
-USE_API_CACHE='true'
 SUBSCRIPTION_LPR='true'
 PLOTLY_SERVER_URL='http://localhost:8050'
 AUTH0_SELF_SERVICE_INTEGRATION='true'

docs/decisions/0009-useCache_deprecation.rst

@@ -0,0 +1,45 @@
+9. Deprecation of `useCache` option
+===================================
+
+Status
+******
+
+Accepted
+
+Context
+*******
+
+Frontend platform offers a variety of helpers to facilitate API calls to our backend services. Part of their suite of
+helper functions are `getAuthenticatedHttpClient` which allows a `useCache` option. The inclusion of the `useCache` option
+treats the original `getAuthenticatedHttpClient` as a
+`cachedAuthenticatedHttpClient` `method <https://github.com/openedx/frontend-platform/blob/15ef507e41127b4fd4ace5d31f7e527381678572/src/auth/AxiosJwtAuthService.js#L111-L117>`_.
+This has the effect of modifying the headers of the API request to our backend service. Within the the
+`Access-Control-Request-Headers` preflight request or OPTIONS call, the inclusion of `useCache` resulted in the
+`Cache-Control` header also being sent resulting in an unresolved API response and breaking usability for certain
+components that depended on the upstream data.
+
+Decision
+********
+
+Deprecate the usage of `useCache` within `getAuthenticatedHttpClient` to avoid inadvertently modifying the expected
+request headers to our backend services. For instances where client side caching of API calls is necessary, use
+`React Query <https://tanstack.com/query/v4/docs/framework/react/overview>`_ when constructing the API call. There are
+several examples that currently reside in our enterprise MFEs that
+`effectively leverage <https://github.com/openedx/frontend-app-learner-portal-enterprise/blob/master/src/components/app/data/hooks/useBFF.js>`_ React Query, and the
+usage of `query keys <https://github.com/openedx/frontend-app-learner-portal-enterprise/blob/master/src/components/app/data/queries/queryKeyFactory.js>`_ and
+`helpers <https://github.com/openedx/frontend-app-learner-portal-enterprise/tree/master/src/components/app/data/queries>`_.
+
+
+Consequences
+************
+
+This will in the intermediary remove any client side caching facilitated by the `useCache` option. But should intentionally
+be updated with a React Query implementation to maintain the benefits of client side caching. In the meantime, the backend
+services cache will suffice
+
+Alternatives Considered
+***********************
+The underlying package, `frontend-platform`, which depended on the breaking package upgrade, `Axios`, was downgraded as part of resolving
+issues related to the `useCache` field. It was determined that the `useCache` field was primarily used in enterprise, so
+it made the most sense to remove the `useCache` field in favor of introducing tools like `React Query` for client side
+caching.

package.json

@@ -42,7 +42,6 @@
     "@tanstack/react-query": "4.36.1",
     "@tanstack/react-query-devtools": "4.36.1",
     "algoliasearch": "4.24.0",
-    "axios-mock-adapter": "1.22.0",
     "classnames": "2.5.1",
     "color": "3.2.1",
     "color-contrast-checker": "^2.1.0",
@@ -101,6 +100,7 @@
     "@testing-library/react-hooks": "5.1.3",
     "@testing-library/user-event": "12.8.3",
     "@wojtekmaj/enzyme-adapter-react-17": "0.8.0",
+    "axios-mock-adapter": "1.22.0",
     "css-loader": "5.2.7",
     "enzyme": "3.11.0",
     "husky": "0.14.3",

src/components/PeopleManagement/CreateGroupModalContent.jsx

@@ -11,7 +11,7 @@ import { FormattedMessage } from '@edx/frontend-platform/i18n';
 import InviteModalSummary from '../learner-credit-management/invite-modal/InviteModalSummary';
 import InviteSummaryCount from '../learner-credit-management/invite-modal/InviteSummaryCount';
 import FileUpload from '../learner-credit-management/invite-modal/FileUpload';
-import { isInviteEmailAddressesInputValueValid } from '../learner-credit-management/cards/data';
+import { isInviteEmailAddressesInputValueValid, removeInvalidEmailsFromList } from '../learner-credit-management/cards/data';
 import { HELP_CENTER_URL, MAX_LENGTH_GROUP_NAME } from './constants';
 import EnterpriseCustomerUserDataTable from './EnterpriseCustomerUserDataTable';
 import { useEnterpriseLearners } from '../learner-credit-management/data';
@@ -56,7 +56,8 @@ const CreateGroupModalContent = ({
       onEmailAddressesChange([]);
       return;
     }
-    setLearnerEmails(prev => [...prev, ...value]);
+    // Merge new emails with old emails (removing duplicates)
+    setLearnerEmails(prev => _.union(prev, value));
     setIsCreateGroupListSelection(true);
   }, [onEmailAddressesChange, setIsCreateGroupListSelection]);
 
@@ -70,11 +71,13 @@ const CreateGroupModalContent = ({
   }, [onEmailAddressesChange]);
 
   const handleCsvUpload = useCallback((emails) => {
+    // Remove errored emails from the main list
+    const cleanEmails = removeInvalidEmailsFromList(learnerEmails, memberInviteMetadata);
     // Merge new emails with old emails (removing duplicates)
-    const allEmails = _.union(learnerEmails, splitAndTrim('\n', emails));
+    const allEmails = _.union(cleanEmails, splitAndTrim('\n', emails));
     setLearnerEmails(allEmails);
     setIsCreateGroupFileUpload(true);
-  }, [learnerEmails, setIsCreateGroupFileUpload]);
+  }, [learnerEmails, memberInviteMetadata, setIsCreateGroupFileUpload]);
 
   // Validate the learner emails emails from user input whenever it changes
   useEffect(() => {

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

@@ -348,6 +348,40 @@ describe('<CreateGroupModal />', () => {
       expect(screen.getAllByText('[email protected]')).toHaveLength(2);
     }, { timeout: EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY + 1000 });
   });
+  it('should clear errors from bad csv file after uploading good csv file', async () => {
+    render(<CreateGroupModalWrapper />);
+    const fakeFile = new File(['iamnotanemail'], 'bademails.csv', { type: 'text/csv' });
+    const dropzone = screen.getByText('Drag and drop your file here or click to upload.');
+    Object.defineProperty(dropzone, 'files', {
+      value: [fakeFile],
+      writable: true,
+    });
+    fireEvent.drop(dropzone);
+
+    await waitFor(() => {
+      expect(screen.getByText('bademails.csv')).toBeInTheDocument();
+      expect(screen.getByText("Members can't be invited as entered.")).toBeInTheDocument();
+      expect(screen.getByText('iamnotanemail is not a valid email.')).toBeInTheDocument();
+    }, { timeout: EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY + 1000 });
+
+    const dropzone2 = screen.getByText('bademails.csv');
+    const fakeFile2 = new File(['[email protected]'], 'goodemails.csv', { type: 'text/csv' });
+    Object.defineProperty(dropzone2, 'files', {
+      value: [fakeFile2],
+    });
+    fireEvent.drop(dropzone2);
+
+    const groupNameInput = screen.getByTestId('group-name');
+    userEvent.type(groupNameInput, 'test group name');
+
+    await waitFor(() => {
+      expect(screen.getByText('goodemails.csv')).toBeInTheDocument();
+      expect(screen.getByText('Summary (1)')).toBeInTheDocument();
+      expect(screen.getAllByText('[email protected]')).toHaveLength(2);
+    }, { timeout: EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY + 1000 });
+    expect(screen.queryByText("Members can't be invited as entered.")).not.toBeInTheDocument();
+    expect(screen.queryByText('iamnotanemail is not a valid email.')).not.toBeInTheDocument();
+  });
   it('displays error for email not belonging in an org', async () => {
     const mockGroupData = { uuid: 'test-uuid' };
     LmsApiService.createEnterpriseGroup.mockResolvedValue({ status: 201, data: mockGroupData });
@@ -405,4 +439,22 @@ describe('<CreateGroupModal />', () => {
       )).toBeInTheDocument();
     });
   });
+  it('does not show duplicate error when members are bulk added multiple times', async () => {
+    render(<CreateGroupModalWrapper />);
+    // testing interaction with adding members from the datatable
+    const membersCheckboxes = screen.getAllByRole('checkbox');
+    // skipping first one because its the select all checkbox
+    userEvent.click(membersCheckboxes[1]);
+    const addMembersButton = screen.getByText('Add');
+    userEvent.click(addMembersButton);
+    // Select a second member while keeping first selected, and add again
+    userEvent.click(membersCheckboxes[2]);
+    userEvent.click(addMembersButton);
+
+    await waitFor(() => {
+      expect(screen.getAllByText('[email protected]')).toHaveLength(2);
+      expect(screen.getAllByText('[email protected]')).toHaveLength(2);
+    });
+    expect(screen.queryByText('Only 1 invite per email address will be sent.')).not.toBeInTheDocument();
+  });
 });

src/components/learner-credit-management/assignment-modal/AssignmentModalContent.jsx

@@ -59,28 +59,26 @@ const AssignmentModalContent = ({
     setDropdownToggleLabel,
     setGroupMemberEmails,
   });
-  const handleEmailAddressInputChange = (e) => {
-    const inputValue = e.target.value;
-    setEmailAddressesInputValue(inputValue);
-  };
+
   const handleEmailAddressesChanged = useCallback((value) => {
     if (!value) {
       setLearnerEmails([]);
-      onEmailAddressesChange([]);
       return;
     }
     const emails = value.split('\n').filter((email) => email.trim().length > 0);
     setLearnerEmails(emails);
-  }, [onEmailAddressesChange]);
+  }, []);
 
   const debouncedHandleEmailAddressesChanged = useMemo(
     () => debounce(handleEmailAddressesChanged, EMAIL_ADDRESSES_INPUT_VALUE_DEBOUNCE_DELAY),
     [handleEmailAddressesChanged],
   );
 
-  useEffect(() => {
-    debouncedHandleEmailAddressesChanged(emailAddressesInputValue);
-  }, [emailAddressesInputValue, debouncedHandleEmailAddressesChanged]);
+  const handleEmailAddressInputChange = (e) => {
+    const inputValue = e.target.value;
+    debouncedHandleEmailAddressesChanged(inputValue);
+    setEmailAddressesInputValue(inputValue);
+  };
 
   useEffect(() => {
     handleGroupsChanged(checkedGroups);

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

@@ -2,7 +2,7 @@ import isEmail from 'validator/lib/isEmail';
 
 import { MAX_EMAIL_ENTRY_LIMIT, MAX_INITIAL_LEARNER_EMAILS_DISPLAYED_COUNT } from './constants';
 import { formatPrice } from '../../data';
-import { makePlural } from '../../../../utils';
+import { makePlural, removeStringsFromList } from '../../../../utils';
 
 /**
  * Transforms and formats a policy's display name for rendering within the assignment modal's allocation alert modals.
@@ -37,7 +37,7 @@ export const hasLearnerEmailsSummaryListTruncation = (learnerEmails) => (
  * @param {Number} contentPrice Price of the content (USD).
  * @param {Array<String>} remainingBalance Amount remaining in the budget (USD).
  *
- * @returns Object containing various properties about the validity of the learner emails
+ * @returns EmailValidityReport containing various properties about the validity of the learner emails
  * input, including a validation error when appropriate, and whether the assignment allocation
  * should proceed.
  */
@@ -53,9 +53,9 @@ export const isAssignEmailAddressesInputValueValid = ({
   const remainingBalanceAfterAssignment = remainingBalance - totalAssignmentCost;
   const hasEnoughBalanceForAssignment = remainingBalanceAfterAssignment >= 0;
 
-  const lowerCasedEmails = [];
-  const invalidEmails = [];
-  const duplicateEmails = [];
+  const lowerCasedEmails: string[] = [];
+  const invalidEmails: string[] = [];
+  const duplicateEmails: string[] = [];
 
   learnerEmails.forEach((email) => {
     const lowerCasedEmail = email.toLowerCase();
@@ -109,23 +109,48 @@ export const isAssignEmailAddressesInputValueValid = ({
   };
 };
 
+export type LearnerEmailsValidityArgs = {
+  learnerEmails: string[],
+  allEnterpriseLearners: string[] | null,
+};
+
+export type LearnerEmailsValidityReport = {
+  /* Whether we can invite with the current set of learner emails */
+  canInvite: boolean,
+  /* If there are no invalid emails and the count of emails is under the limit */
+  isValidInput: boolean,
+  /* List of valid lower-cased emails */
+  lowerCasedEmails: string[],
+  /* List of duplicate emails */
+  duplicateEmails: string[],
+  /* List of entries that are not valid email addresses */
+  invalidEmails: string[],
+  /* List of emails that aren't in the organization */
+  emailsNotInOrg: string[],
+  /* Error message shown to the user */
+  validationError: string,
+};
+
 /**
  * Determine the validity of the learner emails user input. The input is valid if
  * all emails are valid. Invalid and duplicate emails are returned.
  *
  * @param {Array<String>} learnerEmails List of learner emails.
  *
- * @returns Object containing various properties about the validity of the learner emails
+ * @returns LearnerEmailsValidityReport Object containing various properties about the validity of the learner emails
  * input, including a validation error when appropriate, and whether the member invitation
  * should proceed.
  */
-export const isInviteEmailAddressesInputValueValid = ({ learnerEmails, allEnterpriseLearners = null }) => {
+export const isInviteEmailAddressesInputValueValid = ({
+  learnerEmails,
+  allEnterpriseLearners = null,
+}: LearnerEmailsValidityArgs): LearnerEmailsValidityReport => {
   let validationError;
   const learnerEmailsCount = learnerEmails.length;
-  const lowerCasedEmails = [];
-  const invalidEmails = [];
-  const duplicateEmails = [];
-  const emailsNotInOrg = [];
+  const lowerCasedEmails: string[] = [];
+  const invalidEmails: string[] = [];
+  const duplicateEmails: string[] = [];
+  const emailsNotInOrg: string[] = [];
 
   learnerEmails.forEach((email) => {
     const lowerCasedEmail = email.toLowerCase();
@@ -159,7 +184,7 @@ export const isInviteEmailAddressesInputValueValid = ({ learnerEmails, allEnterp
     if (learnerEmailsCount > MAX_EMAIL_ENTRY_LIMIT) {
       validationError.reason = 'over_email_max';
       validationError.message = `${learnerEmailsCount} emails entered (${MAX_EMAIL_ENTRY_LIMIT} maximum). `
-      + `Delete ${learnerEmailsCount - MAX_EMAIL_ENTRY_LIMIT} emails to proceed.`;
+        + `Delete ${learnerEmailsCount - MAX_EMAIL_ENTRY_LIMIT} emails to proceed.`;
     }
     if (invalidEmails.length > 0) {
       validationError.reason = 'invalid_email';
@@ -198,3 +223,21 @@ export const isInviteEmailAddressesInputValueValid = ({ learnerEmails, allEnterp
     emailsNotInOrg,
   };
 };
+
+/**
+ * Remove invalid emails from list based on
+ *
+ * @param {Array<String>} learnerEmails List of learner emails.
+ *
+ * @returns Object containing various properties about the validity of the learner emails
+ * input, including a validation error when appropriate, and whether the member invitation
+ * should proceed.
+ */
+export const removeInvalidEmailsFromList = (
+  learnerEmails: string[],
+  inviteMetadata: LearnerEmailsValidityReport,
+): string[] => {
+  const { duplicateEmails, invalidEmails, emailsNotInOrg } = inviteMetadata;
+  const allInvalidEmails = [...(duplicateEmails || []), ...(invalidEmails || []), ...(emailsNotInOrg || [])];
+  return removeStringsFromList(learnerEmails, allInvalidEmails);
+};