From f098b74c19470c87f03d4356e174bd9e0fc167d0 Mon Sep 17 00:00:00 2001 From: Michael Roytman Date: Thu, 20 Mar 2025 14:26:50 -0400 Subject: [PATCH] fix: useExamAccess hook does not make call to fetchExamAccessToken after isExam changes This commit fixes a bug that prevents the fetchExamAccessToken hook in the useExamAccess hook from running. This occurs when the value of isExam returned by the useIsExam hook changes from false to true. Because the dependency array of the useEffect hook within the useExamAccess hook was ['id'], the useEffect hook would not rerun on changes to isExam. The fix is to add isExam to the dependency array. --- .../sequence/Unit/hooks/useExamAccess.js | 7 +- .../Unit/hooks/useExamAccess.test.jsx | 75 +++++++++++++++---- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/courseware/course/sequence/Unit/hooks/useExamAccess.js b/src/courseware/course/sequence/Unit/hooks/useExamAccess.js index ff9408be83..9a50ea674b 100644 --- a/src/courseware/course/sequence/Unit/hooks/useExamAccess.js +++ b/src/courseware/course/sequence/Unit/hooks/useExamAccess.js @@ -18,9 +18,10 @@ const useExamAccess = ({ const fetchExamAccessToken = useFetchExamAccessToken(); // NOTE: We cannot use this hook in the useEffect hook below to grab the updated exam access token in the finally - // block, due to the rules of hooks. Instead, we get the value of the exam access token from a call to the hook. + // block, due to the rules of hooks. Instead, we get the value of the exam access token from a call to + // the hook below. // When the fetchExamAccessToken call completes, the useExamAccess hook will re-run - // (due to a change to the Redux store, and, thus, a change to the the context), at which point the updated + // (due to a change to the Redux store, and, thus, a change to the context), at which point the updated // exam access token will be fetched via the useExamAccessToken hook call below. // The important detail is that there should never be a return value (false, ''). const examAccessToken = useExamAccessToken(); @@ -35,7 +36,7 @@ const useExamAccess = ({ logError(error); }); } - }, [id]); + }, [id, isExam]); return { blockAccess, diff --git a/src/courseware/course/sequence/Unit/hooks/useExamAccess.test.jsx b/src/courseware/course/sequence/Unit/hooks/useExamAccess.test.jsx index 1a746242ff..d6d4085270 100644 --- a/src/courseware/course/sequence/Unit/hooks/useExamAccess.test.jsx +++ b/src/courseware/course/sequence/Unit/hooks/useExamAccess.test.jsx @@ -1,5 +1,6 @@ +import { useState } from 'react'; import { logError } from '@edx/frontend-platform/logging'; -import { renderHook } from '@testing-library/react-hooks'; +import { act, renderHook } from '@testing-library/react-hooks'; import { useExamAccessToken, useFetchExamAccessToken, useIsExam } from '@edx/frontend-lib-special-exams'; import { initializeMockApp } from '../../../../../setupTest'; @@ -16,10 +17,27 @@ jest.mock('@edx/frontend-lib-special-exams', () => ({ const id = 'test-id'; +// This object allows us to manipulate the value of the accessToken. +const testAccessToken = { curr: '' }; + const mockFetchExamAccessToken = jest.fn().mockImplementation(() => Promise.resolve()); useFetchExamAccessToken.mockReturnValue(mockFetchExamAccessToken); -const testAccessToken = 'test-access-token'; +const mockUseIsExam = (initialState = false) => { + const [isExam, setIsExam] = useState(initialState); + + // This setTimeout block is intended to replicate the case where a unit is an exam, but + // the call to fetch exam metadata has not yet completed. That is the value of isExam starts + // as false and transitions to true once the call resolves. + if (!initialState) { + setTimeout( + () => setIsExam(true), + 500, + ); + } + + return isExam; +}; describe('useExamAccess hook', () => { beforeAll(async () => { @@ -28,13 +46,13 @@ describe('useExamAccess hook', () => { }); beforeEach(() => { jest.clearAllMocks(); - - // Mock implementations from previous test runs may not have been "consumed", so reset mock implementations. + jest.useFakeTimers(); useExamAccessToken.mockReset(); - useExamAccessToken.mockReturnValueOnce(''); - useExamAccessToken.mockReturnValueOnce(testAccessToken); + + useIsExam.mockImplementation(() => mockUseIsExam()); + useExamAccessToken.mockImplementation(() => testAccessToken.curr); }); - describe.only('behavior', () => { + describe('behavior', () => { it('returns accessToken and blockAccess and doesn\'t call token API if not an exam', () => { const { result } = renderHook(() => useExamAccess({ id })); const { accessToken, blockAccess } = result.current; @@ -43,18 +61,23 @@ describe('useExamAccess hook', () => { expect(blockAccess).toBe(false); expect(mockFetchExamAccessToken).not.toHaveBeenCalled(); }); - it('returns true for blockAccess if an exam but accessToken not yet fetched', () => { - useIsExam.mockImplementation(() => (true)); + it('returns true for blockAccess if an exam but accessToken not yet fetched', async () => { + useIsExam.mockImplementation(() => mockUseIsExam(true)); - const { result } = renderHook(() => useExamAccess({ id })); + const { result, waitForNextUpdate } = renderHook(() => useExamAccess({ id })); const { accessToken, blockAccess } = result.current; expect(accessToken).toEqual(''); expect(blockAccess).toBe(true); expect(mockFetchExamAccessToken).toHaveBeenCalled(); + + // This is to get rid of the act(...) warning. + await act(async () => { + await waitForNextUpdate(); + }); }); it('returns false for blockAccess if an exam and accessToken fetch succeeds', async () => { - useIsExam.mockImplementation(() => (true)); + useIsExam.mockImplementation(() => mockUseIsExam(true)); const { result, waitForNextUpdate } = renderHook(() => useExamAccess({ id })); // We wait for the promise to resolve and for updates to state to complete so that blockAccess is updated. @@ -62,12 +85,36 @@ describe('useExamAccess hook', () => { const { accessToken, blockAccess } = result.current; - expect(accessToken).toEqual(testAccessToken); + expect(accessToken).toEqual(testAccessToken.curr); + expect(blockAccess).toBe(false); + expect(mockFetchExamAccessToken).toHaveBeenCalled(); + }); + it('in progress', async () => { + const { result, waitForNextUpdate } = renderHook(() => useExamAccess({ id })); + + let { accessToken, blockAccess } = result.current; + + expect(accessToken).toEqual(''); + expect(blockAccess).toBe(false); + expect(mockFetchExamAccessToken).not.toHaveBeenCalled(); + + testAccessToken.curr = 'test-access-token'; + + // The runAllTimers will update the value of isExam, and the waitForNextUpdate will + // wait for call to setBlockAccess in the finally clause of useEffect hook. + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + ({ accessToken, blockAccess } = result.current); + + expect(accessToken).toEqual('test-access-token'); expect(blockAccess).toBe(false); expect(mockFetchExamAccessToken).toHaveBeenCalled(); }); it('returns false for blockAccess if an exam and accessToken fetch fails', async () => { - useIsExam.mockImplementation(() => (true)); + useIsExam.mockImplementation(() => mockUseIsExam(true)); const testError = 'test-error'; mockFetchExamAccessToken.mockImplementationOnce(() => Promise.reject(testError)); @@ -79,7 +126,7 @@ describe('useExamAccess hook', () => { const { accessToken, blockAccess } = result.current; - expect(accessToken).toEqual(testAccessToken); + expect(accessToken).toEqual(testAccessToken.curr); expect(blockAccess).toBe(false); expect(mockFetchExamAccessToken).toHaveBeenCalled(); expect(logError).toHaveBeenCalledWith(testError);