Skip to content

Commit

Permalink
Consolidate auto-grading types (#6787)
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya authored Oct 21, 2024
1 parent 2dc1a35 commit aa8f1b9
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 59 deletions.
7 changes: 7 additions & 0 deletions lms/static/scripts/frontend_apps/api-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,14 @@ export type ActivityCalculation = 'cumulative' | 'separate';
export type AutoGradingConfig = {
grading_type: GradingType;
activity_calculation: ActivityCalculation;

/**
* Required number of annotations if activityCalculation is 'separate' or
* combined number of annotations and replies otherwise.
*/
required_annotations: number;

/** Required number of replies if activityCalculation is 'separate' */
required_replies?: number;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,14 @@ import {
import type { ComponentChildren } from 'preact';
import { useCallback, useId } from 'preact/hooks';

import type { ActivityCalculation, GradingType } from '../api-types';
import type {
GradingType,
AutoGradingConfig as APIAutoGradingConfig,
} from '../api-types';

export type AutoGradingConfig = {
export type AutoGradingConfig = APIAutoGradingConfig & {
/** Whether auto grading is enabled for the assignment or not */
enabled?: boolean;
gradingType: GradingType;
activityCalculation: ActivityCalculation;

/**
* Required number of annotations if activityCalculation is 'separate' or
* combined number of annotations and replies otherwise.
*/
requiredAnnotations: number;

/**
* Required number of replies if activityCalculation is 'separate'
*/
requiredReplies?: number;
};

type AnnotationsGoalInputProps = {
Expand Down Expand Up @@ -90,10 +80,10 @@ export default function AutoGradingConfigurator({
}: AutoGradingConfiguratorProps) {
const {
enabled = false,
gradingType,
activityCalculation,
requiredAnnotations,
requiredReplies = 0,
grading_type: gradingType,
activity_calculation: activityCalculation,
required_annotations: requiredAnnotations,
required_replies: requiredReplies = 0,
} = config;
const updateConfig = useCallback(
(newConfig: Partial<AutoGradingConfig>) =>
Expand Down Expand Up @@ -127,7 +117,9 @@ export default function AutoGradingConfigurator({
data-testid="grading-type-radio-group"
aria-labelledby={gradingTypeId}
selected={gradingType}
onChange={gradingType => updateConfig({ gradingType })}
onChange={gradingType =>
updateConfig({ grading_type: gradingType })
}
>
<RadioGroup.Radio
value="all_or_nothing"
Expand All @@ -152,7 +144,7 @@ export default function AutoGradingConfigurator({
aria-labelledby={activityCalculationId}
selected={activityCalculation}
onChange={activityCalculation =>
updateConfig({ activityCalculation })
updateConfig({ activity_calculation: activityCalculation })
}
>
<RadioGroup.Radio
Expand All @@ -177,7 +169,7 @@ export default function AutoGradingConfigurator({
gradingType={gradingType}
value={requiredAnnotations}
onChange={requiredAnnotations =>
updateConfig({ requiredAnnotations })
updateConfig({ required_annotations: requiredAnnotations })
}
>
{activityCalculation === 'cumulative'
Expand All @@ -188,7 +180,9 @@ export default function AutoGradingConfigurator({
<AnnotationsGoalInput
gradingType={gradingType}
value={requiredReplies}
onChange={requiredReplies => updateConfig({ requiredReplies })}
onChange={requiredReplies =>
updateConfig({ required_replies: requiredReplies })
}
min={0}
>
Replies
Expand Down
34 changes: 8 additions & 26 deletions lms/static/scripts/frontend_apps/components/FilePickerApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -203,42 +203,24 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) {
if (!assignmentAutoGradingConfig) {
return {
enabled: false,
gradingType: 'all_or_nothing',
activityCalculation: 'cumulative',
requiredAnnotations: 1,
grading_type: 'all_or_nothing',
activity_calculation: 'cumulative',
required_annotations: 1,
};
}

// Initialize with the assignment's auto-grading config if it exists
return {
enabled: true,
gradingType: assignmentAutoGradingConfig.grading_type,
activityCalculation: assignmentAutoGradingConfig.activity_calculation,
requiredAnnotations: assignmentAutoGradingConfig.required_annotations,
requiredReplies: assignmentAutoGradingConfig.required_replies,
...assignmentAutoGradingConfig,
};
},
);
// The auto-grading config as expected by the backend
const autoGradingConfigToSave = useMemo(
() =>
autoGradingEnabled && autoGradingConfig.enabled
? {
grading_type: autoGradingConfig.gradingType,
activity_calculation: autoGradingConfig.activityCalculation,
required_annotations: autoGradingConfig.requiredAnnotations,
required_replies: autoGradingConfig.requiredReplies,
}
: null,
[
autoGradingConfig.activityCalculation,
autoGradingConfig.enabled,
autoGradingConfig.gradingType,
autoGradingConfig.requiredAnnotations,
autoGradingConfig.requiredReplies,
autoGradingEnabled,
],
);
const autoGradingConfigToSave: APIAutoGradingConfig | null = useMemo(() => {
const { enabled, ...rest } = autoGradingConfig;
return autoGradingEnabled && enabled ? rest : null;
}, [autoGradingConfig, autoGradingEnabled]);

// Flag indicating if we are editing content that was previously selected.
const [editingContent, setEditingContent] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ describe('AutoGradingConfigurator', () => {

beforeEach(() => {
fakeAutoGradingConfig = {
gradingType: 'all_or_nothing',
activityCalculation: 'cumulative',
requiredAnnotations: 1,
grading_type: 'all_or_nothing',
activity_calculation: 'cumulative',
required_annotations: 1,
};
fakeUpdateAutoGradingConfig = sinon.stub();
});
Expand Down Expand Up @@ -66,12 +66,12 @@ describe('AutoGradingConfigurator', () => {

assert.calledWith(
fakeUpdateAutoGradingConfig,
sinon.match({ activityCalculation }),
sinon.match({ activity_calculation: activityCalculation }),
);
});

it('renders inputs based on activity calculation value', () => {
fakeAutoGradingConfig.activityCalculation = activityCalculation;
fakeAutoGradingConfig.activity_calculation = activityCalculation;

const wrapper = createComponent();
const inputs = wrapper.find('AnnotationsGoalInput');
Expand All @@ -97,12 +97,12 @@ describe('AutoGradingConfigurator', () => {

assert.calledWith(
fakeUpdateAutoGradingConfig,
sinon.match({ gradingType }),
sinon.match({ grading_type: gradingType }),
);
});

it('renders different input label depending on grading type value', () => {
fakeAutoGradingConfig.gradingType = gradingType;
fakeAutoGradingConfig.grading_type = gradingType;

const wrapper = createComponent();
const input = wrapper.find('AnnotationsGoalInput').first();
Expand All @@ -119,16 +119,16 @@ describe('AutoGradingConfigurator', () => {
{
inputIndex: 0,
value: '15',
expectedConfig: { requiredAnnotations: 15 },
expectedConfig: { required_annotations: 15 },
},
{
inputIndex: 1,
value: '3',
expectedConfig: { requiredReplies: 3 },
expectedConfig: { required_replies: 3 },
},
].forEach(({ inputIndex, value, expectedConfig }) => {
it('updates config when inputs change', () => {
fakeAutoGradingConfig.activityCalculation = 'separate';
fakeAutoGradingConfig.activity_calculation = 'separate';

const wrapper = createComponent();
const inputs = wrapper.find('AnnotationsGoalInput');
Expand Down

0 comments on commit aa8f1b9

Please sign in to comment.