From e2666b387eee379a1a73ab21bf0c6af4f26c65c3 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Tue, 2 Jul 2024 15:12:24 +0800 Subject: [PATCH 1/4] fix(submissions): fix polling after navigate away from page --- .../assessment/submission/actions/index.js | 51 ++++++++------- .../SubmissionEditForm.jsx | 26 -------- .../SubmissionEditStepForm.jsx | 26 -------- .../pages/SubmissionEditIndex/index.jsx | 65 ++++++++++++++++++- 4 files changed, 91 insertions(+), 77 deletions(-) diff --git a/client/app/bundles/course/assessment/submission/actions/index.js b/client/app/bundles/course/assessment/submission/actions/index.js index 24f5db20428..c19aa662464 100644 --- a/client/app/bundles/course/assessment/submission/actions/index.js +++ b/client/app/bundles/course/assessment/submission/actions/index.js @@ -1,4 +1,5 @@ import CourseAPI from 'api/course'; +import GlobalAPI from 'api'; import { setNotification } from 'lib/actions'; import pollJob from 'lib/helpers/jobHelpers'; @@ -32,6 +33,10 @@ export function getEvaluationResult(submissionId, answerId, questionId) { }; } +export function getJobStatus(jobUrl) { + return GlobalAPI.jobs.get(jobUrl); +} + export function fetchSubmission(id, onGetMonitoringSessionId) { return (dispatch) => { dispatch({ type: actionTypes.FETCH_SUBMISSION_REQUEST }); @@ -48,29 +53,29 @@ 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); - }); + // 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({ diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx index 58ae1dae2c9..6b6f75320d2 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx @@ -161,32 +161,6 @@ const SubmissionEditForm = (props) => { } }, [submissionTimeLimitAt]); - const POLL_INTERVAL_MILLISECONDS = 2000; - const pollerRef = useRef(null); - const pollAllFeedback = () => { - questionIds.forEach((id) => { - const question = questions[id]; - const feedbackRequestToken = - liveFeedback?.feedbackByQuestion?.[question.id]?.pendingFeedbackToken; - if (feedbackRequestToken) { - onFetchLiveFeedback(question.answerId, question.id); - } - }); - }; - - useEffect(() => { - // check for feedback from Codaveri on page load for each question - pollerRef.current = setInterval( - pollAllFeedback, - POLL_INTERVAL_MILLISECONDS, - ); - - // clean up poller on unmount - return () => { - clearInterval(pollerRef.current); - }; - }); - const renderAutogradeSubmissionButton = () => { if (graderView && submitted) { return ( diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx index a2a845caf73..2ca43cb706a 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx @@ -157,32 +157,6 @@ const SubmissionEditStepForm = (props) => { } }, [submissionTimeLimitAt]); - const POLL_INTERVAL_MILLISECONDS = 2000; - const pollerRef = useRef(null); - const pollAllFeedback = () => { - questionIds.forEach((id) => { - const question = questions[id]; - const feedbackRequestToken = - liveFeedback?.feedbackByQuestion?.[question.id]?.pendingFeedbackToken; - if (feedbackRequestToken) { - onFetchLiveFeedback(question.answerId, question.id); - } - }); - }; - - useEffect(() => { - // check for feedback from Codaveri on page load for each question - pollerRef.current = setInterval( - pollAllFeedback, - POLL_INTERVAL_MILLISECONDS, - ); - - // clean up poller on unmount - return () => { - clearInterval(pollerRef.current); - }; - }); - const handleNext = () => { setMaxStep(Math.max(maxStep, stepIndex + 1)); setStepIndex(stepIndex + 1); diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx index a4376f89009..7ae8dcc884c 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx @@ -1,4 +1,4 @@ -import { Component } from 'react'; +import { Component, createRef } from 'react'; import { FormattedMessage } from 'react-intl'; import { connect } from 'react-redux'; import InsertDriveFile from '@mui/icons-material/InsertDriveFile'; @@ -27,6 +27,8 @@ import { exitStudentView, fetchSubmission, finalise, + getEvaluationResult, + getJobStatus, mark, publish, purgeSubmissionStore, @@ -66,6 +68,9 @@ import SubmissionEditStepForm from './SubmissionEditStepForm'; import SubmissionEmptyForm from './SubmissionEmptyForm'; import TimeLimitBanner from './TimeLimitBanner'; +const EVALUATE_POLL_INTERVAL_MILLISECONDS = 500; +const FEEDBACK_POLL_INTERVAL_MILLISECONDS = 2000; + class VisibleSubmissionEditIndex extends Component { constructor(props) { super(props); @@ -76,16 +81,72 @@ class VisibleSubmissionEditIndex extends Component { ? null : parseInt(stepString, 10) - 1; - this.state = { step }; + this.state = { + feedbackPollerTimer: createRef(null), + evaluatePollerTimer: createRef(null), + }; + } + + handleLiveFeedbackPolling = () => { + const { questions, liveFeedback } = this.props; + const { assessment, submission } = this.props; + + Object.values(questions).forEach((question) => { + const feedbackRequestToken = + liveFeedback?.feedbackByQuestion?.[question.id]?.pendingFeedbackToken; + if (feedbackRequestToken) { + this.onFetchLiveFeedback(question.answerId, question.id); + } + }); + } + + handleEvaluationPolling = () => { + const { answers, dispatch, submission } = this.props; + Object.values(answers.initial) + .filter((a) => a.autograding && a.autograding.path) + .forEach((answer) => { + getJobStatus(answer.autograding.path) + .then((response) => { + switch (response.data.status) { + case 'submitted': + break; + case 'completed': + dispatch( + getEvaluationResult( + submission.id, + answer.id, + answer.questionId, + ), + ); + break; + case 'errored': + dispatch({ + type: actionTypes.AUTOGRADE_FAILURE, + questionId: answer.questionId, + }); + break; + default: + throw new Error('Unknown job status'); + } + }); + }); } componentDidMount() { const { dispatch, match, setSessionId } = this.props; dispatch(fetchSubmission(match.params.submissionId, setSessionId)); + + this.state.feedbackPollerTimer.current = + setInterval(this.handleLiveFeedbackPolling, FEEDBACK_POLL_INTERVAL_MILLISECONDS); + this.state.evaluatePollerTimer.current = + setInterval(this.handleEvaluationPolling, EVALUATE_POLL_INTERVAL_MILLISECONDS); } componentWillUnmount() { const { dispatch } = this.props; + + clearInterval(this.state.feedbackPollerTimer.current); + clearInterval(this.state.evaluatePollerTimer.current); dispatch(purgeSubmissionStore()); } From c405c8d7b37790d5eb6f34f2b99f40d554a22ab4 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Tue, 2 Jul 2024 16:27:47 +0800 Subject: [PATCH 2/4] feat(programming): tweaked frontend and added tooltips --- .../components/answers/Programming/index.jsx | 101 ++++++++++-------- client/locales/en.json | 7 +- client/locales/zh.json | 3 + 3 files changed, 64 insertions(+), 47 deletions(-) diff --git a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx index 2c3319c626c..9c4a25a95bf 100644 --- a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx +++ b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx @@ -8,6 +8,7 @@ import { CardContent, Drawer, IconButton, + Tooltip, Typography, } from '@mui/material'; import { green, grey, orange, red, yellow } from '@mui/material/colors'; @@ -27,14 +28,16 @@ import { questionShape } from '../../../propTypes'; import { parseLanguages } from '../../../utils'; import ProgrammingFile from './ProgrammingFile'; +import { defineMessages } from 'react-intl'; const styles = { card: { marginBottom: 1, borderStyle: 'solid', - borderWidth: 0.2, + borderWidth: 1.0, borderColor: grey[400], borderRadius: 2, + boxShadow: 'none', minWidth: '300px', maxWidth: '300px', }, @@ -62,6 +65,21 @@ const styles = { }, }; +const translations = defineMessages({ + lineHeader: { + id: 'course.assessment.submission.answers.Programming.liveFeedbackLineHeader', + defaultMessage: 'Line {linenum}', + }, + itemResolved: { + id: 'course.assessment.submission.answers.Programming.liveFeedbackItemResolved', + defaultMessage: 'Feedback liked.', + }, + itemDismissed: { + id: 'course.assessment.submission.answers.Programming.liveFeedbackItemDismissed', + defaultMessage: 'Feedback disliked.', + }, +}); + const ProgrammingFiles = ({ readOnly, questionId, @@ -95,32 +113,6 @@ const ProgrammingFiles = ({ } }; - // const editorKeyboardHandler = { - // handleKeyboard: (data, hash, keyString) => { - // const selectedRow = editorRef.current?.editor?.selection?.cursor?.row; - // const lastRow = - // (editorRef.current?.editor?.session?.getLength() ?? 1) - 1; - // if (selectedRow || selectedRow === 0) { - // if (keyString === 'up') { - // setSelectedLine(Math.max(selectedRow - 1, 0) + 1); - // } else if (keyString === 'down') { - // setSelectedLine(Math.min(selectedRow + 1, lastRow) + 1); - // } - // } - // }, - // }; - - // useEffect(() => { - // editorRef.current?.editor?.keyBinding?.addKeyboardHandler( - // editorKeyboardHandler, - // ); - // return () => { - // editorRef.current?.editor?.keyBinding?.removeKeyboardHandler( - // editorKeyboardHandler, - // ); - // }; - // }); - const renderFeedbackCard = (feedbackItem) => { let cardStyle = styles.card; if (feedbackItem.state === 'resolved') { @@ -131,6 +123,22 @@ const ProgrammingFiles = ({ cardStyle = { ...styles.card, ...styles.cardSelected }; } + const feedbackTooltipProps = { + placement: 'top', + slotProps: { + popper: { + modifiers: [ + { + name: 'offset', + options: { + offset: [0, -12], + }, + }, + ], + }, + } + }; + const focusEditorOnFeedbackLine = () => { editorRef.current?.editor?.gotoLine(feedbackItem.linenum, 0); editorRef.current?.editor?.selection?.setAnchor( @@ -146,37 +154,29 @@ const ProgrammingFiles = ({ return ( - - {feedbackItem.feedback} - - L{feedbackItem.linenum} + {t(translations.lineHeader, { linenum: feedbackItem.linenum })} {feedbackItem.state === 'resolved' && ( - {t({ - id: 'course.assessment.submission.answers.Programming.liveFeedbackItemResolved', - defaultMessage: 'Item resolved.', - })} + {t(translations.itemResolved)} )} {feedbackItem.state === 'dismissed' && ( - {t({ - id: 'course.assessment.submission.answers.Programming.liveFeedbackItemDismissed', - defaultMessage: 'Item dismissed.', - })} + {t(translations.itemDismissed)} )} + { - // TODO: expose BE route to Codaveri feedback rating endpoint and call here dispatch({ type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_RESOLVED, payload: { @@ -190,8 +190,11 @@ const ProgrammingFiles = ({ > + + { dispatch({ type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_DISMISSED, @@ -206,6 +209,8 @@ const ProgrammingFiles = ({ > + + { @@ -217,12 +222,20 @@ const ProgrammingFiles = ({ lineId: feedbackItem.id, }, }); + // TODO: expose BE route to Codaveri feedback rating endpoint and call here }} size="small" > + + + {feedbackItem.feedback} + ); }; @@ -249,9 +262,7 @@ const ProgrammingFiles = ({ annotations = feedbackFiles['main.py'] ?? []; } const keyString = `editor-container-${index}`; - const shouldOpenDrawer = annotations?.some( - (feedbackItem) => feedbackItem.state === 'pending', - ); + const shouldOpenDrawer = annotations.length > 0; return (
@@ -275,7 +286,7 @@ const ProgrammingFiles = ({ style: { alignContent: 'start', position: 'absolute' }, }} open={shouldOpenDrawer} - PaperProps={{ style: { position: 'absolute' } }} + PaperProps={{ style: { position: 'absolute', width: '315px', alignContent: 'start', border: 0 } }} variant="persistent" >
{annotations.map(renderFeedbackCard)}
diff --git a/client/locales/en.json b/client/locales/en.json index fc63af7882f..a022ade90db 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -2796,10 +2796,13 @@ "defaultMessage": "View Topic" }, "course.assessment.submission.answers.Programming.liveFeedbackItemDismissed": { - "defaultMessage": "Item dismissed." + "defaultMessage": "Feedback disliked." }, "course.assessment.submission.answers.Programming.liveFeedbackItemResolved": { - "defaultMessage": "Item resolved." + "defaultMessage": "Feedback liked." + }, + "course.assessment.submission.answers.Programming.liveFeedbackLineHeader": { + "defaultMessage": "Line {linenum}" }, "course.assessment.submission.answers.Programming.ProgrammingFile.downloadFile": { "defaultMessage": "Download File" diff --git a/client/locales/zh.json b/client/locales/zh.json index 444517bf3fa..6ca914ed684 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -2774,6 +2774,9 @@ "course.assessment.submission.answers.Programming.liveFeedbackItemResolved": { "defaultMessage": "项目已解决。" }, + "course.assessment.submission.answers.Programming.liveFeedbackLineHeader": { + "defaultMessage": "线 {linenum}" + }, "course.assessment.submission.answers.Programming.ProgrammingFile.downloadFile": { "defaultMessage": "下载文件" }, From 7b52c6c59a978d604e7ba2286b971c4fdf8ec779 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Tue, 2 Jul 2024 22:31:42 +0800 Subject: [PATCH 3/4] fix(submissions): fix eval polling after submit answer --- .../submission/actions/answers/index.js | 49 +++++++-------- .../assessment/submission/actions/index.js | 27 +------- .../course/assessment/submission/constants.ts | 1 + .../pages/SubmissionEditIndex/index.jsx | 63 +++++++++---------- .../submission/reducers/questionsFlags.js | 13 ++++ 5 files changed, 70 insertions(+), 83 deletions(-) diff --git a/client/app/bundles/course/assessment/submission/actions/answers/index.js b/client/app/bundles/course/assessment/submission/actions/answers/index.js index 44331bb351f..5882f7d4893 100644 --- a/client/app/bundles/course/assessment/submission/actions/answers/index.js +++ b/client/app/bundles/course/assessment/submission/actions/answers/index.js @@ -33,28 +33,27 @@ 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) { +// 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(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)); @@ -73,16 +72,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}`, { diff --git a/client/app/bundles/course/assessment/submission/actions/index.js b/client/app/bundles/course/assessment/submission/actions/index.js index c19aa662464..784f737a2ba 100644 --- a/client/app/bundles/course/assessment/submission/actions/index.js +++ b/client/app/bundles/course/assessment/submission/actions/index.js @@ -23,12 +23,12 @@ 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 }); }); }; } @@ -53,29 +53,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({ diff --git a/client/app/bundles/course/assessment/submission/constants.ts b/client/app/bundles/course/assessment/submission/constants.ts index 699e105b80d..739f017a76f 100644 --- a/client/app/bundles/course/assessment/submission/constants.ts +++ b/client/app/bundles/course/assessment/submission/constants.ts @@ -154,6 +154,7 @@ const actionTypes = mirrorCreator([ 'UNSUBMIT_SUCCESS', 'UNSUBMIT_FAILURE', 'AUTOGRADE_REQUEST', + 'AUTOGRADE_SUBMITTED', 'AUTOGRADE_SUCCESS', 'AUTOGRADE_FAILURE', 'AUTOGRADE_SAVING_SUCCESS', diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx index 7ae8dcc884c..3bd161c0ff6 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx @@ -89,7 +89,6 @@ class VisibleSubmissionEditIndex extends Component { handleLiveFeedbackPolling = () => { const { questions, liveFeedback } = this.props; - const { assessment, submission } = this.props; Object.values(questions).forEach((question) => { const feedbackRequestToken = @@ -101,34 +100,37 @@ class VisibleSubmissionEditIndex extends Component { } handleEvaluationPolling = () => { - const { answers, dispatch, submission } = this.props; + const { answers, questionsFlags, dispatch, submission } = this.props; Object.values(answers.initial) - .filter((a) => a.autograding && a.autograding.path) .forEach((answer) => { - getJobStatus(answer.autograding.path) - .then((response) => { - switch (response.data.status) { - case 'submitted': - break; - case 'completed': - dispatch( - getEvaluationResult( - submission.id, - answer.id, - answer.questionId, - ), - ); - break; - case 'errored': - dispatch({ - type: actionTypes.AUTOGRADE_FAILURE, - questionId: answer.questionId, - }); - break; - default: - throw new Error('Unknown job status'); - } - }); + if (questionsFlags[answer.questionId]?.isAutograding && + questionsFlags[answer.questionId]?.jobUrl) { + getJobStatus(questionsFlags[answer.questionId].jobUrl) + .then((response) => { + switch (response.data.status) { + case 'submitted': + break; + case 'completed': + dispatch( + getEvaluationResult( + submission.id, + answer.id, + answer.questionId, + ), + ); + break; + case 'errored': + dispatch({ + type: actionTypes.AUTOGRADE_FAILURE, + answerId: answer.id, + questionId: answer.questionId, + }); + break; + default: + throw new Error('Unknown job status'); + } + }); + } }); } @@ -249,11 +251,8 @@ class VisibleSubmissionEditIndex extends Component { }; onSubmitAnswer = (answerId, answer, resetField) => { - const { - dispatch, - match: { params }, - } = this.props; - dispatch(submitAnswer(params.submissionId, answerId, answer, resetField)); + const { dispatch } = this.props; + dispatch(submitAnswer(answer.questionId, answerId, answer, resetField)); }; onReevaluateAnswer = (answerId, questionId) => { diff --git a/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js b/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js index 07afc4f3064..512e755902c 100644 --- a/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js +++ b/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js @@ -16,6 +16,7 @@ export default function (state = {}, action) { isAutograding: Boolean(answer?.autograding) && answer?.autograding?.status === 'submitted', + jobUrl: answer?.autograding?.jobUrl, jobError: Boolean(answer?.autograding) && answer?.autograding?.status === 'errored', @@ -34,6 +35,17 @@ export default function (state = {}, action) { }, }; } + case actions.AUTOGRADE_SUBMITTED: { + const { questionId, jobUrl } = action.payload; + return { + ...state, + [questionId]: { + ...state[questionId], + isAutograding: true, + jobUrl, + }, + }; + } case actions.REEVALUATE_SUCCESS: case actions.AUTOGRADE_SUCCESS: { const { questionId } = action.payload; @@ -43,6 +55,7 @@ export default function (state = {}, action) { ...state[questionId], isAutograding: false, jobError: false, + jobErrorMessage: null, }, }; } From 9e84e66ee61b266691c401d03ad6574ea048650a Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Wed, 3 Jul 2024 16:47:37 +0800 Subject: [PATCH 4/4] style(programming): further FE tweaks, minor refactor feedback items --- .../submission/actions/answers/index.js | 1 - .../assessment/submission/actions/index.js | 3 +- .../components/answers/Programming/index.jsx | 224 ++++++++++-------- .../SubmissionEditForm.jsx | 4 +- .../SubmissionEditStepForm.jsx | 4 +- .../pages/SubmissionEditIndex/index.jsx | 123 +++++----- client/locales/en.json | 21 +- client/locales/zh.json | 21 +- 8 files changed, 221 insertions(+), 180 deletions(-) diff --git a/client/app/bundles/course/assessment/submission/actions/answers/index.js b/client/app/bundles/course/assessment/submission/actions/answers/index.js index 5882f7d4893..630a413bb1d 100644 --- a/client/app/bundles/course/assessment/submission/actions/answers/index.js +++ b/client/app/bundles/course/assessment/submission/actions/answers/index.js @@ -264,7 +264,6 @@ export function generateLiveFeedback(submissionId, answerId, questionId) { }); } -// TODO should each answer/question store its own feedback array? export function fetchLiveFeedback( answerId, questionId, diff --git a/client/app/bundles/course/assessment/submission/actions/index.js b/client/app/bundles/course/assessment/submission/actions/index.js index 784f737a2ba..99bb076bf19 100644 --- a/client/app/bundles/course/assessment/submission/actions/index.js +++ b/client/app/bundles/course/assessment/submission/actions/index.js @@ -1,5 +1,5 @@ -import CourseAPI from 'api/course'; import GlobalAPI from 'api'; +import CourseAPI from 'api/course'; import { setNotification } from 'lib/actions'; import pollJob from 'lib/helpers/jobHelpers'; @@ -13,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) => { diff --git a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx index 9c4a25a95bf..b3508607794 100644 --- a/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx +++ b/client/app/bundles/course/assessment/submission/components/answers/Programming/index.jsx @@ -1,5 +1,6 @@ import { useRef, useState } from 'react'; import { useFieldArray, useFormContext, useWatch } from 'react-hook-form'; +import { defineMessages } from 'react-intl'; import { Close, ThumbDown, ThumbUp } from '@mui/icons-material'; import { Box, @@ -28,7 +29,6 @@ import { questionShape } from '../../../propTypes'; import { parseLanguages } from '../../../utils'; import ProgrammingFile from './ProgrammingFile'; -import { defineMessages } from 'react-intl'; const styles = { card: { @@ -41,13 +41,18 @@ const styles = { minWidth: '300px', maxWidth: '300px', }, - header: { + cardActions: { + px: 0, + paddingTop: 0.5, + paddingBottom: 0, display: 'flex', - backgroundColor: orange[100], - borderRadius: 2, - padding: 1, - borderBottomLeftRadius: 0, - borderBottomRightRadius: 0, + }, + cardContent: { + px: 1, + paddingTop: 0, + '&:last-child': { + paddingBottom: 1, + }, }, cardSelected: { backgroundColor: yellow[100], @@ -56,27 +61,50 @@ const styles = { backgroundColor: orange.A100, }, cardResolved: { - opacity: 0.6, - backgroundColor: green['100'], + borderColor: '#cecece', + backgroundColor: green[100], + color: grey[600], }, cardDismissed: { - opacity: 0.6, - backgroundColor: red['100'], + borderColor: '#cecece', + backgroundColor: red[100], + color: grey[600], + }, + cardActionButton: { + opacity: 1.0, + marginX: -0.5, + padding: 0.4, + transform: 'scale(0.86)', + transformOrigin: 'right', + }, + cardActionButtonHighlightOnResolve: { + '&:disabled': { + color: green.A700, + }, + }, + cardActionButtonHighlightOnDismiss: { + '&:disabled': { + color: red.A700, + }, }, }; const translations = defineMessages({ lineHeader: { - id: 'course.assessment.submission.answers.Programming.liveFeedbackLineHeader', + id: 'course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLineHeading', defaultMessage: 'Line {linenum}', }, - itemResolved: { - id: 'course.assessment.submission.answers.Programming.liveFeedbackItemResolved', - defaultMessage: 'Feedback liked.', + likeItem: { + id: 'course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLike', + defaultMessage: 'Like', + }, + dislikeItem: { + id: 'course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDislike', + defaultMessage: 'Dislike', }, - itemDismissed: { - id: 'course.assessment.submission.answers.Programming.liveFeedbackItemDismissed', - defaultMessage: 'Feedback disliked.', + deleteItem: { + id: 'course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDelete', + defaultMessage: 'Dismiss', }, }); @@ -136,7 +164,7 @@ const ProgrammingFiles = ({ }, ], }, - } + }, }; const focusEditorOnFeedbackLine = () => { @@ -152,88 +180,92 @@ const ProgrammingFiles = ({ editorRef.current?.editor?.focus(); }; + const renderLikeButton = () => ( + + { + dispatch({ + type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_RESOLVED, + payload: { + questionId, + path: 'main.py', + lineId: feedbackItem.id, + }, + }); + }} + sx={{ + ...styles.cardActionButton, + ...styles.cardActionButtonHighlightOnResolve, + }} + > + + + + ); + + const renderDislikeButton = () => ( + + { + dispatch({ + type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_DISMISSED, + payload: { + questionId, + path: 'main.py', + lineId: feedbackItem.id, + }, + }); + }} + sx={{ + ...styles.cardActionButton, + ...styles.cardActionButtonHighlightOnDismiss, + }} + > + + + + ); + + const renderDeleteButton = () => ( + + { + dispatch({ + type: actionTypes.LIVE_FEEDBACK_ITEM_DELETE, + payload: { + questionId, + path: 'main.py', + lineId: feedbackItem.id, + }, + }); + }} + sx={{ ...styles.cardActionButton, marginRight: 1 }} + > + + + + ); + return ( {t(translations.lineHeader, { linenum: feedbackItem.linenum })} - {feedbackItem.state === 'resolved' && ( - - {t(translations.itemResolved)} - - )} - {feedbackItem.state === 'dismissed' && ( - - {t(translations.itemDismissed)} - - )} - - { - dispatch({ - type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_RESOLVED, - payload: { - questionId, - path: 'main.py', - lineId: feedbackItem.id, - }, - }); - }} - size="small" - > - - - - - { - dispatch({ - type: actionTypes.LIVE_FEEDBACK_ITEM_MARK_DISMISSED, - payload: { - questionId, - path: 'main.py', - lineId: feedbackItem.id, - }, - }); - }} - size="small" - > - - - - - { - dispatch({ - type: actionTypes.LIVE_FEEDBACK_ITEM_DELETE, - payload: { - questionId, - path: 'main.py', - lineId: feedbackItem.id, - }, - }); - // TODO: expose BE route to Codaveri feedback rating endpoint and call here - }} - size="small" - > - - - + {renderLikeButton()} + {renderDislikeButton()} + {renderDeleteButton()} - + {feedbackItem.feedback} @@ -286,7 +318,14 @@ const ProgrammingFiles = ({ style: { alignContent: 'start', position: 'absolute' }, }} open={shouldOpenDrawer} - PaperProps={{ style: { position: 'absolute', width: '315px', alignContent: 'start', border: 0 } }} + PaperProps={{ + style: { + position: 'absolute', + width: '315px', + alignContent: 'start', + border: 0, + }, + }} variant="persistent" >
{annotations.map(renderFeedbackCard)}
@@ -333,7 +372,6 @@ const Programming = (props) => { saveAnswerAndUpdateClientVersion={saveAnswerAndUpdateClientVersion} /> )} - {/* */}
); diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx index 6b6f75320d2..050c6140895 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditForm.jsx @@ -1,4 +1,4 @@ -import { lazy, Suspense, useEffect, useRef, useState } from 'react'; +import { lazy, Suspense, useEffect, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { injectIntl } from 'react-intl'; import { connect } from 'react-redux'; @@ -86,7 +86,6 @@ const SubmissionEditForm = (props) => { onSaveDraft, onSubmit, onSubmitAnswer, - onFetchLiveFeedback, onGenerateLiveFeedback, onGenerateFeedback, onReevaluateAnswer, @@ -832,7 +831,6 @@ SubmissionEditForm.propTypes = { onSubmit: PropTypes.func, onSubmitAnswer: PropTypes.func, onReevaluateAnswer: PropTypes.func, - onFetchLiveFeedback: PropTypes.func, onGenerateLiveFeedback: PropTypes.func, onGenerateFeedback: PropTypes.func, handleUnsubmit: PropTypes.func, diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx index 2ca43cb706a..ae418e9ce06 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/SubmissionEditStepForm.jsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import Hotkeys from 'react-hot-keys'; import { FormattedMessage, injectIntl } from 'react-intl'; @@ -98,7 +98,6 @@ const SubmissionEditStepForm = (props) => { onSaveDraft, onSubmit, onSubmitAnswer, - onFetchLiveFeedback, onGenerateLiveFeedback, onGenerateFeedback, onReevaluateAnswer, @@ -759,7 +758,6 @@ SubmissionEditStepForm.propTypes = { onSubmit: PropTypes.func, onSubmitAnswer: PropTypes.func, onReevaluateAnswer: PropTypes.func, - onFetchLiveFeedback: PropTypes.func, onGenerateLiveFeedback: PropTypes.func, onGenerateFeedback: PropTypes.func, handleUnsubmit: PropTypes.func, diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx index 3bd161c0ff6..04062a814b7 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/index.jsx @@ -48,7 +48,7 @@ import { submitAnswer, } from '../../actions/answers'; import ProgressPanel from '../../components/ProgressPanel'; -import { workflowStates } from '../../constants'; +import actionTypes, { workflowStates } from '../../constants'; import { answerShape, assessmentShape, @@ -81,74 +81,30 @@ class VisibleSubmissionEditIndex extends Component { ? null : parseInt(stepString, 10) - 1; - this.state = { - feedbackPollerTimer: createRef(null), - evaluatePollerTimer: createRef(null), - }; - } - - handleLiveFeedbackPolling = () => { - const { questions, liveFeedback } = this.props; - - Object.values(questions).forEach((question) => { - const feedbackRequestToken = - liveFeedback?.feedbackByQuestion?.[question.id]?.pendingFeedbackToken; - if (feedbackRequestToken) { - this.onFetchLiveFeedback(question.answerId, question.id); - } - }); - } - - handleEvaluationPolling = () => { - const { answers, questionsFlags, dispatch, submission } = this.props; - Object.values(answers.initial) - .forEach((answer) => { - if (questionsFlags[answer.questionId]?.isAutograding && - questionsFlags[answer.questionId]?.jobUrl) { - getJobStatus(questionsFlags[answer.questionId].jobUrl) - .then((response) => { - switch (response.data.status) { - case 'submitted': - break; - case 'completed': - dispatch( - getEvaluationResult( - submission.id, - answer.id, - answer.questionId, - ), - ); - break; - case 'errored': - dispatch({ - type: actionTypes.AUTOGRADE_FAILURE, - answerId: answer.id, - questionId: answer.questionId, - }); - break; - default: - throw new Error('Unknown job status'); - } - }); - } - }); + this.state = { step }; + this.evaluatePollerTimer = createRef(null); + this.feedbackPollerTimer = createRef(null); } componentDidMount() { const { dispatch, match, setSessionId } = this.props; dispatch(fetchSubmission(match.params.submissionId, setSessionId)); - this.state.feedbackPollerTimer.current = - setInterval(this.handleLiveFeedbackPolling, FEEDBACK_POLL_INTERVAL_MILLISECONDS); - this.state.evaluatePollerTimer.current = - setInterval(this.handleEvaluationPolling, EVALUATE_POLL_INTERVAL_MILLISECONDS); + this.feedbackPollerTimer.current = setInterval( + this.handleLiveFeedbackPolling, + FEEDBACK_POLL_INTERVAL_MILLISECONDS, + ); + this.evaluatePollerTimer.current = setInterval( + this.handleEvaluationPolling, + EVALUATE_POLL_INTERVAL_MILLISECONDS, + ); } componentWillUnmount() { const { dispatch } = this.props; - clearInterval(this.state.feedbackPollerTimer.current); - clearInterval(this.state.evaluatePollerTimer.current); + clearInterval(this.feedbackPollerTimer.current); + clearInterval(this.evaluatePollerTimer.current); dispatch(purgeSubmissionStore()); } @@ -160,6 +116,55 @@ class VisibleSubmissionEditIndex extends Component { dispatch(autogradeSubmission(params.submissionId)); }; + handleEvaluationPolling = () => { + const { answers, questionsFlags, dispatch, submission } = this.props; + Object.values(answers.initial).forEach((answer) => { + if ( + questionsFlags[answer.questionId]?.isAutograding && + questionsFlags[answer.questionId]?.jobUrl + ) { + getJobStatus(questionsFlags[answer.questionId].jobUrl).then( + (response) => { + switch (response.data.status) { + case 'submitted': + break; + case 'completed': + dispatch( + getEvaluationResult( + submission.id, + answer.id, + answer.questionId, + ), + ); + break; + case 'errored': + dispatch({ + type: actionTypes.AUTOGRADE_FAILURE, + answerId: answer.id, + questionId: answer.questionId, + }); + break; + default: + throw new Error('Unknown job status'); + } + }, + ); + } + }); + }; + + handleLiveFeedbackPolling = () => { + const { questions, liveFeedback } = this.props; + + Object.values(questions).forEach((question) => { + const feedbackRequestToken = + liveFeedback?.feedbackByQuestion?.[question.id]?.pendingFeedbackToken; + if (feedbackRequestToken) { + this.onFetchLiveFeedback(question.answerId, question.id); + } + }); + }; + handleMark = () => { const { dispatch, @@ -416,7 +421,6 @@ class VisibleSubmissionEditIndex extends Component { isCodaveriEnabled={isCodaveriEnabled} isSaving={isSaving} maxStep={maxStep === undefined ? questionIds.length - 1 : maxStep} - onFetchLiveFeedback={this.onFetchLiveFeedback} onGenerateFeedback={this.onGenerateFeedback} onGenerateLiveFeedback={this.onGenerateLiveFeedback} onReevaluateAnswer={this.onReevaluateAnswer} @@ -461,7 +465,6 @@ class VisibleSubmissionEditIndex extends Component { isCodaveriEnabled={isCodaveriEnabled} isSaving={isSaving} maxStep={maxStep === undefined ? questionIds.length - 1 : maxStep} - onFetchLiveFeedback={this.onFetchLiveFeedback} onGenerateFeedback={this.onGenerateFeedback} onGenerateLiveFeedback={this.onGenerateLiveFeedback} onReevaluateAnswer={this.onReevaluateAnswer} diff --git a/client/locales/en.json b/client/locales/en.json index a022ade90db..49823a54dc3 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -2795,15 +2795,6 @@ "course.assessment.submission.answers.ForumPostResponse.TopicCard.viewTopicInNewTab": { "defaultMessage": "View Topic" }, - "course.assessment.submission.answers.Programming.liveFeedbackItemDismissed": { - "defaultMessage": "Feedback disliked." - }, - "course.assessment.submission.answers.Programming.liveFeedbackItemResolved": { - "defaultMessage": "Feedback liked." - }, - "course.assessment.submission.answers.Programming.liveFeedbackLineHeader": { - "defaultMessage": "Line {linenum}" - }, "course.assessment.submission.answers.Programming.ProgrammingFile.downloadFile": { "defaultMessage": "Download File" }, @@ -2876,6 +2867,18 @@ "course.assessment.submission.comments": { "defaultMessage": "Comments" }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDelete": { + "defaultMessage": "Dismiss" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDislike": { + "defaultMessage": "Dislike" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLike": { + "defaultMessage": "Like" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLineHeading": { + "defaultMessage": "Line {linenum}" + }, "course.assessment.submission.continue": { "defaultMessage": "Continue" }, diff --git a/client/locales/zh.json b/client/locales/zh.json index 6ca914ed684..ab49a983654 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -2768,15 +2768,6 @@ "course.assessment.submission.answers.ForumPostResponse.TopicCard.viewTopicInNewTab": { "defaultMessage": "查看主题" }, - "course.assessment.submission.answers.Programming.liveFeedbackItemDismissed": { - "defaultMessage": "项目已忽略。" - }, - "course.assessment.submission.answers.Programming.liveFeedbackItemResolved": { - "defaultMessage": "项目已解决。" - }, - "course.assessment.submission.answers.Programming.liveFeedbackLineHeader": { - "defaultMessage": "线 {linenum}" - }, "course.assessment.submission.answers.Programming.ProgrammingFile.downloadFile": { "defaultMessage": "下载文件" }, @@ -2849,6 +2840,18 @@ "course.assessment.submission.comments": { "defaultMessage": "注释" }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDelete": { + "defaultMessage": "忽略" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemDislike": { + "defaultMessage": "不喜欢" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLike": { + "defaultMessage": "喜欢" + }, + "course.assessment.submission.answers.Programming.ProgrammingFiles.liveFeedbackItemLineHeading": { + "defaultMessage": "线 {linenum}" + }, "course.assessment.submission.continue": { "defaultMessage": "继续" },