From 10dd0525eed205740057aa3bf1c1e770a10b72bf Mon Sep 17 00:00:00 2001 From: Isaac Lee <124631592+ilee2u@users.noreply.github.com> Date: Thu, 21 Mar 2024 13:05:45 -0400 Subject: [PATCH] fix: workaround for pollAttempt fail in courseview (#142) * fix: workaround for pollAttempt fail in courseview - Combined fetchActiveAttempt & fetchLatestExamAttempt into one function with an optional "sequenceId" variable. At the very least, this works without causing errors in non-exam-sequence views. Since the sequenceId is undefined in that case, due to the sequenceId needing to be extracted from the sequence page itself, we can at least still get the latest exam attempt data. - To my best knowledge, we originally called for the attempt of an exam with a given sequenceId in order to get the latest attempt for an exam regardless of whether or not its active, such that the timer can be seen on non exam-sequence pages. However, we don't truly need the sequence_id for this. - It's also possible that this was also designed this way so that we could make sure a learner hadn't started two exams at once, and so their "latest exam attempt" would pretain to the exam they started most recently. We already have safeguards for this in edx_exams (See: https://github.com/edx/edx-exams/blob/main/edx_exams/apps/core/api.py#L201). * temp: comments on what to do next * test: fixed tests to match new code - also removed timerEnds param from TimerProvider useEffect, replaced it with a lint ignore * chore: remove extra comments * fix: use reference for deadline --- src/core/OuterExamTimer.jsx | 3 +-- src/data/api.js | 37 +++++++++++++++++++++---------------- src/data/redux.test.jsx | 12 ++++++++---- src/data/thunks.js | 2 -- src/timer/TimerProvider.jsx | 10 +++++++--- 5 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/core/OuterExamTimer.jsx b/src/core/OuterExamTimer.jsx index 77ecc992..425bc689 100644 --- a/src/core/OuterExamTimer.jsx +++ b/src/core/OuterExamTimer.jsx @@ -8,10 +8,9 @@ import { getLatestAttemptData } from '../data'; import { IS_STARTED_STATUS } from '../constants'; const ExamTimer = ({ courseId }) => { - const { activeAttempt } = useSelector(state => state.specialExams); + const { activeAttempt, apiErrorMsg } = useSelector(state => state.specialExams); const { authenticatedUser } = useContext(AppContext); const showTimer = !!(activeAttempt && IS_STARTED_STATUS(activeAttempt.attempt_status)); - const { apiErrorMsg } = useSelector(state => state.specialExams); const dispatch = useDispatch(); useEffect(() => { diff --git a/src/data/api.js b/src/data/api.js index 5cd62c42..a01247de 100644 --- a/src/data/api.js +++ b/src/data/api.js @@ -1,6 +1,5 @@ import { getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { logError } from '@edx/frontend-platform/logging'; import { ExamAction } from '../constants'; import { generateHumanizedTime } from '../helpers'; @@ -13,14 +12,14 @@ async function fetchActiveAttempt() { return activeAttemptResponse.data; } -async function fetchLatestExamAttempt(sequenceId) { +async function fetchAttemptForExamSequnceId(sequenceId) { + const attemptUrl = new URL(`${getConfig().EXAMS_BASE_URL}/api/v1/exams/attempt/latest`); // the calls the same endpoint as fetchActiveAttempt but it behaves slightly different // with an exam's section specified. The attempt for that requested exam is always returned // even if it is not 'active' (timer is not running) - const attemptUrl = new URL(`${getConfig().EXAMS_BASE_URL}/api/v1/exams/attempt/latest`); attemptUrl.searchParams.append('content_id', sequenceId); - const response = await getAuthenticatedHttpClient().get(attemptUrl.href); - return response.data; + const attemptResponse = await getAuthenticatedHttpClient().get(attemptUrl.href); + return attemptResponse.data; } export async function fetchExamAttemptsData(courseId, sequenceId) { @@ -70,25 +69,31 @@ export async function fetchLatestAttempt(courseId) { export async function pollExamAttempt(pollUrl, sequenceId) { let data; + + // sites configured with only edx-proctoring must have pollUrl set if (pollUrl) { const edxProctoringURL = new URL( `${getConfig().LMS_BASE_URL}${pollUrl}`, ); const urlResponse = await getAuthenticatedHttpClient().get(edxProctoringURL.href); data = urlResponse.data; - } else if (sequenceId && getConfig().EXAMS_BASE_URL) { - data = await fetchLatestExamAttempt(sequenceId); - - // Update dictionaries returned by edx-exams to have correct status key for legacy compatibility - if (data.attempt_status) { - data.status = data.attempt_status; - delete data.attempt_status; - } + + return data; + + // exams configured with edx-exams expect sequenceId if pollUrl is not set when viewing the exam sequence + } if (sequenceId) { + data = await fetchAttemptForExamSequnceId(sequenceId); + // outside the exam sequence, we can't get the sequenceId easily, so here we just call the last active attempt } else { - // sites configured with only edx-proctoring must have pollUrl set - // sites configured with edx-exams expect sequenceId if pollUrl is not set - logError(`pollExamAttempt recieved unexpected parameters pollUrl=${pollUrl} sequenceId=${sequenceId}`); + data = await fetchActiveAttempt(); + } + + // Update dictionaries returned by edx-exams to have correct status key for legacy compatibility + if (data.attempt_status) { + data.status = data.attempt_status; + delete data.attempt_status; } + return data; } diff --git a/src/data/redux.test.jsx b/src/data/redux.test.jsx index bffa0cde..6a036a03 100644 --- a/src/data/redux.test.jsx +++ b/src/data/redux.test.jsx @@ -990,10 +990,14 @@ describe('Data layer integration tests', () => { await api.pollExamAttempt(null, sequenceId); expect(axiosMock.history.get[0].url).toEqual(expectedUrl); }); - test('pollUrl is required if edx-exams in not enabled, an error should be logged', async () => { - mergeConfig({ EXAMS_BASE_URL: null }); - api.pollExamAttempt(null, null); - expect(loggingService.logError).toHaveBeenCalled(); + test('should call the latest attempt w/o a sequence if if neither a pollUrl or sequence id is provided', async () => { + const expectedUrl = `${getConfig().EXAMS_BASE_URL}/api/v1/exams/attempt/latest`; + axiosMock.onGet(expectedUrl).reply(200, { + time_remaining_seconds: 1739.9, + status: ExamStatus.STARTED, + }); + await api.pollExamAttempt(null); + expect(axiosMock.history.get[0].url).toEqual(expectedUrl); }); }); }); diff --git a/src/data/thunks.js b/src/data/thunks.js index c0628b7f..31759531 100644 --- a/src/data/thunks.js +++ b/src/data/thunks.js @@ -271,8 +271,6 @@ export function pollAttempt(url) { } try { - // TODO: make sure sequenceId pulled here is correct both in-exam-sequence and in outline - // test w/ timed exam const { exam } = getState().specialExams; const data = await pollExamAttempt(url, exam.content_id); if (!data) { diff --git a/src/timer/TimerProvider.jsx b/src/timer/TimerProvider.jsx index 74fbc87e..2671fe2b 100644 --- a/src/timer/TimerProvider.jsx +++ b/src/timer/TimerProvider.jsx @@ -95,15 +95,20 @@ const TimerProvider = ({ timeLimitMins, ]); + // Set deadline as a reference to timerEnds that updates when it changes + const deadline = useRef(new Date(timerEnds)); + useEffect(() => { + deadline.current = new Date(timerEnds); + }, [timerEnds]); + useEffect(() => { const timerRef = { current: true }; let timerTick = -1; - const deadline = new Date(timerEnds); const ticker = () => { timerTick++; const now = Date.now(); - const remainingTime = (deadline.getTime() - now) / 1000; + const remainingTime = (deadline.current.getTime() - now) / 1000; const secondsLeft = Math.floor(remainingTime); setTimeState(getFormattedRemainingTime(secondsLeft)); @@ -143,7 +148,6 @@ const TimerProvider = ({ timerRef.current = null; }; }, [ - timerEnds, pingInterval, workerUrl, processTimeLeft,