Skip to content

Commit

Permalink
feat: [BD-26] Improve handling of API errors, refactor redux store in…
Browse files Browse the repository at this point in the history
…itialization, fix multiple API calls, add new pages (#21)

* [BD-26][EDUCATOR-5808] Add expired and onboarding failed pages (#37)

* feat: added expired page

* feat: added onboarding error page

* [BD-26] Refactor API calls and redux store initialization (#38)

* feat: improve handling of API errors

* docs: update decision docs

* tests: update tests

* tests: add tests for ExamApiError component and small refactoring of tests for expired page
  • Loading branch information
viktorrusakov authored Jun 15, 2021
1 parent 5ea7bec commit f250975
Show file tree
Hide file tree
Showing 17 changed files with 703 additions and 74 deletions.
1 change: 1 addition & 0 deletions docs/decisions/0001-library-rationale.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ with the ability to take control over the rendering process and show exam-relate
Usage example is

.. code-block:: javascript
<LearningApp> // Learning app
...
<SequenceExamWrapper>
Expand Down
51 changes: 51 additions & 0 deletions docs/decisions/0003-api-errors-handling.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
3. Handling of API errors
-------------------------

Context
-------
During the development of the library we realized that sometimes the backend API could fail
for some reason and we have to properly notify the user about it and show the error message,
so they could contact support to get it fixed.

Decision
--------
The decision was made to keep a separate field in our redux store for error messages. Each time
the API fails to respond with success status, the action is dispatched to populate the
field with an appropriate message which in turn gets rendered in the separate component
(located in src/exam/ExamAPIError.jsx).

Keeping the message in redux store allows for simple usage, all you need to do
is to add a catch block with error handler in it to the API call in your thunk function
and it will take care of rendering the error in case there is one.

The handler is located in src/data/thunks.js and looks as follows

.. code-block:: javascript
function handleAPIError(error, dispatch) {
const { message, detail } = error;
dispatch(setApiError({ errorMsg: message || detail }));
}
..
For example, to render an error which comes from the API directly you would do:

.. code-block:: javascript
try {
const data = await fetchExamReviewPolicy(exam.id);
} catch (error) {
handleAPIError(error, dispatch);
}
..
if you want to render custom message:

.. code-block:: javascript
try {
const data = await fetchExamReviewPolicy(exam.id);
} catch (error) {
handleAPIError({ message: 'Your custom error message' }, dispatch);
}
..
17 changes: 17 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable import/prefer-default-export */
export const ExamStatus = Object.freeze({
ELIGIBLE: 'eligible',
CREATED: 'created',
DOWNLOAD_SOFTWARE_CLICKED: 'download_software_clicked',
READY_TO_START: 'ready_to_start',
Expand All @@ -11,9 +12,25 @@ export const ExamStatus = Object.freeze({
REJECTED: 'rejected',
ERROR: 'error',
READY_TO_RESUME: 'ready_to_resume',
ONBOARDING_MISSING: 'onboarding_missing',
ONBOARDING_PENDING: 'onboarding_pending',
ONBOARDING_FAILED: 'onboarding_failed',
ONBOARDING_EXPIRED: 'onboarding_expired',
});

export const INCOMPLETE_STATUSES = [
ExamStatus.ELIGIBLE, ExamStatus.CREATED, ExamStatus.DOWNLOAD_SOFTWARE_CLICKED,
ExamStatus.READY_TO_START, ExamStatus.STARTED, ExamStatus.READY_TO_SUBMIT,
];

export const ONBOARDING_ERRORS = [
ExamStatus.ONBOARDING_EXPIRED, ExamStatus.ONBOARDING_FAILED,
ExamStatus.ONBOARDING_MISSING, ExamStatus.ONBOARDING_PENDING,
];

export const IS_STARTED_STATUS = (status) => [ExamStatus.STARTED, ExamStatus.READY_TO_SUBMIT].includes(status);
export const IS_INCOMPLETE_STATUS = (status) => INCOMPLETE_STATUSES.includes(status);
export const IS_ONBOARDING_ERROR = (status) => ONBOARDING_ERRORS.includes(status);

// Available actions are taken from
// https://github.com/edx/edx-proctoring/blob/1444ca40a43869fb4e2731cea4862888c5b5f286/edx_proctoring/views.py#L765
Expand Down
66 changes: 62 additions & 4 deletions src/data/slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,69 @@ export const examSlice = createSlice({
initialState: {
isLoading: true,
timeIsOver: false,
activeAttempt: null,
activeAttempt: null, // has the same structure as attempt in exam object
allowProctoringOptOut: false,
proctoringSettings: {},
exam: {},
verification: {},
proctoringSettings: {
platform_name: '',
contact_us: '',
link_urls: null,
exam_proctoring_backend: {
download_url: '',
instructions: [],
name: '',
rules: {},
},
provider_tech_support_email: '',
provider_tech_support_phone: '',
provider_name: '',
learner_notification_from_email: '',
integration_specific_email: '',
},
exam: {
id: null,
course_id: '',
content_id: '',
external_id: '',
exam_name: '',
time_limit_mins: null,
is_proctored: false,
is_practice_exam: false,
is_active: true,
due_date: null,
hide_after_due: false,
backend: '',
prerequisite_status: {
are_prerequisites_satisifed: true,
satisfied_prerequisites: [],
failed_prerequisites: [],
pending_prerequisites: [],
declined_prerequisites: [],
},
attempt: {
in_timed_exam: true,
taking_as_proctored: true,
exam_type: '',
exam_display_name: '',
exam_url_path: '',
time_remaining_seconds: null,
low_threshold_sec: null,
critically_low_threshold_sec: null,
course_id: '',
attempt_id: null,
accessibility_time_string: '',
attempt_status: '',
exam_started_poll_url: '',
desktop_application_js_url: '',
ping_interval: null,
attempt_code: '',
},
type: '',
},
verification: {
status: '',
can_verify: false,
expires: '',
},
apiErrorMsg: '',
},
reducers: {
Expand Down
70 changes: 47 additions & 23 deletions src/data/thunks.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,18 @@ export function getProctoringSettings() {
const { exam } = getState().examState;
if (!exam.id) {
logError('Failed to get exam settings. No exam id.');
handleAPIError(
{ message: 'Failed to fetch proctoring settings. No exam id was found.' },
dispatch,
);
return;
}
dispatch(setIsLoading({ isLoading: true }));
const proctoringSettings = await fetchProctoringSettings(exam.id);
dispatch(setProctoringSettings({ proctoringSettings }));
dispatch(setIsLoading({ isLoading: false }));
try {
const proctoringSettings = await fetchProctoringSettings(exam.id);
dispatch(setProctoringSettings({ proctoringSettings }));
} catch (error) {
handleAPIError(error, dispatch);
}
};
}

Expand Down Expand Up @@ -183,20 +189,22 @@ export function pollAttempt(url) {
return;
}

const data = await pollExamAttempt(url).catch(
error => handleAPIError(error, dispatch),
);
const updatedAttempt = {
...currentAttempt,
time_remaining_seconds: data.time_remaining_seconds,
accessibility_time_string: data.accessibility_time_string,
attempt_status: data.status,
};
dispatch(setActiveAttempt({
activeAttempt: updatedAttempt,
}));
if (data.status === ExamStatus.SUBMITTED) {
dispatch(expireExamAttempt());
try {
const data = await pollExamAttempt(url);
const updatedAttempt = {
...currentAttempt,
time_remaining_seconds: data.time_remaining_seconds,
accessibility_time_string: data.accessibility_time_string,
attempt_status: data.status,
};
dispatch(setActiveAttempt({
activeAttempt: updatedAttempt,
}));
if (data.status === ExamStatus.SUBMITTED) {
dispatch(expireExamAttempt());
}
} catch (error) {
handleAPIError(error, dispatch);
}
};
}
Expand Down Expand Up @@ -292,7 +300,7 @@ export function expireExam() {
if (!attemptId) {
logError('Failed to expire exam. No attempt id.');
handleAPIError(
{ message: 'Failed to expire exam. No attempt id was provided.' },
{ message: 'Failed to expire exam. No attempt id was found.' },
dispatch,
);
return;
Expand Down Expand Up @@ -334,6 +342,10 @@ export function startProctoringSoftwareDownload() {
const attemptId = exam.attempt.attempt_id;
if (!attemptId) {
logError('Failed to start downloading proctoring software. No attempt id.');
handleAPIError(
{ message: 'Failed to start downloading proctoring software. No attempt id was found.' },
dispatch,
);
return;
}
await updateAttemptAfter(
Expand All @@ -344,8 +356,12 @@ export function startProctoringSoftwareDownload() {

export function getVerificationData() {
return async (dispatch) => {
const data = await fetchVerificationStatus();
dispatch(setVerificationData({ verification: data }));
try {
const data = await fetchVerificationStatus();
dispatch(setVerificationData({ verification: data }));
} catch (error) {
handleAPIError(error, dispatch);
}
};
}

Expand All @@ -354,10 +370,18 @@ export function getExamReviewPolicy() {
const { exam } = getState().examState;
if (!exam.id) {
logError('Failed to fetch exam review policy. No exam id.');
handleAPIError(
{ message: 'Failed to fetch exam review policy. No exam id was found.' },
dispatch,
);
return;
}
const data = await fetchExamReviewPolicy(exam.id);
dispatch(setReviewPolicy({ policy: data.review_policy }));
try {
const data = await fetchExamReviewPolicy(exam.id);
dispatch(setReviewPolicy({ policy: data.review_policy }));
} catch (error) {
handleAPIError(error, dispatch);
}
};
}

Expand Down
24 changes: 21 additions & 3 deletions src/exam/Exam.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { useContext } from 'react';
import React, { useContext, useEffect } from 'react';
import PropTypes from 'prop-types';
import { Spinner } from '@edx/paragon';
import { ExamTimerBlock } from '../timer';
import Instructions from '../instructions';
import ExamStateContext from '../context';
import ExamAPIError from './ExamAPIError';
import { ExamType } from '../constants';

/**
* Exam component is intended to render exam instructions before and after exam.
Expand All @@ -18,10 +19,27 @@ import ExamAPIError from './ExamAPIError';
const Exam = ({ isTimeLimited, children }) => {
const state = useContext(ExamStateContext);
const {
isLoading, activeAttempt, showTimer, stopExam,
isLoading, activeAttempt, showTimer, stopExam, exam,
expireExam, pollAttempt, apiErrorMsg, pingAttempt,
getVerificationData, getAllowProctoringOptOut, getProctoringSettings,
} = state;

const { type: examType, content_id: sequenceId, id: examId } = exam || {};

useEffect(() => {
if (examId) {
getProctoringSettings();
}
if (examType === ExamType.PROCTORED) {
getVerificationData();
getAllowProctoringOptOut(sequenceId);
}

// this makes sure useEffect gets called only one time after the exam has been fetched
// we can't leave this empty since initially exam is just an empty object, so
// API calls above would not get triggered
}, [examId]);

if (isLoading) {
return (
<div className="d-flex justify-content-center align-items-center flex-column my-5 py-5">
Expand All @@ -43,7 +61,7 @@ const Exam = ({ isTimeLimited, children }) => {
pingAttempt={pingAttempt}
/>
)}
{apiErrorMsg && <ExamAPIError details={apiErrorMsg} />}
{apiErrorMsg && <ExamAPIError />}
{isTimeLimited
? <Instructions>{sequenceContent}</Instructions>
: sequenceContent}
Expand Down
Loading

0 comments on commit f250975

Please sign in to comment.