Merge pull request #526 from edx/mroytman/MST-1099-fix-loading-issues

[MST-1099] Fix Flickering By Removing Unnecessary Re-Renders

src/constants.js

@@ -0,0 +1,4 @@
+export const IDLE_STATUS = 'idle';
+export const LOADING_STATUS = 'loading';
+export const SUCCESS_STATUS = 'success';
+export const FAILURE_STATUS = 'failure';

src/hooks.js

@@ -1,23 +1,31 @@
 import { useEffect, useState } from 'react';
 
+import {
+  IDLE_STATUS, LOADING_STATUS, SUCCESS_STATUS, FAILURE_STATUS,
+} from './constants';
+
 // eslint-disable-next-line import/prefer-default-export
 export function useAsyncCall(asyncFunc) {
-  const [isLoading, setIsLoading] = useState();
-  const [data, setData] = useState();
+  // React doesn't batch setStates call in async useEffect hooks,
+  // so we use a combined object here to ensure that users
+  // re-render once.
+  const [data, setData] = useState({ status: IDLE_STATUS });
 
   useEffect(
     () => {
       (async () => {
-        setIsLoading(true);
+        setData(currData => ({ ...currData, status: LOADING_STATUS }));
         const response = await asyncFunc();
-        setIsLoading(false);
-        if (response) {
-          setData(response);
+
+        if (Object.keys(response).length === 0) {
+          setData(currData => ({ ...currData, status: FAILURE_STATUS, data: response }));
+        } else {
+          setData(currData => ({ ...currData, status: SUCCESS_STATUS, data: response }));
         }
       })();
     },
     [asyncFunc],
   );
 
-  return [isLoading, data];
+  return data;
 }

src/id-verification/IdVerificationContextProvider.jsx

@@ -5,6 +5,7 @@ import { AppContext } from '@edx/frontend-platform/react';
 import { getProfileDataManager } from '../account-settings/data/service';
 import PageLoading from '../account-settings/PageLoading';
 import { useAsyncCall } from '../hooks';
+import { IDLE_STATUS, LOADING_STATUS, SUCCESS_STATUS } from '../constants';
 
 import { getExistingIdVerification, getEnrollments } from './data/service';
 import AccessBlocked from './AccessBlocked';
@@ -14,17 +15,10 @@ import { VerifiedNameContext } from './VerifiedNameContext';
 
 export default function IdVerificationContextProvider({ children }) {
   const { authenticatedUser } = useContext(AppContext);
-  const { isVerifiedNameHistoryLoading, verifiedName, verifiedNameEnabled } = useContext(VerifiedNameContext);
+  const { verifiedNameHistoryCallStatus, verifiedName, verifiedNameEnabled } = useContext(VerifiedNameContext);
 
-  // Call verification status endpoint to check whether we can verify.
-  const [existingIdVerification, setExistingIdVerification] = useState(null);
-  const [isIDVerificationLoading, idVerificationData] = useAsyncCall(getExistingIdVerification);
-  const [isEnrollmentsLoading, enrollmentsData] = useAsyncCall(getEnrollments);
-  useEffect(() => {
-    if (idVerificationData) {
-      setExistingIdVerification(idVerificationData);
-    }
-  }, [idVerificationData]);
+  const idVerificationData = useAsyncCall(getExistingIdVerification);
+  const enrollmentsData = useAsyncCall(getEnrollments);
 
   const [facePhotoFile, setFacePhotoFile] = useState(null);
   const [idPhotoFile, setIdPhotoFile] = useState(null);
@@ -34,36 +28,6 @@ export default function IdVerificationContextProvider({ children }) {
     hasGetUserMediaSupport ? MEDIA_ACCESS.PENDING : MEDIA_ACCESS.UNSUPPORTED,
   );
 
-  const [canVerify, setCanVerify] = useState(true);
-  const [error, setError] = useState('');
-  useEffect(() => {
-    // With verified name we can redo verification multiple times
-    // if not a successful request prevents re-verification
-    if (!verifiedNameEnabled && existingIdVerification && !existingIdVerification.canVerify) {
-      const { status } = existingIdVerification;
-      setCanVerify(false);
-      if (status === 'pending' || status === 'approved') {
-        setError(ERROR_REASONS.EXISTING_REQUEST);
-      } else {
-        setError(ERROR_REASONS.CANNOT_VERIFY);
-      }
-    } else if (verifiedNameEnabled) {
-      setCanVerify(true);
-    }
-  }, [existingIdVerification, verifiedNameEnabled]);
-
-  useEffect(() => {
-    if (!isEnrollmentsLoading && enrollmentsData) {
-      const verifiedEnrollments = enrollmentsData.filter((enrollment) => (
-        VERIFIED_MODES.includes(enrollment.mode)
-      ));
-      if (verifiedEnrollments.length === 0) {
-        setCanVerify(false);
-        setError(ERROR_REASONS.COURSE_ENROLLMENT);
-      }
-    }
-  }, [enrollmentsData]);
-
   const [profileDataManager, setProfileDataManager] = useState(null);
   useEffect(() => {
     // Determine if the user's profile data is managed by a third-party identity provider.
@@ -92,6 +56,40 @@ export default function IdVerificationContextProvider({ children }) {
   // this flag ensures that they are directed straight back to the summary panel
   const [reachedSummary, setReachedSummary] = useState(false);
 
+  let canVerify = false;
+  let error = '';
+  let existingIdVerification;
+
+  if (idVerificationData?.data) {
+    existingIdVerification = idVerificationData.data;
+  }
+
+  if (verifiedNameHistoryCallStatus === SUCCESS_STATUS && idVerificationData.status === SUCCESS_STATUS) {
+    // With verified name we can redo verification multiple times
+    // if not a successful request prevents re-verification
+    if (!verifiedNameEnabled && existingIdVerification && !existingIdVerification.canVerify) {
+      const { status } = existingIdVerification;
+      canVerify = false;
+      if (status === 'pending' || status === 'approved') {
+        error = ERROR_REASONS.EXISTING_REQUEST;
+      } else {
+        error = ERROR_REASONS.CANNOT_VERIFY;
+      }
+    } else if (verifiedNameEnabled) {
+      canVerify = true;
+    }
+  }
+
+  if (enrollmentsData.status === SUCCESS_STATUS && enrollmentsData?.data) {
+    const verifiedEnrollments = enrollmentsData.data.filter((enrollment) => (
+      VERIFIED_MODES.includes(enrollment.mode)
+    ));
+    if (verifiedEnrollments.length === 0) {
+      canVerify = false;
+      error = ERROR_REASONS.COURSE_ENROLLMENT;
+    }
+  }
+
   const contextValue = {
     existingIdVerification,
     facePhotoFile,
@@ -109,7 +107,6 @@ export default function IdVerificationContextProvider({ children }) {
     portraitPhotoMode,
     idPhotoMode,
     reachedSummary,
-    setExistingIdVerification,
     setFacePhotoFile,
     setIdPhotoFile,
     setIdPhotoName,
@@ -141,8 +138,9 @@ export default function IdVerificationContextProvider({ children }) {
     },
   };
 
-  // If we are waiting for verification status endpoint, show spinner.
-  if (isIDVerificationLoading || isVerifiedNameHistoryLoading) {
+  const loadingStatuses = [IDLE_STATUS, LOADING_STATUS];
+  // If we are waiting for verification status or verified name history endpoint, show spinner.
+  if (loadingStatuses.includes(idVerificationData.status) || loadingStatuses.includes(verifiedNameHistoryCallStatus)) {
     return <PageLoading srMessage="Loading verification status" />;
   }
 

src/id-verification/VerifiedNameContext.jsx

@@ -1,31 +1,31 @@
-import React, { createContext, useEffect, useState } from 'react';
+import React, { createContext } from 'react';
 import PropTypes from 'prop-types';
 
 import { getVerifiedNameHistory } from '../account-settings/data/service';
 import { getMostRecentApprovedOrPendingVerifiedName } from '../utils';
 import { useAsyncCall } from '../hooks';
+import { SUCCESS_STATUS } from '../constants';
 
 export const VerifiedNameContext = createContext();
 
 export function VerifiedNameContextProvider({ children }) {
-  const [verifiedNameEnabled, setVerifiedNameEnabled] = useState(false);
-  const [verifiedName, setVerifiedName] = useState('');
-  const [isVerifiedNameHistoryLoading, verifiedNameHistory] = useAsyncCall(getVerifiedNameHistory);
+  const verifiedNameHistoryData = useAsyncCall(getVerifiedNameHistory);
 
-  useEffect(() => {
-    if (verifiedNameHistory) {
-      const { verified_name_enabled: verifiedNameFeatureEnabled, results } = verifiedNameHistory;
-      setVerifiedNameEnabled(verifiedNameFeatureEnabled);
+  let verifiedNameEnabled = false;
+  let verifiedName = '';
+  const { status, data } = verifiedNameHistoryData;
+  if (status === SUCCESS_STATUS && data) {
+    const { verified_name_enabled: verifiedNameFeatureEnabled, results } = data;
+    verifiedNameEnabled = verifiedNameFeatureEnabled;
 
-      if (verifiedNameFeatureEnabled) {
-        const applicableVerifiedName = getMostRecentApprovedOrPendingVerifiedName(results);
-        setVerifiedName(applicableVerifiedName);
-      }
+    if (verifiedNameFeatureEnabled) {
+      const applicableVerifiedName = getMostRecentApprovedOrPendingVerifiedName(results);
+      verifiedName = applicableVerifiedName;
     }
-  }, [verifiedNameHistory]);
+  }
 
   const value = {
-    isVerifiedNameHistoryLoading,
+    verifiedNameHistoryCallStatus: status,
     verifiedNameEnabled,
     verifiedName,
   };

src/id-verification/data/service.js

@@ -44,7 +44,7 @@ export async function getEnrollments() {
     const { data } = await getAuthenticatedHttpClient().get(url, requestConfig);
     return data;
   } catch (e) {
-    return [];
+    return {};
   }
 }
 

src/id-verification/tests/IdVerificationContextProvider.test.jsx

@@ -16,8 +16,8 @@ jest.mock('../../account-settings/data/service', () => ({
 }));
 
 jest.mock('../data/service', () => ({
-  getExistingIdVerification: jest.fn(),
-  getEnrollments: jest.fn(() => []),
+  getExistingIdVerification: jest.fn(() => ({})),
+  getEnrollments: jest.fn(() => ({})),
 }));
 
 describe('IdVerificationContextProvider', () => {

src/id-verification/tests/VerifiedNameContextProvider.test.jsx

@@ -15,7 +15,7 @@ const VerifiedNameContextTestComponent = () => {
 };
 
 jest.mock('../../account-settings/data/service', () => ({
-  getVerifiedNameHistory: jest.fn(),
+  getVerifiedNameHistory: jest.fn(() => ({})),
 }));
 
 describe('VerifiedNameContextProvider', () => {
@@ -31,11 +31,11 @@ describe('VerifiedNameContextProvider', () => {
 
   it('calls getVerifiedNameHistory', async () => {
     jest.mock('../../account-settings/data/service', () => ({
-      getVerifiedNameHistory: jest.fn(),
+      getVerifiedNameHistory: jest.fn(() => ({})),
     }));
 
     render(<VerifiedNameContextProvider {...defaultProps} />);
-    expect(getVerifiedNameHistory).toHaveBeenCalledTimes(1);
+    await waitFor(() => expect(getVerifiedNameHistory).toHaveBeenCalledTimes(1));
   });
 
   it('sets verifiedName and verifiedNameEnabled correctly when verified name feature enabled', async () => {

src/tests/hooks.test.jsx

@@ -2,15 +2,14 @@ import PropTypes from 'prop-types';
 import { render, waitFor } from '@testing-library/react';
 
 import { useAsyncCall } from '../hooks';
+import { LOADING_STATUS, SUCCESS_STATUS, FAILURE_STATUS } from '../constants';
 
-const TestUseAsyncCallHookComponent = (props) => {
-  const { asyncFunc } = props;
-  const [isCallLoading, callData] = useAsyncCall(asyncFunc);
-
+const TestUseAsyncCallHookComponent = ({ asyncFunc }) => {
+  const { status, data } = useAsyncCall(asyncFunc);
   return (
     <>
-      { isCallLoading && <div>loading</div> }
-      <div>{ callData }</div>
+      <div>{status}</div>
+      {data && Object.keys(data).length !== 0 && <div data-testid="data">{ data.data }</div>}
     </>
   );
 };
@@ -20,27 +19,31 @@ TestUseAsyncCallHookComponent.propTypes = {
 };
 
 describe('useAsyncCall mock', () => {
-  it('returns data correctly for response', async () => {
-    const mockAsyncFunc = jest.fn(async () => ('data'));
+  it('returns status and data correctly for successful response', async () => {
+    const mockAsyncFunc = jest.fn(async () => ({ data: 'data' }));
 
     const { queryByText } = render(<TestUseAsyncCallHookComponent asyncFunc={mockAsyncFunc} />);
 
     await waitFor(() => (expect(mockAsyncFunc).toHaveBeenCalledTimes(1)));
+    expect(queryByText(SUCCESS_STATUS)).not.toBeNull();
     expect(queryByText('data')).not.toBeNull();
   });
-  it('returns data correctly for no response', async () => {
-    const mockAsyncFunc = jest.fn(async () => {});
+  it('returns status and data correctly for unsuccessful response', async () => {
+    const mockAsyncFunc = jest.fn(async () => ({}));
 
-    const { queryByText } = render(<TestUseAsyncCallHookComponent asyncFunc={mockAsyncFunc} />);
+    const { queryByText, queryByTestId } = render(<TestUseAsyncCallHookComponent asyncFunc={mockAsyncFunc} />);
 
     await waitFor(() => (expect(mockAsyncFunc).toHaveBeenCalledTimes(1)));
-    expect(queryByText('data')).toBeNull();
+    expect(queryByText(FAILURE_STATUS)).not.toBeNull();
+    expect(queryByTestId('data')).toBeNull();
   });
-  it('returns isLoading correctly', async () => {
-    const mockAsyncFunc = jest.fn(async () => {});
+  it('returns status and data correctly for pending request', async () => {
+    const mockAsyncFunc = jest.fn(async () => ({}));
 
-    const { queryByText } = render(<TestUseAsyncCallHookComponent asyncFunc={mockAsyncFunc} />);
-    expect(queryByText('loading')).not.toBeNull();
-    expect(queryByText('data')).toBeNull();
+    const { queryByText, queryByTestId } = render(<TestUseAsyncCallHookComponent asyncFunc={mockAsyncFunc} />);
+    expect(queryByText(LOADING_STATUS)).not.toBeNull();
+    expect(queryByTestId('data')).toBeNull();
+
+    await waitFor(() => (expect(mockAsyncFunc).toHaveBeenCalledTimes(1)));
   });
 });