Skip to content
Open
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

# production
/build
/dist

# misc
.DS_Store
Expand Down
55 changes: 54 additions & 1 deletion src/pages/Assignments/AssignmentEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { render, screen, within } from "@testing-library/react";
import "@testing-library/jest-dom";
import { vi, beforeEach, describe, expect, it } from "vitest";
import AssignmentEditor from "./AssignmentEditor";
import { transformAssignmentRequest, IAssignmentFormValues } from "./AssignmentUtil";
import { transformAssignmentRequest, transformAssignmentResponse, IAssignmentFormValues } from "./AssignmentUtil";

// Mock useAPI to avoid real network calls
const sendRequestMock = vi.fn();
Expand Down Expand Up @@ -194,4 +194,57 @@ describe("transformAssignmentRequest", () => {

expect(payload.assignment.vary_by_round).toBe(false);
});

it("maps topic rubric variation to vary_by_topic", () => {
const values: IAssignmentFormValues = {
id: 1,
name: "Test Assignment",
directory_path: "assignment_1",
spec_location: "http://example.com",
private: false,
show_template_review: false,
require_quiz: false,
has_badge: false,
staggered_deadline: false,
is_calibrated: false,
review_rubric_varies_by_topic: true,
weights: [],
notification_limits: [],
use_date_updater: [],
submission_allowed: [],
review_allowed: [],
teammate_allowed: [],
metareview_allowed: [],
reminder: [],
};

const payload = JSON.parse(transformAssignmentRequest(values));

expect(payload.assignment.vary_by_topic).toBe(true);
});
});

describe("transformAssignmentResponse", () => {
it("prefills topic rubric variation from vary_by_topic", () => {
const assignment = {
id: 1,
name: "Test Assignment",
directory_path: "assignment_1",
spec_location: "http://example.com",
private: false,
show_template_review: false,
require_quiz: false,
has_badge: false,
staggered_deadline: false,
is_calibrated: false,
vary_by_topic: true,
num_review_rounds: 1,
due_dates: [],
assignment_questionnaires: [],
};

const values = transformAssignmentResponse(JSON.stringify(assignment));

expect(values.review_rubric_varies_by_topic).toBe(true);
});
});
144 changes: 137 additions & 7 deletions src/pages/Assignments/AssignmentEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ interface TopicData {
updatedAt?: string;
}

interface TopicRubricMapping {
id: number;
questionnaire_id: number;
questionnaire_name?: string;
project_topic_id: number | null;
used_in_round: number | null;
}

const initialValues: IAssignmentFormValues = {
name: "",
directory_path: "",
Expand Down Expand Up @@ -115,6 +123,8 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
const [calibrationSubmissions, setCalibrationSubmissions] = useState<any[]>([]);

const { data: topicsResponse, error: topicsApiError, sendRequest: fetchTopics } = useAPI();
const { data: topicRubricMappingsResponse, error: topicRubricMappingsError, sendRequest: fetchTopicRubricMappings } = useAPI();
const { error: saveTopicRubricError, sendRequest: saveTopicRubricMapping } = useAPI();
const { data: updateResponse, error: updateError, sendRequest: updateAssignment } = useAPI();
const { data: deleteResponse, error: deleteError, sendRequest: deleteTopic } = useAPI();
const { data: createResponse, error: createError, sendRequest: createTopic } = useAPI();
Expand Down Expand Up @@ -165,6 +175,7 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
const [assignmentDuties, setAssignmentDuties] = useState<any[]>([]);
const [selectedDutyIds, setSelectedDutyIds] = useState<number[]>([]);
const [roleBasedLocalError, setRoleBasedLocalError] = useState<string | null>(null);
const [topicRubricMappings, setTopicRubricMappings] = useState<TopicRubricMapping[]>([]);


useEffect(() => {
Expand Down Expand Up @@ -291,6 +302,15 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
}
}, [id, fetchTopics]);

const refreshTopicRubricMappings = useCallback(() => {
if (!id) return;
fetchTopicRubricMappings({ url: `/assignment_questionnaires?assignment_id=${id}` });
}, [fetchTopicRubricMappings, id]);

useEffect(() => {
refreshTopicRubricMappings();
}, [refreshTopicRubricMappings]);

const refreshAccessibleDuties = useCallback(() => {
fetchAccessibleDuties({ url: `/duties/accessible_duties` });
}, [fetchAccessibleDuties]);
Expand Down Expand Up @@ -343,6 +363,14 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
setTopicsLoading(false);
}
}, [topicsResponse]);

useEffect(() => {
if (topicRubricMappingsResponse?.data) {
const mappings = (topicRubricMappingsResponse.data || [])
.filter((mapping: any) => mapping.project_topic_id !== null && mapping.project_topic_id !== undefined);
setTopicRubricMappings(mappings);
}
}, [topicRubricMappingsResponse]);

// Handle topics API errors
useEffect(() => {
Expand All @@ -352,6 +380,18 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
}
}, [topicsApiError]);

useEffect(() => {
if (topicRubricMappingsError) {
dispatch(alertActions.showAlert({ variant: "danger", message: topicRubricMappingsError }));
}
}, [topicRubricMappingsError, dispatch]);

useEffect(() => {
if (saveTopicRubricError) {
dispatch(alertActions.showAlert({ variant: "danger", message: saveTopicRubricError }));
}
}, [saveTopicRubricError, dispatch]);

const toggleDutySelection = useCallback((dutyId: number) => {
setSelectedDutyIds((prev) =>
prev.includes(dutyId) ? prev.filter((id) => id !== dutyId) : [...prev, dutyId]
Expand Down Expand Up @@ -491,6 +531,62 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
console.log(`Applying to partner ad for topic ${topicId}: ${applicationText}`);
// TODO: Implement partner ad application logic
}, []);

const findTopicRubricMapping = useCallback((topicDatabaseId: number, usedInRound: number | null) => {
return topicRubricMappings.find((mapping) =>
Number(mapping.project_topic_id) === Number(topicDatabaseId) &&
(mapping.used_in_round ?? null) === (usedInRound ?? null)
);
}, [topicRubricMappings]);

const handleTopicRubricChange = useCallback(async (
topicDatabaseId: number,
questionnaireId: number | null,
usedInRound: number | null
) => {
if (!id) return;

const existingMapping = findTopicRubricMapping(topicDatabaseId, usedInRound);

try {
if (!questionnaireId) {
if (existingMapping) {
await saveTopicRubricMapping({
url: `/assignment_questionnaires/${existingMapping.id}`,
method: HttpMethod.DELETE,
});
}
} else if (existingMapping) {
await saveTopicRubricMapping({
url: `/assignment_questionnaires/${existingMapping.id}`,
method: HttpMethod.PATCH,
data: {
assignment_questionnaire: {
questionnaire_id: questionnaireId,
},
},
});
} else {
await saveTopicRubricMapping({
url: `/assignment_questionnaires`,
method: HttpMethod.POST,
data: {
assignment_questionnaire: {
assignment_id: Number(id),
questionnaire_id: questionnaireId,
project_topic_id: topicDatabaseId,
used_in_round: usedInRound,
},
},
});
}

refreshTopicRubricMappings();
dispatch(alertActions.showAlert({ variant: "success", message: "Topic rubric mapping saved successfully" }));
} catch {
// useAPI surfaces the request error through saveTopicRubricError.
}
}, [dispatch, findTopicRubricMapping, id, refreshTopicRubricMappings, saveTopicRubricMapping]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated



Expand Down Expand Up @@ -580,10 +676,16 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
return;
}

// validate sum of weights = 100%
const totalWeight = values.weights?.reduce((acc: number, curr: number) => acc + curr, 0) || 0;
// validate sum of weights = 100% only when the user has actually entered weights
const filledWeights = (values.weights || []).filter(
(weight: any) => weight !== "" && weight !== null && weight !== undefined
);
const totalWeight = filledWeights.reduce(
(acc: number, curr: any) => acc + Number(curr),
0
);

const hasWeights = (values.weights?.length ?? 0) > 0;
const hasWeights = filledWeights.length > 0;

if (hasWeights && totalWeight !== 100) {
dispatch(alertActions.showAlert({ variant: "danger", message: "Sum of weights must be 100%" }));
Expand Down Expand Up @@ -624,6 +726,13 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
value: q.id,
}));

const reviewRubricOptions = (assignmentData.questionnaires || [])
.filter((q: any) => q.questionnaire_type === "ReviewQuestionnaire")
.map((q: any) => ({
label: q.name,
value: q.id,
}));
Comment on lines +756 to +761
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use reviewRubricOptions for the assignment review-rubric rows too.

This filtered list only gets passed into TopicsTab; the review-round/default rows in the Rubrics table still read from questionnaireOptions, so non-review questionnaires can still be saved as the assignment’s review rubric once they appear in assignmentData.questionnaires.

Suggested patch
{
  id: i + 1,
  rowKey: i + 1,
  title: `Review round ${i + 1}:`,
- questionnaire_options: questionnaireOptions,
+ questionnaire_options: reviewRubricOptions,
  selected_questionnaire: roundSelections[i + 1]?.id,
  questionnaire_type: 'dropdown',
},
...
{
  id: 0,
  rowKey: 0,
  title: "Review rubric:",
- questionnaire_options: questionnaireOptions,
+ questionnaire_options: reviewRubricOptions,
  selected_questionnaire: roundSelections[1]?.id,
  questionnaire_type: 'dropdown',
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/Assignments/AssignmentEditor.tsx` around lines 756 - 761, The
review-rubric rows in the Rubrics table still use the generic
questionnaireOptions, allowing non-review questionnaires to be saved; change
those rows to use the filtered reviewRubricOptions instead. Locate where the
Rubrics table renders the review-round/default rows (the code that currently
references questionnaireOptions) and replace the data source/prop with
reviewRubricOptions so only items from the review-questionnaire filter are
selectable/saved; also ensure any prop passed down to TopicsTab or its child
components that represent review-rubric choices uses reviewRubricOptions (not
questionnaireOptions) to keep behavior consistent.


const reviewRounds = assignmentData.number_of_review_rounds;

// Build initial form values from existing assignment data (update) or defaults (create)
Expand Down Expand Up @@ -746,12 +855,18 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
topicsData={topicsData}
topicsLoading={topicsLoading}
topicsError={topicsError}
varyByTopic={!!formik.values.review_rubric_varies_by_topic}
varyByRound={!!formik.values.review_rubric_varies_by_round}
reviewRounds={formik.values.number_of_review_rounds || reviewRounds || 1}
reviewRubricOptions={reviewRubricOptions}
topicRubricMappings={topicRubricMappings}
onTopicSettingChange={handleTopicSettingChange}
onDropTeam={handleDropTeam}
onDeleteTopic={handleDeleteTopic}
onEditTopic={handleEditTopic}
onCreateTopic={handleCreateTopic}
onApplyPartnerAd={handleApplyPartnerAd}
onTopicRubricChange={handleTopicRubricChange}
onTopicsChanged={() => id && fetchTopics({ url: `/project_topics?assignment_id=${id}` })}
/>
</Tab>
Expand Down Expand Up @@ -841,12 +956,27 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {
},
{
cell: ({ row }) => <div style={{ marginRight: '10px' }}>{row.original.questionnaire_type === 'dropdown' &&
(() => {
let questionnaireFieldName = `questionnaire_round_${row.original.id}`;
let questionnaireControlId = `assignment-questionnaire_${row.original.id}`;

if (row.original.title === "Author feedback:") {
questionnaireFieldName = "author_feedback_questionnaire";
questionnaireControlId = "assignment-author_feedback_questionnaire";
} else if (row.original.title === "Teammate review:") {
questionnaireFieldName = "teammate_review_questionnaire";
questionnaireControlId = "assignment-teammate_review_questionnaire";
}

return (
<FormSelect
controlId={`assignment-questionnaire_${row.original.id}`}
name={`questionnaire_round_${row.original.id}`}
controlId={questionnaireControlId}
name={questionnaireFieldName}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
options={row.original.questionnaire_options || []}
// Formik initialValues handles prefill via questionnaire_round_X fields
/>}
/>
);
})()}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{row.original.questionnaire_type === 'tag_prompts' &&
<div style={{ marginBottom: '10px' }}><Button variant="outline-secondary">+Tag prompt+</Button>
<Button variant="outline-secondary">-Tag prompt-</Button></div>}</div>,
Expand Down Expand Up @@ -1265,7 +1395,7 @@ const AssignmentEditor: React.FC<IEditor> = ({ mode }) => {

{/* Submit button */}
<div className="mt-3 d-flex justify-content-start gap-2" style={{ alignItems: 'center' }}>
<Button type="submit" variant="outline-secondary">
<Button type="button" variant="outline-secondary" onClick={() => formik.submitForm()}>
Save
</Button> |
<a href="/assignments" style={{ color: '#a4a366', textDecoration: 'none' }}>Back</a>
Expand Down
4 changes: 4 additions & 0 deletions src/pages/Assignments/AssignmentUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export interface IAssignmentFormValues {
assignment_questionnaires?: {
id: number;
used_in_round?: number;
project_topic_id?: number | null;
questionnaire_id?: number;
questionnaire?: { id: number; name: string };
}[];
[key: string]: any;
Expand Down Expand Up @@ -177,6 +179,7 @@ export const transformAssignmentRequest = (values: IAssignmentFormValues) => {

// Per-round rubric configuration
vary_by_round: values.review_rubric_varies_by_round,
vary_by_topic: values.review_rubric_varies_by_topic,
rounds_of_reviews: values.number_of_review_rounds,
assignment_questionnaires_attributes: assignmentQuestionnaires,

Expand Down Expand Up @@ -240,6 +243,7 @@ export const transformAssignmentResponse = (assignmentResponse: string) => {
// review rounds / rubrics
review_rubric_varies_by_round:
assignment.varying_rubrics_by_round ?? assignment.vary_by_round,
review_rubric_varies_by_topic: (assignment as any).vary_by_topic ?? false,
number_of_review_rounds: assignment.num_review_rounds,
is_role_based: (assignment as any).is_role_based ?? false,

Expand Down
Loading