Skip to content
This repository has been archived by the owner on Sep 3, 2024. It is now read-only.

feat: add grading method field in problem editor #430

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions example/setupTest.example.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mergeConfig({
STUDIO_BASE_URL: process.env.STUDIO_BASE_URL,
BLOCKSTORE_COLLECTION_UUID: process.env.BLOCKSTORE_COLLECTION_UUID,
SECURE_ORIGIN_XBLOCK_BOOTSTRAP_HTML_URL: process.env.SECURE_ORIGIN_XBLOCK_BOOTSTRAP_HTML_URL,
ENABLE_GRADING_METHOD_IN_PROBLEMS: process.env.ENABLE_GRADING_METHOD_IN_PROBLEMS,
});

window.MutationObserver = MutationObserver;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,18 @@ export const scoringCardHooks = (scoring, updateSettings, defaultValue) => {
updateSettings({ scoring: { ...scoring, weight } });
};

const handleGradingMethodChange = (event) => {
const { value } = event.target;
updateSettings({ scoring: { ...scoring, gradingMethod: value } });
};

return {
attemptDisplayValue,
handleUnlimitedChange,
handleMaxAttemptChange,
handleOnChange,
handleWeightChange,
handleGradingMethodChange,
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ describe('Problem settings hooks', () => {
unlimited: false,
number: 5,
},
gradingMethod: 'last_score',
};
const defaultValue = 1;
test('test scoringCardHooks initializes display value when attempts.number is null', () => {
Expand Down Expand Up @@ -268,6 +269,11 @@ describe('Problem settings hooks', () => {
output.handleWeightChange({ target: { value } });
expect(updateSettings).toHaveBeenCalledWith({ scoring: { ...scoring, weight: parseFloat(value) } });
});
test('test handleGradingMethodChange', () => {
const value = 'first_score';
output.handleGradingMethodChange({ target: { value } });
expect(updateSettings).toHaveBeenCalledWith({ scoring: { ...scoring, gradingMethod: value } });
});
});

describe('Show answer card hooks', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ SettingsWidget.propTypes = {
showanswer: PropTypes.string,
showResetButton: PropTypes.bool,
rerandomize: PropTypes.string,
gradingMethod: PropTypes.string,
}).isRequired,
// eslint-disable-next-line
settings: PropTypes.any.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('SettingsWidget', () => {
maxAttempts: 2,
showanswer: 'finished',
showResetButton: false,
gradingMethod: 'last_score',
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ const messages = defineMessages({
defaultMessage: 'Points',
description: 'Scoring weight input label',
},
scoringGradingMethodInputLabel: {
id: 'authoring.problemeditor.settings.scoring.grading.method.inputLabel',
defaultMessage: 'Grading Method',
description: 'Grading method input label',
},
gradingMethodSummary: {
id: 'authoring.problemeditor.settings.scoring.grading.method',
defaultMessage: '{gradingMethod}',
description: 'Summary text for scoring grading method',
},
unlimitedAttemptsSummary: {
id: 'authoring.problemeditor.settings.scoring.unlimited',
defaultMessage: 'Unlimited attempts',
Expand All @@ -107,6 +117,11 @@ const messages = defineMessages({
defaultMessage: 'Specify point weight and the number of answer attempts',
description: 'Descriptive text for scoring settings',
},
scoringSettingsLabelWithGradingMethod: {
id: 'authoring.problemeditor.settings.scoring.label.withGradingMethod',
defaultMessage: 'Specify grading method, point weight and the number of answer attempts',
description: 'Descriptive text for scoring settings when grading method is enabled',
},
attemptsHint: {
id: 'authoring.problemeditor.settings.scoring.attempts.hint',
defaultMessage: 'If a default value is not set in advanced settings, unlimited attempts are allowed',
Expand All @@ -117,6 +132,11 @@ const messages = defineMessages({
defaultMessage: 'If a value is not set, the problem is worth one point',
description: 'Summary text for scoring weight',
},
gradingMethodHint: {
id: 'authoring.problemeditor.settings.scoring.grading.method.hint',
defaultMessage: 'Define the grading method for this problem. By default, it is the score of the last submission made by the student.',
description: 'Summary text for scoring grading method',
},
showAnswerSettingsTitle: {
id: 'authoring.problemeditor.settings.showAnswer.title',
defaultMessage: 'Show answer',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import _ from 'lodash-es';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { getConfig } from '@edx/frontend-platform';
import { Form, Hyperlink } from '@openedx/paragon';
import { selectors } from '../../../../../../data/redux';
import SettingsOption from '../SettingsOption';
import messages from '../messages';
import { scoringCardHooks } from '../hooks';
import { GradingMethod, GradingMethodKeys } from '../../../../../../data/constants/problem';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could become problematic if we ever want to move this file. Does

Suggested change
import { GradingMethod, GradingMethodKeys } from '../../../../../../data/constants/problem';
import { GradingMethod, GradingMethodKeys } from 'data/constants/problem';

not work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your review! I was trying your suggestions and unfortunately, it doesn't work. I get that it doesn't find the module :(. It must be an absolute path, and that's how it's currently done in all the code.


export const ScoringCard = ({
scoring,
Expand All @@ -20,32 +22,73 @@ export const ScoringCard = ({
learningContextId,
isLibrary,
}) => {
const isGradingMethodEnabled = getConfig().ENABLE_GRADING_METHOD_IN_PROBLEMS;
const {
handleUnlimitedChange,
handleMaxAttemptChange,
handleWeightChange,
handleGradingMethodChange,
handleOnChange,
attemptDisplayValue,
} = scoringCardHooks(scoring, updateSettings, defaultValue);

const getScoringSummary = (weight, attempts, unlimited) => {
const getScoringSummary = (weight, attempts, unlimited, gradingMethod) => {
let summary = intl.formatMessage(messages.weightSummary, { weight });
summary += ` ${String.fromCharCode(183)} `;
summary += unlimited
? intl.formatMessage(messages.unlimitedAttemptsSummary)
: intl.formatMessage(messages.attemptsSummary, { attempts: attempts || defaultValue });
if (isGradingMethodEnabled) {
summary += ` ${String.fromCharCode(183)} `;
summary += intl.formatMessage(messages.gradingMethodSummary, {
gradingMethod: intl.formatMessage(GradingMethod[gradingMethod]),
});
}
return summary;
};

return (
<SettingsOption
title={intl.formatMessage(messages.scoringSettingsTitle)}
summary={getScoringSummary(scoring.weight, scoring.attempts.number, scoring.attempts.unlimited)}
summary={
getScoringSummary(
scoring.weight,
scoring.attempts.number,
scoring.attempts.unlimited,
scoring.gradingMethod,
)
}
className="scoringCard"
>
<div className="mb-4">
<FormattedMessage {...messages.scoringSettingsLabel} />
{isGradingMethodEnabled && <FormattedMessage {...messages.scoringSettingsLabelWithGradingMethod} />}
{!isGradingMethodEnabled && <FormattedMessage {...messages.scoringSettingsLabel} />}
Comment on lines +64 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{isGradingMethodEnabled && <FormattedMessage {...messages.scoringSettingsLabelWithGradingMethod} />}
{!isGradingMethodEnabled && <FormattedMessage {...messages.scoringSettingsLabel} />}
{isGradingMethodEnabled ?
(<FormattedMessage {...messages.scoringSettingsLabelWithGradingMethod} />
) : (
<FormattedMessage {...messages.scoringSettingsLabel} />
)}

</div>
{isGradingMethodEnabled && (
<Form.Group>
<Form.Control
as="select"
value={scoring.gradingMethod}
onChange={handleGradingMethodChange}
floatingLabel={intl.formatMessage(messages.scoringGradingMethodInputLabel)}
>
{Object.values(GradingMethodKeys).map((gradingMethod) => {
const optionDisplayName = GradingMethod[gradingMethod];
return (
<option
key={gradingMethod}
value={gradingMethod}
>
{intl.formatMessage(optionDisplayName)}
</option>
);
})}
</Form.Control>
<Form.Control.Feedback>
<FormattedMessage {...messages.gradingMethodHint} />
</Form.Control.Feedback>
</Form.Group>
)}
<Form.Group>
<Form.Control
type="number"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import React from 'react';
import { shallow } from '@edx/react-unit-test-utils';
import { mergeConfig } from '@edx/frontend-platform';
import { formatMessage } from '../../../../../../../testUtils';
import { scoringCardHooks } from '../hooks';
import { ScoringCard } from './ScoringCard';
import { GradingMethodKeys } from '../../../../../../data/constants/problem';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about relative import paths as before (I see that formatMessage is also doing this here, could be nice to clean that up too)


mergeConfig({
ENABLE_GRADING_METHOD_IN_PROBLEMS: true,
});

jest.mock('../hooks', () => ({
scoringCardHooks: jest.fn(),
Expand All @@ -15,6 +21,7 @@ describe('ScoringCard', () => {
unlimited: false,
number: 5,
},
gradingMethod: GradingMethodKeys.LAST_SCORE,
updateSettings: jest.fn().mockName('args.updateSettings'),
intl: { formatMessage },
};
Expand All @@ -29,6 +36,7 @@ describe('ScoringCard', () => {
handleMaxAttemptChange: jest.fn().mockName('scoringCardHooks.handleMaxAttemptChange'),
handleWeightChange: jest.fn().mockName('scoringCardHooks.handleWeightChange'),
handleOnChange: jest.fn().mockName('scoringCardHooks.handleOnChange'),
handleGradingMethodChange: jest.fn().mockName('scoringCardHooks.handleGradingMethodChange'),
local: 5,
};

Expand Down
Loading
Loading