fix: pollAttempt call on download page #113
Conversation
Zacharis278
force-pushed
the
zhancock/latest-attempt-fix
branch
from
7cb040b
to
7ff6963
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #113 +/- ##
==========================================
- Coverage 93.84% 93.78% -0.06%
==========================================
Files 71 71
Lines 1023 1030 +7
Branches 280 281 +1
==========================================
+ Hits 960 966 +6
- Misses 58 59 +1
Partials 5 5
☔ View full report in Codecov by Sentry. |
Zacharis278
force-pushed
the
zhancock/latest-attempt-fix
branch
3 times, most recently
from
1ab724f
to
2815b5c
Compare
| const activeAttemptUrl = new URL(`${getConfig().EXAMS_BASE_URL}/api/v1/exams/attempt/latest`); | ||
| const activeAttemptResponse = await getAuthenticatedHttpClient().get(activeAttemptUrl.href); | ||
| return activeAttemptResponse.data; | ||
| } | ||
|
|
||
| async function fetchLatestExamAttempt(sequenceId) { |
There was a problem hiding this comment.
basically the same thing as the above function. I broke this out to make reading the behavior a bit easier
There was a problem hiding this comment.
Would it be worth adding a comment noting this so that people don't confused by these functions having similar functionality later on?
| @@ -50,7 +50,7 @@ const DownloadSoftwareProctoredExamInstructions = ({ intl, skipProctoredExam }) | |||
| ? `${getConfig().EXAMS_BASE_URL}/lti/start_proctoring/${attemptId}` : downloadUrl; | |||
|
|
|||
| const handleDownloadClick = () => { | |||
| pollExamAttempt(`${pollUrl}?sourceid=instructions`) | |||
| pollExamAttempt(pollUrl, sequenceId) | |||
There was a problem hiding this comment.
sourceid=instructions doesn't do anything. Was just copied behavior from the old frontend.
src/data/api.js
Outdated
|
|
||
| // 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; | ||
| } | ||
| } else { | ||
| logError('pollExamAttempt called but no pollUrl is set'); |
There was a problem hiding this comment.
I feel like this error msg could be improved by describing that we intended to call the legacy system specifically (which from what I can tell is what requires the pollUrl value), not just this API function.
| const activeAttemptUrl = new URL(`${getConfig().EXAMS_BASE_URL}/api/v1/exams/attempt/latest`); | ||
| const activeAttemptResponse = await getAuthenticatedHttpClient().get(activeAttemptUrl.href); | ||
| return activeAttemptResponse.data; | ||
| } | ||
|
|
||
| async function fetchLatestExamAttempt(sequenceId) { |
There was a problem hiding this comment.
Would it be worth adding a comment noting this so that people don't confused by these functions having similar functionality later on?
Zacharis278
force-pushed
the
zhancock/latest-attempt-fix
branch
2 times, most recently
from
af1abf7
to
e3f3221
Compare
Zacharis278
force-pushed
the
zhancock/latest-attempt-fix
branch
from
568abf7
to
9645672
Compare
|
🎉 This PR is included in version 2.20.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* fix: pollAttempt call on download page * feat: review cleanup * fix: string format
MST-1951
pair PR with edx/frontend-app-exams-dashboard#11
The network call made by the download interstitial is a bit of a special snowflake it bypasses the thunk and calls the api directly. Added support to the existing 'poll' function to support requesting a specific exam by sequence id in this case.