Skip to content

Commit ab40f09

Browse files
LWS49LWS49
committed
feat(grading) - add prefill to 0 and adjust prefill policies for some question types
- refactor to differentiate question types that can be prefilled either to 0 or max, both or neither (for previous code, only boolean for if it can be prefilled) - TextResponse and Comprehension question types adjusted to allow prefilling to full - add tests for different policies
1 parent af5f94c commit ab40f09

3 files changed

Lines changed: 308 additions & 40 deletions

File tree

‎client/app/bundles/course/assessment/submission/reducers/grading/__test__/index.test.js‎

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
import { dispatch, store } from 'store';
2+
3+
import actions, { questionTypes } from '../../../constants';
4+
5+
const answerId = 3;
6+
7+
const buildPayload = ({ questionType, grade, correct, testCases } = {}) => ({
8+
submission: {
9+
pointsAwarded: null,
10+
basePoints: 1000,
11+
submittedAt: '2017-05-11T17:02:17.000+08:00',
12+
bonusEndAt: '2017-05-11T17:02:17.000+08:00',
13+
bonusPoints: 0,
14+
},
15+
assessment: {},
16+
annotations: [],
17+
posts: [],
18+
topics: [],
19+
questions: [{ id: 1, type: questionType, maximumGrade: 10 }],
20+
answers: [
21+
{
22+
id: answerId,
23+
fields: {
24+
id: answerId,
25+
questionId: 1,
26+
},
27+
questionId: 1,
28+
grading: {
29+
grade,
30+
id: answerId,
31+
},
32+
explanation: correct !== undefined ? { correct } : undefined,
33+
testCases,
34+
},
35+
],
36+
});
37+
38+
const dispatchFetchSuccess = (payload) =>
39+
dispatch({ type: actions.FETCH_SUBMISSION_SUCCESS, payload });
40+
41+
const getGrade = () =>
42+
store.getState().assessments.submission.grading.questions[1].grade;
43+
44+
describe('getPrefilledGrade via FETCH_SUBMISSION_SUCCESS', () => {
45+
describe('Question Types with ALWAYS_PREFILL_POLICY', () => {
46+
it('prefills maximum grade for an ungraded correct answer', () => {
47+
dispatchFetchSuccess(
48+
buildPayload({
49+
questionType: questionTypes.MultipleChoice,
50+
grade: null,
51+
correct: true,
52+
}),
53+
);
54+
55+
expect(getGrade()).toBe(10);
56+
});
57+
58+
it('prefills 0 for an ungraded incorrect answer', () => {
59+
dispatchFetchSuccess(
60+
buildPayload({
61+
questionType: questionTypes.MultipleChoice,
62+
grade: null,
63+
correct: false,
64+
}),
65+
);
66+
67+
expect(getGrade()).toBe(0);
68+
});
69+
70+
it('leaves grade as null when there is no explanation', () => {
71+
dispatchFetchSuccess(
72+
buildPayload({
73+
questionType: questionTypes.MultipleChoice,
74+
grade: null,
75+
correct: undefined,
76+
}),
77+
);
78+
79+
expect(getGrade()).toBeNull();
80+
});
81+
82+
it('preserves existing grades even if answer is correct', () => {
83+
dispatchFetchSuccess(
84+
buildPayload({
85+
questionType: questionTypes.MultipleChoice,
86+
grade: 5,
87+
correct: true,
88+
}),
89+
);
90+
91+
expect(getGrade()).toBe(5);
92+
});
93+
94+
it('preserves existing grade even if answer is incorrect', () => {
95+
dispatchFetchSuccess(
96+
buildPayload({
97+
questionType: questionTypes.MultipleChoice,
98+
grade: 8,
99+
correct: false,
100+
}),
101+
);
102+
103+
expect(getGrade()).toBe(8);
104+
});
105+
});
106+
107+
describe('Question Types with NEVER_PREFILL_POLICY', () => {
108+
it('does not prefill for an ungraded correct answer', () => {
109+
dispatchFetchSuccess(
110+
buildPayload({
111+
questionType: questionTypes.VoiceResponse,
112+
grade: null,
113+
correct: true,
114+
}),
115+
);
116+
117+
expect(getGrade()).toBeNull();
118+
});
119+
120+
it('does not prefill for an ungraded incorrect answer', () => {
121+
dispatchFetchSuccess(
122+
buildPayload({
123+
questionType: questionTypes.VoiceResponse,
124+
grade: null,
125+
correct: false,
126+
}),
127+
);
128+
129+
expect(getGrade()).toBeNull();
130+
});
131+
132+
it('preserves existing grades', () => {
133+
dispatchFetchSuccess(
134+
buildPayload({
135+
questionType: questionTypes.VoiceResponse,
136+
grade: 5,
137+
correct: true,
138+
}),
139+
);
140+
141+
expect(getGrade()).toBe(5);
142+
});
143+
});
144+
145+
describe('Question Types with ONLY_PREFILL_FULL_POLICY', () => {
146+
it('prefills maximum grade for an ungraded correct answer', () => {
147+
dispatchFetchSuccess(
148+
buildPayload({
149+
questionType: questionTypes.TextResponse,
150+
grade: null,
151+
correct: true,
152+
}),
153+
);
154+
155+
expect(getGrade()).toBe(10);
156+
});
157+
158+
it('does not prefill 0 for an ungraded incorrect answer', () => {
159+
dispatchFetchSuccess(
160+
buildPayload({
161+
questionType: questionTypes.TextResponse,
162+
grade: null,
163+
correct: false,
164+
}),
165+
);
166+
167+
expect(getGrade()).toBeNull();
168+
});
169+
170+
it('leaves grade as null when there is no explanation', () => {
171+
dispatchFetchSuccess(
172+
buildPayload({
173+
questionType: questionTypes.TextResponse,
174+
grade: null,
175+
correct: undefined,
176+
}),
177+
);
178+
179+
expect(getGrade()).toBeNull();
180+
});
181+
182+
it('preserves existing grades', () => {
183+
dispatchFetchSuccess(
184+
buildPayload({
185+
questionType: questionTypes.TextResponse,
186+
grade: 5,
187+
correct: true,
188+
}),
189+
);
190+
191+
expect(getGrade()).toBe(5);
192+
});
193+
});
194+
195+
describe('Programming', () => {
196+
const withTestCases = {
197+
public_test: [{ identifier: 'test1' }],
198+
};
199+
200+
it('prefills maximum grade when test cases exist and answer is correct', () => {
201+
dispatchFetchSuccess(
202+
buildPayload({
203+
questionType: questionTypes.Programming,
204+
grade: null,
205+
correct: true,
206+
testCases: withTestCases,
207+
}),
208+
);
209+
210+
expect(getGrade()).toBe(10);
211+
});
212+
213+
it('does not prefill 0 when test cases exist and answer is incorrect', () => {
214+
dispatchFetchSuccess(
215+
buildPayload({
216+
questionType: questionTypes.Programming,
217+
grade: null,
218+
correct: false,
219+
testCases: withTestCases,
220+
}),
221+
);
222+
223+
expect(getGrade()).toBeNull();
224+
});
225+
226+
it('leaves grade as null when no test cases exist, even if answer is correct', () => {
227+
dispatchFetchSuccess(
228+
buildPayload({
229+
questionType: questionTypes.Programming,
230+
grade: null,
231+
correct: true,
232+
testCases: null,
233+
}),
234+
);
235+
236+
expect(getGrade()).toBeNull();
237+
});
238+
});
239+
});

‎client/app/bundles/course/assessment/submission/reducers/grading/index.js‎

Lines changed: 66 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -39,58 +39,89 @@ const extractGrades = (answers) =>
3939
return draft;
4040
}, {});
4141

42-
const isSpecificAnswerGradePrefillableMap = {
43-
[questionTypes.MultipleChoice]: () => true,
44-
[questionTypes.MultipleResponse]: () => true,
45-
[questionTypes.Programming]: (answer) => {
46-
const { testCases } = answer;
47-
const isPublicTestCasesExist = testCases?.public_test?.length > 0;
48-
const isPrivateTestCasesExist = testCases?.private_test?.length > 0;
49-
const isEvaluationTestCasesExist = testCases?.evaluation_test?.length > 0;
50-
return (
51-
isPublicTestCasesExist ||
52-
isPrivateTestCasesExist ||
53-
isEvaluationTestCasesExist
54-
);
55-
},
56-
[questionTypes.RubricBasedResponse]: () => false,
57-
[questionTypes.TextResponse]: () => false,
58-
[questionTypes.Comprehension]: () => false,
59-
[questionTypes.FileUpload]: () => false,
60-
[questionTypes.Scribing]: () => false,
61-
[questionTypes.VoiceResponse]: () => false,
62-
[questionTypes.ForumPostResponse]: () => false,
42+
const NEVER_PREFILL_POLICY = {
43+
canPrefillFullCredit: () => false,
44+
canPrefillZeroCredit: false,
45+
};
46+
47+
const ALWAYS_PREFILL_POLICY = {
48+
canPrefillFullCredit: () => true,
49+
canPrefillZeroCredit: true,
50+
};
51+
52+
const ONLY_PREFILL_FULL_POLICY = {
53+
canPrefillFullCredit: () => true,
54+
canPrefillZeroCredit: false,
6355
};
6456

65-
const isAnswerGradePrefillable = (answer, questionType) => {
66-
const isAnswerPrefillable =
67-
answer.grading.grade === null && answer.explanation?.correct;
68-
const isSpecificAnswerPrefillable =
69-
isSpecificAnswerGradePrefillableMap[questionType](answer);
70-
return isAnswerPrefillable && isSpecificAnswerPrefillable;
57+
const PROGRAMMING_PREFILL_POLICY = {
58+
canPrefillFullCredit: ({ testCases }) =>
59+
(testCases?.public_test?.length ?? 0) > 0 ||
60+
(testCases?.private_test?.length ?? 0) > 0 ||
61+
(testCases?.evaluation_test?.length ?? 0) > 0,
62+
63+
// Partial grading is possible
64+
canPrefillZeroCredit: false,
65+
};
66+
67+
const prefillPolicies = {
68+
[questionTypes.MultipleChoice]: ALWAYS_PREFILL_POLICY,
69+
[questionTypes.MultipleResponse]: ALWAYS_PREFILL_POLICY,
70+
71+
[questionTypes.Programming]: PROGRAMMING_PREFILL_POLICY,
72+
73+
[questionTypes.TextResponse]: ONLY_PREFILL_FULL_POLICY,
74+
[questionTypes.Comprehension]: ONLY_PREFILL_FULL_POLICY,
75+
76+
[questionTypes.RubricBasedResponse]: NEVER_PREFILL_POLICY,
77+
[questionTypes.FileUpload]: NEVER_PREFILL_POLICY,
78+
[questionTypes.Scribing]: NEVER_PREFILL_POLICY,
79+
[questionTypes.VoiceResponse]: NEVER_PREFILL_POLICY,
80+
[questionTypes.ForumPostResponse]: NEVER_PREFILL_POLICY,
81+
};
82+
83+
const getPrefilledGrade = (answer, questionType, maxGrade) => {
84+
const existingGrade = answer?.grading?.grade;
85+
if (existingGrade != null) return existingGrade;
86+
87+
const policy = prefillPolicies[questionType];
88+
89+
if (
90+
answer?.explanation?.correct === true &&
91+
policy?.canPrefillFullCredit(answer)
92+
) {
93+
return maxGrade;
94+
}
95+
96+
if (answer?.explanation?.correct === false && policy?.canPrefillZeroCredit) {
97+
return 0;
98+
}
99+
100+
return null;
71101
};
72102

73103
/**
74-
* Extracts grades from `payload.answer`, and pre-fills the maximum grade for correct
75-
* answers that have not been graded. "Correct" follows the definition of
76-
* `explanation.correct` from the server.
104+
* Extracts grades from `payload.answer` and pre-fills:
105+
* - maximum grade for correct answers
106+
* - 0 for incorrect answers
107+
* when they have not already been graded.
108+
* "Correct" and "incorrect" follows the definition of `explanation.correct` from the server.
77109
*/
78110
const extractPrefillableGrades = (payload) => {
79111
const mapQuestionIdToQuestion = arrayToObjectWithKey(payload.questions, 'id');
80112

81113
return payload.answers.reduce((draft, answer) => {
82114
const { questionId, grading } = answer;
83-
const prefillable = isAnswerGradePrefillable(
115+
const prefilledGrade = getPrefilledGrade(
84116
answer,
85117
mapQuestionIdToQuestion[questionId].type,
118+
mapQuestionIdToQuestion[questionId].maximumGrade,
86119
);
87120
draft[questionId] = {
88121
...grading,
89122
originalGrade: grading.grade,
90-
grade: prefillable
91-
? mapQuestionIdToQuestion[questionId].maximumGrade
92-
: grading.grade,
93-
prefilled: prefillable,
123+
grade: prefilledGrade,
124+
prefilled: grading.grade == null && prefilledGrade !== null,
94125
};
95126

96127
return draft;

‎spec/features/course/assessment/submission/password_protected_and_delayed_publishing_spec.rb‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,13 @@
8181

8282
visit current_path
8383

84-
# Grade the submission with empty answer grade
85-
expect(page).to have_button('Submit for Publishing', disabled: true)
86-
find_field(class: 'grade').set(0)
87-
wait_for_autosave
84+
# Wrong MRQ answers are prefilled to 0 automatically, so the button is
85+
# already enabled without staff needing to manually enter a grade.
86+
expect(page).to have_button('Submit for Publishing', disabled: false)
8887

8988
find_field(class: 'exp').set(50)
9089

9190
click_button('Save')
92-
expect(page).to have_button('Submit for Publishing', disabled: false)
9391
click_button('Submit for Publishing')
9492

9593
expect(current_path).

0 commit comments

Comments
 (0)