From 7f6f44290fdc0088b65d0cbfd446279059baaf26 Mon Sep 17 00:00:00 2001
From: Zachary Hancock <zhancock@edx.org>
Date: Mon, 8 Jan 2024 10:47:23 -0500
Subject: [PATCH] feat: reduce unnecessary api calls (#129)

* feat: reduce unnecessary api calls

* test: fix broken test
---
 src/exam/ExamWrapper.jsx      |  5 ++-
 src/exam/ExamWrapper.test.jsx | 57 ++++++++++++++++++++++++++++++++---
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/src/exam/ExamWrapper.jsx b/src/exam/ExamWrapper.jsx
index 05bde2f4..68b7eee8 100644
--- a/src/exam/ExamWrapper.jsx
+++ b/src/exam/ExamWrapper.jsx
@@ -27,7 +27,10 @@ const ExamWrapper = ({ children, ...props }) => {
   const isGated = sequence && sequence.gatedContent !== undefined && sequence.gatedContent.gated;
 
   useEffect(() => {
-    loadInitialData();
+    // fetch exam data on exam sequences or if no exam data has been fetched yet
+    if (sequence.isTimeLimited || state.isLoading) {
+      loadInitialData();
+    }
   // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
diff --git a/src/exam/ExamWrapper.test.jsx b/src/exam/ExamWrapper.test.jsx
index 33e30f09..bee6ed74 100644
--- a/src/exam/ExamWrapper.test.jsx
+++ b/src/exam/ExamWrapper.test.jsx
@@ -2,16 +2,28 @@ import '@testing-library/jest-dom';
 import { Factory } from 'rosie';
 import React from 'react';
 import SequenceExamWrapper from './ExamWrapper';
-import { store, getExamAttemptsData, startTimedExam } from '../data';
-import { render } from '../setupTest';
+import { store, startTimedExam } from '../data';
+import { getExamAttemptsData } from '../data/thunks';
+import { render, waitFor } from '../setupTest';
 import ExamStateProvider from '../core/ExamStateProvider';
 import { ExamStatus, ExamType } from '../constants';
 
 jest.mock('../data', () => ({
   store: {},
-  getExamAttemptsData: jest.fn(),
   startTimedExam: jest.fn(),
 }));
+
+// because of the way ExamStateProvider and other locations inconsistantly import from
+// thunks directly instead of using the data module we need to mock the underlying
+// thunk file. It would be nice to clean this up in the future.
+jest.mock('../data/thunks', () => {
+  const originalModule = jest.requireActual('../data/thunks');
+  return {
+    ...originalModule,
+    getExamAttemptsData: jest.fn(),
+  };
+});
+
 getExamAttemptsData.mockReturnValue(jest.fn());
 startTimedExam.mockReturnValue(jest.fn());
 store.subscribe = jest.fn();
@@ -24,10 +36,15 @@ describe('SequenceExamWrapper', () => {
   };
   const courseId = 'course-v1:test+test+test';
 
-  it('is successfully rendered and shows instructions if the user is not staff', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
     store.getState = () => ({
       examState: Factory.build('examState'),
+      isLoading: false,
     });
+  });
+
+  it('is successfully rendered and shows instructions if the user is not staff', () => {
     const { queryByTestId } = render(
       <ExamStateProvider>
         <SequenceExamWrapper sequence={sequence} courseId={courseId}>
@@ -114,6 +131,38 @@ describe('SequenceExamWrapper', () => {
     expect(queryByTestId('exam-api-error-component')).not.toBeInTheDocument();
   });
 
+  it('does not fetch exam data if already loaded and the sequence is not an exam', async () => {
+    render(
+      <ExamStateProvider>
+        <SequenceExamWrapper sequence={{ ...sequence, isTimeLimited: false }} courseId={courseId}>
+          <div>children</div>
+        </SequenceExamWrapper>
+      </ExamStateProvider>,
+      { store },
+    );
+    // assert the exam data is not fetched
+    await expect(waitFor(() => expect(getExamAttemptsData).toHaveBeenCalled())).rejects.toThrow();
+  });
+
+  it('does fetch exam data for non exam sequences if not already loaded', async () => {
+    // this would only occur if the user deeplinks directly to a non-exam sequence
+    store.getState = () => ({
+      examState: Factory.build('examState', {
+        isLoading: true,
+      }),
+    });
+
+    render(
+      <ExamStateProvider>
+        <SequenceExamWrapper sequence={{ ...sequence, isTimeLimited: false }} courseId={courseId}>
+          <div>children</div>
+        </SequenceExamWrapper>
+      </ExamStateProvider>,
+      { store },
+    );
+    await waitFor(() => expect(getExamAttemptsData).toHaveBeenCalled());
+  });
+
   it('does not take any actions if sequence item is not exam', () => {
     const { getByTestId } = render(
       <ExamStateProvider>