Skip to content

Commit

Permalink
fix(programming): fix evaluation polling after user navigate away
Browse files Browse the repository at this point in the history
- polling now handled by SubmissionEditIndex component, timers started on mount and cleared on unmount
- autograding job polling (from page load and "Run Code" button) moved from pollJob to component timers
  • Loading branch information
adi-herwana-nus authored and cysjonathan committed Oct 28, 2024
1 parent b1fcfdf commit fe47f4d
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { getClientVersionForAnswerId } from '../../selectors/answers';
import translations from '../../translations';
import { convertAnswerDataToInitialValue } from '../../utils/answers';
import { buildErrorMessage, formatAnswer } from '../utils';
import { fetchSubmission, getEvaluationResult } from '..';
import { fetchSubmission } from '..';

const JOB_POLL_DELAY_MS = 500;
export const STALE_ANSWER_ERR = 'stale_answer';
Expand All @@ -33,28 +33,10 @@ export const updateClientVersion = (answerId, clientVersion) => (dispatch) =>
payload: { answer: { id: answerId, clientVersion } },
});

const pollAutogradingJob =
(jobUrl, submissionId, questionId, answerId) => (dispatch) => {
pollJob(
jobUrl,
() => dispatch(getEvaluationResult(submissionId, answerId, questionId)),
(errorData) => {
dispatch({
type: actionTypes.AUTOGRADE_FAILURE,
questionId,
payload: errorData,
});
dispatch(setNotification(translations.requestFailure));
},
JOB_POLL_DELAY_MS,
);
};

export function submitAnswer(submissionId, answerId, rawAnswer, resetField) {
export function submitAnswer(questionId, answerId, rawAnswer, resetField) {
const currentTime = Date.now();
const answer = formatAnswer(rawAnswer, currentTime);
const payload = { answer };
const questionId = answer.questionId;

return (dispatch) => {
dispatch(updateClientVersion(answerId, currentTime));
Expand All @@ -73,16 +55,14 @@ export function submitAnswer(submissionId, answerId, rawAnswer, resetField) {
if (data.newSessionUrl) {
window.location = data.newSessionUrl;
} else if (data.jobUrl) {
pollAutogradingJob(
data.jobUrl,
submissionId,
questionId,
answerId,
)(dispatch);
dispatch({
type: actionTypes.AUTOGRADE_SUBMITTED,
payload: { questionId, jobUrl: data.jobUrl },
});
} else {
dispatch({
type: actionTypes.AUTOGRADE_SUCCESS,
payload: data,
payload: { ...data, answerId },
});
// When an answer is submitted, the value of that field needs to be updated.
resetField(`${answerId}`, {
Expand Down Expand Up @@ -359,20 +339,10 @@ export function reevaluateAnswer(submissionId, answerId, questionId) {
if (data.newSessionUrl) {
window.location = data.newSessionUrl;
} else if (data.jobUrl) {
pollJob(
data.jobUrl,
() =>
dispatch(getEvaluationResult(submissionId, answerId, questionId)),
(errorData) => {
dispatch({
type: actionTypes.REEVALUATE_FAILURE,
questionId,
payload: errorData,
});
dispatch(setNotification(translations.requestFailure));
},
JOB_POLL_DELAY_MS,
);
dispatch({
type: actionTypes.REEVALUATE_SUBMITTED,
payload: { questionId, jobUrl: data.jobUrl },
});
} else {
dispatch({
type: actionTypes.REEVALUATE_SUCCESS,
Expand Down
33 changes: 7 additions & 26 deletions client/app/bundles/course/assessment/submission/actions/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import GlobalAPI from 'api';
import CourseAPI from 'api/course';
import { setNotification } from 'lib/actions';
import pollJob from 'lib/helpers/jobHelpers';
Expand All @@ -12,7 +13,6 @@ import translations from '../translations';
import { buildErrorMessage, formatAnswers } from './utils';

const JOB_POLL_DELAY_MS = 500;
const JOB_STAGGER_DELAY_MS = 400;

export function getEvaluationResult(submissionId, answerId, questionId) {
return (dispatch) => {
Expand All @@ -22,16 +22,20 @@ export function getEvaluationResult(submissionId, answerId, questionId) {
.then((data) => {
dispatch({
type: actionTypes.AUTOGRADE_SUCCESS,
payload: data,
payload: { ...data, answerId },
});
})
.catch(() => {
dispatch(setNotification(translations.requestFailure));
dispatch({ type: actionTypes.AUTOGRADE_FAILURE, questionId });
dispatch({ type: actionTypes.AUTOGRADE_FAILURE, questionId, answerId });
});
};
}

export function getJobStatus(jobUrl) {
return GlobalAPI.jobs.get(jobUrl);
}

export function fetchSubmission(id, onGetMonitoringSessionId) {
return (dispatch) => {
dispatch({ type: actionTypes.FETCH_SUBMISSION_REQUEST });
Expand All @@ -48,29 +52,6 @@ export function fetchSubmission(id, onGetMonitoringSessionId) {
window.location = data.newSessionUrl;
return;
}
data.answers
.filter((a) => a.autograding && a.autograding.path)
.forEach((answer, index) => {
setTimeout(() => {
pollJob(
answer.autograding.path,
() =>
dispatch(
getEvaluationResult(
id,
answer.fields.id,
answer.questionId,
),
),
() =>
dispatch({
type: actionTypes.AUTOGRADE_FAILURE,
questionId: answer.questionId,
}),
JOB_POLL_DELAY_MS,
);
}, JOB_STAGGER_DELAY_MS * index);
});
if (data.monitoringSessionId !== undefined)
onGetMonitoringSessionId?.(data.monitoringSessionId);
dispatch({
Expand Down
5 changes: 4 additions & 1 deletion client/app/bundles/course/assessment/submission/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export const BUFFER_TIME_TO_FORCE_SUBMIT_MS = 5 * 1000;
// to still be considered a "newly created" submission
export const TIME_LAPSE_NEW_SUBMISSION_MS = 10 * 1000;

export const POLL_INTERVAL_MILLISECONDS = 2000;
export const EVALUATE_POLL_INTERVAL_MILLISECONDS = 500;
export const FEEDBACK_POLL_INTERVAL_MILLISECONDS = 2000;

export const workflowStates = {
Unstarted: 'unstarted' as const,
Expand Down Expand Up @@ -160,6 +161,7 @@ const actionTypes = mirrorCreator([
'UNSUBMIT_SUCCESS',
'UNSUBMIT_FAILURE',
'AUTOGRADE_REQUEST',
'AUTOGRADE_SUBMITTED',
'AUTOGRADE_SUCCESS',
'AUTOGRADE_FAILURE',
'AUTOGRADE_SAVING_SUCCESS',
Expand All @@ -168,6 +170,7 @@ const actionTypes = mirrorCreator([
'FEEDBACK_SUCCESS',
'FEEDBACK_FAILURE',
'REEVALUATE_REQUEST',
'REEVALUATE_SUBMITTED',
'REEVALUATE_SUCCESS',
'REEVALUATE_FAILURE',
'RESET_REQUEST',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@ import usePrompt from 'lib/hooks/router/usePrompt';
import { useAppDispatch, useAppSelector } from 'lib/hooks/store';
import useTranslation from 'lib/hooks/useTranslation';

import { finalise } from '../../actions';
import { finalise, getEvaluationResult, getJobStatus } from '../../actions';
import { fetchLiveFeedback } from '../../actions/answers';
import WarningDialog from '../../components/WarningDialog';
import {
import actionTypes, {
EVALUATE_POLL_INTERVAL_MILLISECONDS,
FEEDBACK_POLL_INTERVAL_MILLISECONDS,
formNames,
POLL_INTERVAL_MILLISECONDS,
workflowStates,
} from '../../constants';
import GradingPanel from '../../containers/GradingPanel';
import { getInitialAnswer } from '../../selectors/answers';
import { getAssessment } from '../../selectors/assessments';
import { getAttachments } from '../../selectors/attachments';
import { getLiveFeedbacks } from '../../selectors/liveFeedbacks';
import { getQuestionFlags } from '../../selectors/questionFlags';
import { getQuestions } from '../../selectors/questions';
import { getSubmission } from '../../selectors/submissions';
import translations from '../../translations';
Expand Down Expand Up @@ -51,6 +53,7 @@ const SubmissionForm: FC<Props> = (props) => {
const assessment = useAppSelector(getAssessment);
const submission = useAppSelector(getSubmission);
const questions = useAppSelector(getQuestions);
const questionFlags = useAppSelector(getQuestionFlags);
const attachments = useAppSelector(getAttachments);
const liveFeedbacks = useAppSelector(getLiveFeedbacks);
const initialValues = useAppSelector(getInitialAnswer);
Expand Down Expand Up @@ -147,7 +150,8 @@ const SubmissionForm: FC<Props> = (props) => {
}
});

const pollerRef = useRef<NodeJS.Timeout | null>(null);
const feedbackPollerRef = useRef<NodeJS.Timeout | null>(null);
const evaluatePollerRef = useRef<NodeJS.Timeout | null>(null);
const pollAllFeedback = (): void => {
questionIds.forEach((id) => {
const question = questions[id];
Expand All @@ -159,17 +163,59 @@ const SubmissionForm: FC<Props> = (props) => {
});
};

const handleEvaluationPolling = (): void => {
Object.values(questions).forEach((question) => {
if (
questionFlags[question.id]?.isAutograding &&
questionFlags[question.id]?.jobUrl
) {
getJobStatus(questionFlags[question.id].jobUrl).then((response) => {
switch (response.data.status) {
case 'submitted':
break;
case 'completed':
dispatch(
getEvaluationResult(
submissionId,
question.answerId,
question.id,
),
);
break;
case 'errored':
dispatch({
type: actionTypes.AUTOGRADE_FAILURE,
answerId: question.answerId,
questionId: question.id,
});
break;
default:
throw new Error('Unknown job status');
}
});
}
});
};

useEffect(() => {
// check for feedback from Codaveri on page load for each question
pollerRef.current = setInterval(
feedbackPollerRef.current = setInterval(
pollAllFeedback,
POLL_INTERVAL_MILLISECONDS,
FEEDBACK_POLL_INTERVAL_MILLISECONDS,
);

evaluatePollerRef.current = setInterval(
handleEvaluationPolling,
EVALUATE_POLL_INTERVAL_MILLISECONDS,
);

// clean up poller on unmount
return () => {
if (pollerRef.current) {
clearInterval(pollerRef.current);
if (feedbackPollerRef.current) {
clearInterval(feedbackPollerRef.current);
}
if (evaluatePollerRef.current) {
clearInterval(evaluatePollerRef.current);
}
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { getSubmissionFlags } from 'course/assessment/submission/selectors/submi
import { getSubmission } from 'course/assessment/submission/selectors/submissions';
import translations from 'course/assessment/submission/translations';
import LoadingIndicator from 'lib/components/core/LoadingIndicator';
import { getSubmissionId } from 'lib/helpers/url-helpers';
import { useAppDispatch, useAppSelector } from 'lib/hooks/store';
import useTranslation from 'lib/hooks/useTranslation';

Expand All @@ -30,7 +29,6 @@ const RunCodeButton: FC<Props> = (props) => {
const { resetField, getValues } = useFormContext();

const question = questions[questionId];
const submissionId = getSubmissionId();

const { answerId, attemptsLeft, attemptLimit } = question;
const { isAutograding, isResetting } = questionFlags[questionId] || {};
Expand All @@ -39,12 +37,7 @@ const RunCodeButton: FC<Props> = (props) => {

const onSubmitAnswer = (): void => {
dispatch(
submitAnswer(
submissionId,
answerId,
getValues(`${answerId}`),
resetField,
),
submitAnswer(question.id, answerId, getValues(`${answerId}`), resetField),
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { getQuestions } from 'course/assessment/submission/selectors/questions';
import { getSubmissionFlags } from 'course/assessment/submission/selectors/submissionFlags';
import translations from 'course/assessment/submission/translations';
import LoadingIndicator from 'lib/components/core/LoadingIndicator';
import { getSubmissionId } from 'lib/helpers/url-helpers';
import { useAppDispatch, useAppSelector } from 'lib/hooks/store';
import useTranslation from 'lib/hooks/useTranslation';

Expand All @@ -33,8 +32,6 @@ const SubmitButton: FC<Props> = (props) => {

const { resetField, getValues } = useFormContext();

const submissionId = getSubmissionId();

const question = questions[questionId];

const { answerId, autogradable, type } = question;
Expand All @@ -51,12 +48,7 @@ const SubmitButton: FC<Props> = (props) => {

const onSubmitAnswer = (): void => {
dispatch(
submitAnswer(
submissionId,
answerId,
getValues(`${answerId}`),
resetField,
),
submitAnswer(question.id, answerId, getValues(`${answerId}`), resetField),
);
};

Expand Down
Loading

0 comments on commit fe47f4d

Please sign in to comment.