-
Notifications
You must be signed in to change notification settings - Fork 525
[Teachin-53] [CFU] Add support of Free response answers, add support of Level Group CFU levels #70515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Conversation
…d improve rendering logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for Free Response answers and Level Group CFU (Check for Understanding) levels to the Student CFU Widget. It introduces a discriminated union type system to handle different level types and implements the necessary rendering logic for these new level types.
Changes:
- Refactored type definitions to use discriminated unions for different CFU level types (LevelGroup vs. other types)
- Implemented Free Response answer rendering with SafeMarkdown support
- Added Level Group support with nested answer handling
- Removed debug console.log statements
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| types.ts | Refactored type definitions to use discriminated unions, added CFULevelGroup and CFULevelOther interfaces, and introduced CFULevelResponseResponse interface |
| studentCFUAnswers.module.scss | Added CSS styles for free response answer container and AI insight section |
| CFUMultiAnswer.tsx | Updated to accept answers as a direct prop and changed response prop to use CFULevelResponseResponse |
| CFUMatchAnswer.tsx | Updated response prop type to CFULevelResponseResponse |
| CFUFreeResponseAnswer.tsx | Implemented rendering of free response answers using SafeMarkdown |
| StudentCFUWidgetQuestionsSection.tsx | Removed debug console.log statements |
| CFUQuestionStudentAnswer.tsx | Added logic to handle Level Group answers with nested level results and index-based answer selection |
| CFUQuestion.tsx | Added Level Group to questionTypeMap and implemented array-based question text rendering for Level Groups |
Comments suppressed due to low confidence (1)
apps/src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx:71
- The switch statement doesn't handle the 'LevelGroup' case explicitly. When a level has type 'LevelGroup', it will fall through to the default case which shows a placeholder. Based on the PR's purpose to add support for Level Group CFU levels, this should be handled properly. Consider either: (1) adding a case for 'LevelGroup' if it requires special rendering, or (2) if LevelGroup should never reach this code due to the isLevelGroupAnswer flag, add a comment explaining this.
switch (levelType) {
case 'Multi':
return (
<CFUMultiAnswer
answers={
(isLevelGroupAnswer
? level.answers?.[levelGroupLevelIndex]
: level.answers) as CFUMultipleLevelAnswer[]
}
level={level}
response={studentResponse}
/>
);
case 'Match':
return <CFUMatchAnswer level={level} response={studentResponse} />;
case 'FreeResponse':
return <CFUFreeResponseAnswer response={studentResponse} />;
default:
return (
<Typography variant="body4">
{/* TODO: Handle additional CFU level type: {type} */}"{levelType}
" Student answer placeholder
</Typography>
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...emplates/studentSnapshot/studentCFUWidget/questionsSection/answers/CFUFreeResponseAnswer.tsx
Outdated
Show resolved
Hide resolved
...src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx
Outdated
Show resolved
Hide resolved
...src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx
Outdated
Show resolved
Hide resolved
apps/src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestion.tsx
Outdated
Show resolved
Hide resolved
...ates/studentSnapshot/studentCFUWidget/questionsSection/answers/studentCFUAnswers.module.scss
Outdated
Show resolved
Hide resolved
...src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx
Outdated
Show resolved
Hide resolved
...src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx
Show resolved
Hide resolved
…ection/answers/CFUFreeResponseAnswer.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ection/CFUQuestionStudentAnswer.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ection/CFUQuestionStudentAnswer.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
apps/src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestionStudentAnswer.tsx:79
- The switch statement does not explicitly handle the 'LevelGroup' case. When a LevelGroup level is passed directly (not as isLevelGroupAnswer), it will fall through to the default case. Consider adding explicit handling for 'LevelGroup' or documenting why it's expected to be handled differently through the isLevelGroupAnswer flag.
switch (levelType) {
case 'Multi':
return (
<CFUMultiAnswer
answers={
(isLevelGroupAnswer
? level.answers?.[levelGroupLevelIndex]
: level.answers || []) as CFUMultipleLevelAnswer[]
}
level={level}
response={studentResponse}
/>
);
case 'Match':
return <CFUMatchAnswer level={level} response={studentResponse} />;
case 'FreeResponse':
return <CFUFreeResponseAnswer response={studentResponse} />;
default:
return (
<Typography variant="body4">
{/* TODO: Handle additional CFU level type: {levelType} */}"
{levelType}
" Student answer placeholder
</Typography>
);
}
apps/src/templates/studentSnapshot/studentCFUWidget/questionsSection/CFUQuestion.tsx:92
- The code assumes that when 'level.question_text' is an array, it's always a LevelGroup level. However, there's no explicit check to verify this. Consider adding a type guard to ensure 'level.type === "LevelGroup"' before treating it as a LevelGroup, or document this assumption clearly if it's guaranteed by the backend.
{Array.isArray(level.question_text) ? (
<>
{level.question_text.map((questionText, index) => (
<CFUQuestionStudentAnswer
key={questionText}
level={level}
isLevelGroupAnswer
levelGroupLevelIndex={index}
response={response}
questionText={questionText}
isOpen={isOpen}
/>
))}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
[Teachin-53] [CFU] Add support of Free response answers, add support of Level Group CFU levels
2026-01-27.21.10.41.mov
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Creation Checklist: