Skip to content

Commit

Permalink
fix: workaround for pollAttempt fail in courseview (#142)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ilee2u authored Mar 21, 2024
1 parent 52826e0 commit 10dd052
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 27 deletions.
3 changes: 1 addition & 2 deletions src/core/OuterExamTimer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
37 changes: 21 additions & 16 deletions src/data/api.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down
12 changes: 8 additions & 4 deletions src/data/redux.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Expand Down
2 changes: 0 additions & 2 deletions src/data/thunks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 7 additions & 3 deletions src/timer/TimerProvider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -143,7 +148,6 @@ const TimerProvider = ({
timerRef.current = null;
};
}, [
timerEnds,
pingInterval,
workerUrl,
processTimeLeft,
Expand Down

0 comments on commit 10dd052

Please sign in to comment.