E2619 - Student Quiz Frontend#179
Conversation
…. Have not started on Quiz rendering yet.
E2607 ResponseController_Frontend
UI fixes for questionnaires, item types, and assignment editor
…to review process
…d when the student task list is completed
… frontend Co-authored-by: Copilot <copilot@github.com>
📝 WalkthroughWalkthroughAdds a student-facing Assigned Reviews page, quiz support in questionnaires, persistent reviewer mapping CRUD, dynamic teammate review rendering with draft/submit flows, and assignment/questionnaire form model updates and routing adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Student
participant UI as AssignedReviews Page
participant Storage as localStorage
participant API as Backend API
User->>UI: Open Assigned Reviews
UI->>Storage: Read assignreviewer:{assignmentId}
UI->>API: GET /student_tasks/list
API-->>UI: student tasks & gating info
UI->>API: GET /response_maps
API-->>UI: response maps
UI->>UI: Merge & normalize rows
User->>UI: Click review row
alt Quiz required & not taken
UI->>API: POST /quiz_response_maps (reviewee_team_id)
API-->>UI: quiz map + questionnaire id
UI->>UI: Navigate to Quiz Form (with redirect_after)
else Direct review
UI->>UI: Navigate to Review Form (with questionnaire params)
end
sequenceDiagram
participant User as Reviewer
participant UI as TeammateReview Page
participant API as Backend API
User->>UI: Load review form (map_id, questionnaire_id)
UI->>API: GET /questionnaires/:id/items
API-->>UI: questionnaire items
alt Existing response (map_id)
UI->>API: GET /responses?map_id=...
API-->>UI: existing scores/comments
UI->>UI: Populate form (including multi-select)
end
User->>UI: Edit answers
User->>UI: Click "Save Draft"
UI->>API: PATCH /responses/:id (scores_attributes only)
API-->>UI: Draft saved
User->>UI: Click "Submit"
alt Missing response record
UI->>API: POST /response_maps (fallback)
API-->>UI: created response_map
end
UI->>API: PATCH /responses/:id (all scores)
UI->>API: POST /responses/:id/submit
API-->>UI: locked, total_score (quiz mode)
alt Quiz mode
UI->>User: Show total_score, optionally redirect
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 2/3 reviews remaining, refill in 20 minutes. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
src/pages/Questionnaires/QuestionnaireForm.tsx (1)
180-180: Props correctly passed to enable quiz authoring.The
questionnaireTypeprop enables theQuestionnaireItemsFieldArrayto conditionally render correct-answer fields for quiz items. The nullish coalescing to""handles the initial undefined state safely.Consider extracting the
itemTypestransformation to improve readability:Extract itemTypes for better readability
+ const itemTypeNames = (itemTypes?.data?.map((t: any) => t.name) as string[]) ?? []; <QuestionnaireItemsFieldArray values={values} errors={errors} touched={touched} - itemTypes={(itemTypes?.data?.map((t: any) => t.name) as string[]) ?? []} + itemTypes={itemTypeNames} questionnaireType={values.questionnaire_type ?? ""} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Questionnaires/QuestionnaireForm.tsx` at line 180, Extract the inline transformation of itemTypes into a local variable to improve readability: create a const (e.g., itemTypeNames) that computes (itemTypes?.data?.map((t: any) => t.name) as string[]) ?? [], then pass itemTypeNames to the QuestionnaireItemsFieldArray prop itemTypes and keep questionnaireType as values.questionnaire_type ?? ""; update the component invocation (QuestionnaireItemsFieldArray) to use the new variable instead of the inline expression.src/pages/Student Teams/TeammateReview.tsx (3)
765-772: Grid radio button uses emptyonChangehandler.The
onChange={() => {}}is a no-op whileonClickhandles the toggle logic. This works but triggers React's controlled component warning in strict mode. TheonClickhandler also allows toggling off a selected radio, which is non-standard radio behavior.Consider using standard radio behavior or document the toggle-off intent
<Form.Check type="radio" name={`grid-${itemId}-row-${rowIdx}`} checked={rowSelections[rowIdx] === col} - onChange={() => {}} - onClick={() => { - const updated = [...rowSelections]; - while (updated.length <= rowIdx) updated.push(''); - updated[rowIdx] = updated[rowIdx] === col ? '' : col; - setAnswers(prev => ({ ...prev, [itemId]: updated.join('|') })); - }} + onChange={(e) => { + const updated = [...rowSelections]; + while (updated.length <= rowIdx) updated.push(''); + // Allow deselection by clicking same option + updated[rowIdx] = e.target.checked ? col : ''; + setAnswers(prev => ({ ...prev, [itemId]: updated.join('|') })); + }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Student` Teams/TeammateReview.tsx around lines 765 - 772, The radio input uses a no-op onChange and implements toggle logic in onClick, causing React warnings and non-standard "toggle-off" behavior; remove the empty onChange, move the selection logic into onChange (use event.target.checked) in the TeammateReview radio rendering, ensure you set checked based on rowSelections[rowIdx] === col, and update setAnswers(itemId) to set the value only when checked (prevent unselecting a radio by clicking an already-selected radio) so the input behaves as a standard controlled radio using rowSelections, rowIdx, col, itemId, and setAnswers.
380-417:ensureResponseRecordcreates response map on-the-fly, but URLmapIdstate may desync.When a 404 triggers fallback ResponseMap creation (lines 397-405),
setMapId(effectiveMapId)updates local state. However, the URL still contains the old (invalid)map_id. If the user refreshes the page, the stale URL param will cause another 404 cycle.Consider updating the URL query param after creating the new map:
Proposed fix to sync URL with new map_id
effectiveMapId = realMapId; setMapId(effectiveMapId); +// Update URL to reflect the real map id +const newParams = new URLSearchParams(location.search); +newParams.set('map_id', String(effectiveMapId)); +navigate(`${location.pathname}?${newParams.toString()}`, { replace: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Student` Teams/TeammateReview.tsx around lines 380 - 417, The fallback in ensureResponseRecord creates a new ResponseMap and calls setMapId(effectiveMapId) but doesn't update the URL query param, so a page refresh will reuse the stale map_id; after creating the new map (where setMapId(effectiveMapId) is called in the 404 handler inside ensureResponseRecord) update the browser URL to reflect the new map_id (use your routing util — e.g. router.replace / history.replaceState — to replace the current query param map_id with effectiveMapId without adding a history entry) so the URL and component state stay in sync on refresh.
230-268: Draft loading effect lacks error logging for debugging.The
.catch(() => { /* no draft found — that's fine */ })silently swallows all errors, not just 404s. This could hide legitimate issues like network failures or auth problems.Proposed improvement for error handling
- .catch(() => { /* no draft found — that's fine */ }) + .catch((err) => { + // 404 is expected when no draft exists yet + if (err?.response?.status !== 404) { + console.error('Error loading draft response:', err); + } + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Student` Teams/TeammateReview.tsx around lines 230 - 268, The effect that calls axiosClient.get('/responses', ...) currently swallows all errors in .catch(() => { /* no draft found — that's fine */ }), which hides real failures; change the catch to accept an error parameter and only ignore 404/not-found responses, while logging or surface other errors (e.g., console.error or a logger) including the error object for debugging. Specifically, update the useEffect catch handler around axiosClient.get to: (err) => { if (cancelled) return; if (err?.response?.status === 404) { /* no draft */ } else { console.error('Failed to load draft response', err); } } (keeping the existing finally that clears setDraftLoading and honoring cancelled), so useEffect, axiosClient.get, setDraftLoading, and the cancelled guard are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Form/FormSelect.tsx`:
- Line 59: The FormSelect component uses value={field.value ?? ""} making
selects controlled, so fix the parent form initializations: in UserEditor
initialize timezone_pref and language_pref with valid defaults (e.g. "EST" and
"en" or whatever matches your options) instead of leaving them undefined; in
TAEditor set the name initialValue to a valid option from taUsers (not just ""),
and in AssignmentEditor set review_strategy to a valid option value (or -1 if
you have a “no selection” option). Update the initialValues objects in
UserEditor, TAEditor, and AssignmentEditor to provide these concrete defaults so
FormSelect receives valid controlled values.
In `@src/pages/Assignments/AssignmentUtil.ts`:
- Around line 117-126: The update path in AssignmentUtil.ts currently only adds
a quiz object when values.require_quiz and values.selected_quiz_questionnaire
are set, so when a user clears the quiz the existing nested record is omitted
but not deleted; modify the logic around assignmentQuestionnaires (the block
referencing values.require_quiz, values.selected_quiz_questionnaire, and
values.assignment_questionnaire_quiz_id) to explicitly send an object with the
existing quiz record id and _destroy: true when an existing quiz id
(values.assignment_questionnaire_quiz_id) is present but either require_quiz is
false or selected_quiz_questionnaire is empty, otherwise keep the current
add/parse behavior for creating/updating the quiz questionnaire.
- Around line 99-112: The code treats the numeric suffix of keys like
questionnaire_round_1 as a 0-based index; parseInt(match[1], 10) should be
treated as the 1-based round number instead. Change the variable naming and
usages so the parsed value is roundNumber (e.g., const roundNumber =
parseInt(match[1], 10)); use
values[`assignment_questionnaire_id_${roundNumber}`] to read existingId and set
used_in_round: roundNumber (not roundIndex + 1). Apply the same fix to the
similar block referenced at lines 282-289 (the reverse-mapping logic) so keys
and used_in_round remain consistently 1-based.
- Around line 190-193: The request body currently only includes
available_to_students and allow_suggestions so the renamed misc flags are
dropped; update the payload builder in AssignmentUtil (the object containing
available_to_students and allow_suggestions) to also include the new keys
use_bookmark, can_review_same_topic, can_choose_topic_to_review, and
allow_selecting_additional_reviews_after_1st_round by reading them from values
(e.g. use_bookmark: values.use_bookmark ?? false, etc.), and if your form still
uses legacy field names, fall back to those (e.g. values.legacyName ??
values.use_bookmark ?? false) so src/utils/interfaces.ts receives the expected
fields.
In `@src/pages/Assignments/AssignReviewer.tsx`:
- Around line 497-519: The optimistic response_map created locally can remain
with a fake id if axiosClient.post('/response_maps') fails; update the catch
block in AssignReviewer (the try around axiosClient.post and subsequent mutate
call) to rollback the optimistic entry instead of silently keeping it: locate
the mutate usage that updates response_maps and responses (checking for
localMapId and map.id replacement), and on error either remove the local
response_maps entry with id === localMapId (and any responses with map_id ===
localMapId) or set a clear unsynced flag/property on that map so
AssignedReviews.tsx and StudentTasks.tsx can ignore/handle it; ensure you
reference localMapId, realMapId, and realParticipantId when implementing the
rollback/marking so local state remains consistent with backend on failure.
- Around line 586-590: The click handler onCreateQuiz always opens the create
flow; change it to branch on whether the team already has
team.quiz_questionnaire_id: if present navigate to the questionnaire edit route
using that ID (include assignment_id and return_to like the create flow),
otherwise keep the existing /questionnaires/new navigation; update the identical
handler usage around the other block referenced (the one at lines 696-703) so
both create/edit actions use the same branching logic and reference
team.quiz_questionnaire_id and onCreateQuiz (or the handler used there) to
determine whether to open the edit route for the existing questionnaire ID or
the new questionnaire flow.
In `@src/pages/Questionnaires/QuestionnaireEditor.tsx`:
- Around line 112-124: When creating a questionnaire (check mode === "create"
and response.data?.id) the axiosClient.patch call that links the questionnaire
to the team currently only logs failures and still calls navigate, which leaves
the system inconsistent; change the flow so that when
axiosClient.patch(`/teams/${teamId}/quiz_questionnaire`, ...) inside the
try/catch fails you surface the error to the user (e.g., show a user-facing
error/toast or set an error state), avoid redirecting (do not call
navigate(returnTo...)) until linking succeeds, and optionally provide a retry
path; update the block around axiosClient.patch and the surrounding logic that
uses mode, teamId, response.data?.id, navigate, and returnTo to implement this
behavior.
In `@src/pages/Questionnaires/QuestionnaireItemsFieldArray.tsx`:
- Around line 409-489: The item-level field correct_answer is rendered in
QuestionnaireItemsFieldArray but not included in the Yup validation schema in
QuestionnaireForm (see the item schema defined in QuestionnaireForm.tsx), so
quiz items can be saved with blank/invalid answers; update the item schema to
validate correct_answer based on item.question_type (e.g., for "Checkbox"
require boolean/string "true"/"false", for "Scale" require a number between 1
and item.weight, for "Multiple choice" require a non-empty value that exists in
item.alternatives, and for "Text field" require non-empty string), and ensure
the schema path matches items[].correct_answer so ErrorMessage for
name={`items[${index}].correct_answer`} shows errors and prevents submit when
invalid.
- Around line 455-470: The UI renders a single-select for both "Multiple choice"
and "Multiple choice checkbox", preventing multiple selections for checkbox
questions; update the branch so when item.question_type === "Multiple choice
checkbox" you render a multi-select or a list of checkbox inputs instead of a
single-select. Specifically, change the Field rendering for the "Multiple choice
checkbox" case (currently using Field as="select"
name={`items[${index}].correct_answer`}) to either a Field as="select" with the
multiple attribute and ensure items[${index}].correct_answer is treated as an
array in Formik, or render per-option <Field type="checkbox"
name={`items[${index}].correct_answer`} value={opt}> inputs so multiple options
can be selected (use Formik array handling/helpers as needed); keep the
"Multiple choice" single-select behavior unchanged.
In `@src/pages/StudentTasks/AssignedReviews.tsx`:
- Around line 114-118: When the page is opened without a scoped :assignmentId
the useEffect that only calls fetchAssignment when resolvedAssignmentId is set
leaves questionnaires empty; update AssignedReviews.tsx to, when
resolvedAssignmentId is falsy, collect unique assignmentIds from the reviews
list and call fetchAssignment({ url: `/assignments/${assignmentId}`, method:
"GET" }) for each unique review.assignmentId so questionnaire metadata is
populated (apply same logic where reviews are mapped — reference the
fetchAssignment call and the openReview path that expects questionnaire_id).
Ensure you dedupe assignmentIds before fetching and keep fetchAssignment in the
dependency array.
- Around line 179-235: buildRows is computing isTeammateReview from the optional
currentUserTeamId but call sites invoke buildRows(maps) without that argument,
so teammate reviews get misclassified; update every call site (including the
source-of-truth fetch that currently calls buildRows(maps) and the other place
noted around the second call) to pass the caller's current user team id (e.g.,
currentUserTeamId or currentUser.team_id) as the second parameter:
buildRows(maps, currentUserTeamId), ensuring the variable is available in scope
before the call so isTeammateReview logic uses the real team id and the correct
questionnaire and label are chosen.
---
Nitpick comments:
In `@src/pages/Questionnaires/QuestionnaireForm.tsx`:
- Line 180: Extract the inline transformation of itemTypes into a local variable
to improve readability: create a const (e.g., itemTypeNames) that computes
(itemTypes?.data?.map((t: any) => t.name) as string[]) ?? [], then pass
itemTypeNames to the QuestionnaireItemsFieldArray prop itemTypes and keep
questionnaireType as values.questionnaire_type ?? ""; update the component
invocation (QuestionnaireItemsFieldArray) to use the new variable instead of the
inline expression.
In `@src/pages/Student` Teams/TeammateReview.tsx:
- Around line 765-772: The radio input uses a no-op onChange and implements
toggle logic in onClick, causing React warnings and non-standard "toggle-off"
behavior; remove the empty onChange, move the selection logic into onChange (use
event.target.checked) in the TeammateReview radio rendering, ensure you set
checked based on rowSelections[rowIdx] === col, and update setAnswers(itemId) to
set the value only when checked (prevent unselecting a radio by clicking an
already-selected radio) so the input behaves as a standard controlled radio
using rowSelections, rowIdx, col, itemId, and setAnswers.
- Around line 380-417: The fallback in ensureResponseRecord creates a new
ResponseMap and calls setMapId(effectiveMapId) but doesn't update the URL query
param, so a page refresh will reuse the stale map_id; after creating the new map
(where setMapId(effectiveMapId) is called in the 404 handler inside
ensureResponseRecord) update the browser URL to reflect the new map_id (use your
routing util — e.g. router.replace / history.replaceState — to replace the
current query param map_id with effectiveMapId without adding a history entry)
so the URL and component state stay in sync on refresh.
- Around line 230-268: The effect that calls axiosClient.get('/responses', ...)
currently swallows all errors in .catch(() => { /* no draft found — that's fine
*/ }), which hides real failures; change the catch to accept an error parameter
and only ignore 404/not-found responses, while logging or surface other errors
(e.g., console.error or a logger) including the error object for debugging.
Specifically, update the useEffect catch handler around axiosClient.get to:
(err) => { if (cancelled) return; if (err?.response?.status === 404) { /* no
draft */ } else { console.error('Failed to load draft response', err); } }
(keeping the existing finally that clears setDraftLoading and honoring
cancelled), so useEffect, axiosClient.get, setDraftLoading, and the cancelled
guard are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89ae8d37-17e0-4f0a-a0ed-721cb6d524c4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
src/App.tsxsrc/components/Form/FormSelect.tsxsrc/hooks/useSignupSheet.tssrc/pages/Assignments/AssignReviewer.tsxsrc/pages/Assignments/AssignmentEditPage.tsxsrc/pages/Assignments/AssignmentEditor.tsxsrc/pages/Assignments/AssignmentUtil.tssrc/pages/Assignments/tabs/GeneralTab.tsxsrc/pages/Questionnaires/Questionnaire.tsxsrc/pages/Questionnaires/QuestionnaireColumns.tsxsrc/pages/Questionnaires/QuestionnaireEditor.tsxsrc/pages/Questionnaires/QuestionnaireForm.tsxsrc/pages/Questionnaires/QuestionnaireItemsFieldArray.tsxsrc/pages/Questionnaires/QuestionnaireUtils.tsxsrc/pages/Student Teams/TeammateReview.tsxsrc/pages/StudentTasks/AssignedReviews.tsxsrc/pages/StudentTasks/StudentTasks.tsxsrc/utils/interfaces.ts
| {inputGroupPrepend} | ||
| <Form.Select | ||
| {...field} | ||
| value={field.value ?? ""} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all FormSelect usages and check if their parent forms initialize the field in initialValues
# Step 1: Find all files using FormSelect
echo "=== Files using FormSelect ==="
rg -l 'FormSelect' --type=tsx --type=ts -g '!FormSelect.tsx'
# Step 2: Extract FormSelect name props and check for initialValues
echo -e "\n=== Checking FormSelect field names and their initialization ==="
rg -n -A2 '<FormSelect' --type=tsx --type=ts -g '!FormSelect.tsx' | rg 'name='Repository: expertiza/reimplementation-front-end
Length of output: 251
🏁 Script executed:
#!/bin/bash
# Find all FormSelect usages in the codebase
echo "=== Files using FormSelect ==="
rg 'FormSelect' -g '*.tsx' -g '*.ts' -l | grep -v FormSelect.tsx
echo -e "\n=== FormSelect usages with name prop ==="
rg -n '<FormSelect' -g '*.tsx' -g '*.ts' -B2 -A3 | head -100Repository: expertiza/reimplementation-front-end
Length of output: 7441
🏁 Script executed:
#!/bin/bash
# Check initialValues in each file using FormSelect
echo "=== Checking initialValues in UserEditor.tsx ==="
rg -A50 'initialValues' src/pages/Users/UserEditor.tsx | head -80
echo -e "\n=== Checking initialValues in ParticipantEditor.tsx ==="
rg -A50 'initialValues' src/pages/Participants/ParticipantEditor.tsx | head -80
echo -e "\n=== Checking initialValues in CourseEditor.tsx ==="
rg -A50 'initialValues' src/pages/Courses/CourseEditor.tsx | head -80
echo -e "\n=== Checking initialValues in RoleEditor.tsx ==="
rg -A50 'initialValues' src/pages/Roles/RoleEditor.tsx | head -80
echo -e "\n=== Checking initialValues in TAEditor.tsx ==="
rg -A50 'initialValues' src/pages/TA/TAEditor.tsx | head -80
echo -e "\n=== Checking initialValues in AssignmentEditor.tsx ==="
rg -A50 'initialValues' src/pages/Assignments/AssignmentEditor.tsx | head -100Repository: expertiza/reimplementation-front-end
Length of output: 17400
Fix timezone/language preferences and review_strategy initialization in parent forms.
The ?? "" fallback makes the select controlled, which is necessary. However, verification found several FormSelect fields that are improperly initialized:
-
UserEditor.tsx:
timezone_prefandlanguage_prefare missing entirely frominitialValues—these fields will beundefinedand fall back to"", which may not be valid option values. -
TAEditor.tsx: The
namefield is initialized to"", but if this isn't a valid value in thetaUsersoptions list, React will warn about an invalid selection. -
AssignmentEditor.tsx:
review_strategyis initialized to"", which may not exist as an option value.
Add these fields to their respective form initialValues with appropriate default option values rather than relying on the component-level fallback. Examples: timezone_pref: "EST", language_pref: "en", review_strategy: "<default-value>", or use -1 if a "no selection" option exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Form/FormSelect.tsx` at line 59, The FormSelect component uses
value={field.value ?? ""} making selects controlled, so fix the parent form
initializations: in UserEditor initialize timezone_pref and language_pref with
valid defaults (e.g. "EST" and "en" or whatever matches your options) instead of
leaving them undefined; in TAEditor set the name initialValue to a valid option
from taUsers (not just ""), and in AssignmentEditor set review_strategy to a
valid option value (or -1 if you have a “no selection” option). Update the
initialValues objects in UserEditor, TAEditor, and AssignmentEditor to provide
these concrete defaults so FormSelect receives valid controlled values.
| // Scan for all questionnaire_round_* fields dynamically (0-based indexing) | ||
| // This works even if number_of_review_rounds is not set correctly | ||
| Object.keys(values).forEach((key) => { | ||
| const match = key.match(/^questionnaire_round_(\d+)$/); | ||
| if (match) { | ||
| const roundIndex = parseInt(match[1], 10); | ||
| const questionnaireId = values[key]; | ||
| if (questionnaireId) { | ||
| const existingId = values[`assignment_questionnaire_id_${roundIndex}`]; | ||
| assignmentQuestionnaires.push({ | ||
| id: existingId ? parseInt(String(existingId), 10) : undefined, | ||
| questionnaire_id: parseInt(String(questionnaireId), 10), // Ensure number type | ||
| used_in_round: roundIndex + 1, // Convert 0-based index to 1-based round number | ||
| }); |
There was a problem hiding this comment.
The new questionnaire-round mapping is off by one.
AssignmentEditor builds field names as questionnaire_round_1, questionnaire_round_2, ... but this code treats the suffix as a 0-based index. That writes round 1 as used_in_round: 2, and the reverse mapping repopulates edits into questionnaire_round_0. The result is shifted review rubrics on save and an unprefilled first round on reload.
Suggested fix
- const roundIndex = parseInt(match[1], 10);
+ const roundNumber = parseInt(match[1], 10);
const questionnaireId = values[key];
if (questionnaireId) {
- const existingId = values[`assignment_questionnaire_id_${roundIndex}`];
+ const existingId = values[`assignment_questionnaire_id_${roundNumber}`];
assignmentQuestionnaires.push({
id: existingId ? parseInt(String(existingId), 10) : undefined,
questionnaire_id: parseInt(String(questionnaireId), 10),
- used_in_round: roundIndex + 1,
+ used_in_round: roundNumber,
});
}- const roundIndex = aq.used_in_round - 1;
- assignmentValues[`questionnaire_round_${roundIndex}`] = aq.questionnaire_id;
- assignmentValues[`assignment_questionnaire_id_${roundIndex}`] = aq.id;
+ const roundNumber = aq.used_in_round;
+ assignmentValues[`questionnaire_round_${roundNumber}`] = aq.questionnaire_id;
+ assignmentValues[`assignment_questionnaire_id_${roundNumber}`] = aq.id;Also applies to: 282-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Assignments/AssignmentUtil.ts` around lines 99 - 112, The code
treats the numeric suffix of keys like questionnaire_round_1 as a 0-based index;
parseInt(match[1], 10) should be treated as the 1-based round number instead.
Change the variable naming and usages so the parsed value is roundNumber (e.g.,
const roundNumber = parseInt(match[1], 10)); use
values[`assignment_questionnaire_id_${roundNumber}`] to read existingId and set
used_in_round: roundNumber (not roundIndex + 1). Apply the same fix to the
similar block referenced at lines 282-289 (the reverse-mapping logic) so keys
and used_in_round remain consistently 1-based.
| // Add quiz questionnaire if selected and required | ||
| if (values.require_quiz && values.selected_quiz_questionnaire) { | ||
| assignmentQuestionnaires.push({ | ||
| id: values.assignment_questionnaire_quiz_id | ||
| ? parseInt(String(values.assignment_questionnaire_quiz_id), 10) | ||
| : undefined, | ||
| questionnaire_id: parseInt(String(values.selected_quiz_questionnaire), 10) | ||
| // No used_in_round for quiz | ||
| }); | ||
| } |
There was a problem hiding this comment.
Turning quiz off will not remove an existing quiz questionnaire.
On update, if the assignment already has a quiz questionnaire and the user unchecks require_quiz or clears selected_quiz_questionnaire, this code just omits the quiz row from assignment_questionnaires_attributes. Rails will keep the existing nested record unless you send its id with _destroy: true, so the old quiz association survives the edit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Assignments/AssignmentUtil.ts` around lines 117 - 126, The update
path in AssignmentUtil.ts currently only adds a quiz object when
values.require_quiz and values.selected_quiz_questionnaire are set, so when a
user clears the quiz the existing nested record is omitted but not deleted;
modify the logic around assignmentQuestionnaires (the block referencing
values.require_quiz, values.selected_quiz_questionnaire, and
values.assignment_questionnaire_quiz_id) to explicitly send an object with the
existing quiz record id and _destroy: true when an existing quiz id
(values.assignment_questionnaire_quiz_id) is present but either require_quiz is
false or selected_quiz_questionnaire is empty, otherwise keep the current
add/parse behavior for creating/updating the quiz questionnaire.
| // Misc flags from other tabs | ||
| allow_tag_prompts: values.allow_tag_prompts ?? false, | ||
| has_quizzes: values.has_quizzes ?? false, | ||
| calibration_for_training: values.calibration_for_training ?? false, | ||
| available_to_students: values.available_to_students ?? false, | ||
| allow_topic_suggestion_from_students: values.allow_topic_suggestion_from_students ?? false, | ||
| enable_bidding_for_topics: values.enable_bidding_for_topics ?? false, | ||
| enable_bidding_for_reviews: values.enable_bidding_for_reviews ?? false, | ||
| enable_authors_to_review_other_topics: values.enable_authors_to_review_other_topics ?? false, | ||
| allow_reviewer_to_choose_topic_to_review: values.allow_reviewer_to_choose_topic_to_review ?? false, | ||
| allow_participants_to_create_bookmarks: values.allow_participants_to_create_bookmarks ?? false, | ||
| staggered_deadline_assignment: values.staggered_deadline_assignment ?? false, | ||
| allow_suggestions: values.allow_suggestions ?? false, | ||
|
|
There was a problem hiding this comment.
The renamed misc flags are no longer serialized.
src/utils/interfaces.ts now expects use_bookmark, can_review_same_topic, can_choose_topic_to_review, and allow_selecting_additional_reviews_after_1st_round, but this request body only forwards available_to_students and allow_suggestions. Saving an assignment will silently drop those settings unless the old form booleans are mapped to the new request keys here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Assignments/AssignmentUtil.ts` around lines 190 - 193, The request
body currently only includes available_to_students and allow_suggestions so the
renamed misc flags are dropped; update the payload builder in AssignmentUtil
(the object containing available_to_students and allow_suggestions) to also
include the new keys use_bookmark, can_review_same_topic,
can_choose_topic_to_review, and
allow_selecting_additional_reviews_after_1st_round by reading them from values
(e.g. use_bookmark: values.use_bookmark ?? false, etc.), and if your form still
uses legacy field names, fall back to those (e.g. values.legacyName ??
values.use_bookmark ?? false) so src/utils/interfaces.ts receives the expected
fields.
| // Persist the response map to the backend and patch localStorage with the real DB id | ||
| try { | ||
| const res = await axiosClient.post('/response_maps', { | ||
| assignment_id: assignmentId, | ||
| reviewer_user_id: reviewerUserId, | ||
| reviewee_team_id: teamId, | ||
| }); | ||
| const realMapId: number = res.data.id; | ||
| const realParticipantId: number = res.data.reviewer_id; | ||
|
|
||
| if (localMapId !== null) { | ||
| mutate(p => { | ||
| const map = p.response_maps.find(m => m.id === localMapId); | ||
| if (map) { | ||
| map.id = realMapId; | ||
| map.reviewer_id = realParticipantId; | ||
| } | ||
| p.responses.forEach(r => { if (r.map_id === localMapId!) r.map_id = realMapId; }); | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.warn('Failed to persist response map to backend — local ID will be used:', err); | ||
| } |
There was a problem hiding this comment.
Rollback the optimistic map when POST /response_maps fails.
If this request errors, the temporary response_maps entry stays in localStorage with a fake id. AssignedReviews.tsx and StudentTasks.tsx both merge local rows with backend rows, so students can end up seeing review links for maps the backend never created. This needs a rollback or an explicit unsynced state instead of silently keeping the local-only assignment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Assignments/AssignReviewer.tsx` around lines 497 - 519, The
optimistic response_map created locally can remain with a fake id if
axiosClient.post('/response_maps') fails; update the catch block in
AssignReviewer (the try around axiosClient.post and subsequent mutate call) to
rollback the optimistic entry instead of silently keeping it: locate the mutate
usage that updates response_maps and responses (checking for localMapId and
map.id replacement), and on error either remove the local response_maps entry
with id === localMapId (and any responses with map_id === localMapId) or set a
clear unsynced flag/property on that map so AssignedReviews.tsx and
StudentTasks.tsx can ignore/handle it; ensure you reference localMapId,
realMapId, and realParticipantId when implementing the rollback/marking so local
state remains consistent with backend on failure.
| {questionnaireType === "Quiz" && QUIZ_ITEM_TYPES.includes(item.question_type) && ( | ||
| <div className="d-flex gap-1 align-items-center mt-1"> | ||
| <span | ||
| style={{ fontSize: "13px", width: "100px", color: "#555" }} | ||
| className="flex-shrink-0" | ||
| > | ||
| Correct answer | ||
| </span> | ||
|
|
||
| {/* Checkbox: toggle correct/incorrect */} | ||
| {item.question_type === "Checkbox" && ( | ||
| <div className="form-check"> | ||
| <Field | ||
| type="checkbox" | ||
| name={`items[${index}].correct_answer`} | ||
| className="form-check-input" | ||
| id={`correct_answer_${index}`} | ||
| checked={item.correct_answer === "true" || item.correct_answer === true as any} | ||
| onChange={(e: React.ChangeEvent<HTMLInputElement>) => | ||
| setFieldValue(`items[${index}].correct_answer`, String(e.target.checked)) | ||
| } | ||
| /> | ||
| <label | ||
| className="form-check-label" | ||
| htmlFor={`correct_answer_${index}`} | ||
| style={{ fontSize: "13px" }} | ||
| > | ||
| Is correct | ||
| </label> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Scale: numeric value within the scale range */} | ||
| {item.question_type === "Scale" && ( | ||
| <Field | ||
| name={`items[${index}].correct_answer`} | ||
| type="number" | ||
| placeholder={`e.g. 1–${item.weight || 5}`} | ||
| className="form-control" | ||
| min={1} | ||
| max={item.weight || undefined} | ||
| style={{ width: "120px" }} | ||
| /> | ||
| )} | ||
|
|
||
| {/* Multiple choice: select one of the alternatives */} | ||
| {(item.question_type === "Multiple choice" || item.question_type === "Multiple choice checkbox") && ( | ||
| <Field | ||
| as="select" | ||
| name={`items[${index}].correct_answer`} | ||
| className="form-control" | ||
| style={{ width: "220px" }} | ||
| > | ||
| <option value="">— Select correct answer —</option> | ||
| {(item.alternatives || "") | ||
| .split(",") | ||
| .map((opt: string) => opt.trim()) | ||
| .filter((opt: string) => opt !== "") | ||
| .map((opt: string) => ( | ||
| <option key={opt} value={opt}>{opt}</option> | ||
| ))} | ||
| </Field> | ||
| )} | ||
|
|
||
| {/* Text field: plain text input */} | ||
| {item.question_type === "Text field" && ( | ||
| <Field | ||
| name={`items[${index}].correct_answer`} | ||
| placeholder="Correct answer" | ||
| className="form-control" | ||
| style={{ width: "220px" }} | ||
| /> | ||
| )} | ||
|
|
||
| <ErrorMessage | ||
| name={`items[${index}].correct_answer`} | ||
| component="div" | ||
| className="text-danger" | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
correct_answer is rendered, but never validated before save.
The UI shows an inline error slot for items[${index}].correct_answer, but src/pages/Questionnaires/QuestionnaireForm.tsx:21-70 does not include correct_answer in the Yup item schema. Right now quiz items can be saved with blank or invalid answers, which breaks the gating/scoring flow this PR adds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Questionnaires/QuestionnaireItemsFieldArray.tsx` around lines 409 -
489, The item-level field correct_answer is rendered in
QuestionnaireItemsFieldArray but not included in the Yup validation schema in
QuestionnaireForm (see the item schema defined in QuestionnaireForm.tsx), so
quiz items can be saved with blank/invalid answers; update the item schema to
validate correct_answer based on item.question_type (e.g., for "Checkbox"
require boolean/string "true"/"false", for "Scale" require a number between 1
and item.weight, for "Multiple choice" require a non-empty value that exists in
item.alternatives, and for "Text field" require non-empty string), and ensure
the schema path matches items[].correct_answer so ErrorMessage for
name={`items[${index}].correct_answer`} shows errors and prevents submit when
invalid.
| {(item.question_type === "Multiple choice" || item.question_type === "Multiple choice checkbox") && ( | ||
| <Field | ||
| as="select" | ||
| name={`items[${index}].correct_answer`} | ||
| className="form-control" | ||
| style={{ width: "220px" }} | ||
| > | ||
| <option value="">— Select correct answer —</option> | ||
| {(item.alternatives || "") | ||
| .split(",") | ||
| .map((opt: string) => opt.trim()) | ||
| .filter((opt: string) => opt !== "") | ||
| .map((opt: string) => ( | ||
| <option key={opt} value={opt}>{opt}</option> | ||
| ))} | ||
| </Field> |
There was a problem hiding this comment.
Multiple choice checkbox can only store one correct option.
This branch renders a single-select <select>, so quiz authors cannot represent questions with multiple correct answers. Any "Multiple choice checkbox" item with 2+ valid choices will be authored incorrectly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Questionnaires/QuestionnaireItemsFieldArray.tsx` around lines 455 -
470, The UI renders a single-select for both "Multiple choice" and "Multiple
choice checkbox", preventing multiple selections for checkbox questions; update
the branch so when item.question_type === "Multiple choice checkbox" you render
a multi-select or a list of checkbox inputs instead of a single-select.
Specifically, change the Field rendering for the "Multiple choice checkbox" case
(currently using Field as="select" name={`items[${index}].correct_answer`}) to
either a Field as="select" with the multiple attribute and ensure
items[${index}].correct_answer is treated as an array in Formik, or render
per-option <Field type="checkbox" name={`items[${index}].correct_answer`}
value={opt}> inputs so multiple options can be selected (use Formik array
handling/helpers as needed); keep the "Multiple choice" single-select behavior
unchanged.
| {itemType === 'UploadFile' && ( | ||
| <div className="d-flex flex-column gap-2"> | ||
| <Form.Control | ||
| type="file" | ||
| onChange={(e) => { | ||
| const selectedFile = e.target.files && e.target.files.length > 0 ? e.target.files[0] : null; | ||
| setFileSelections(prev => ({ ...prev, [itemId]: selectedFile })); | ||
| }} | ||
| /> | ||
| {fileSelections[itemId] && ( | ||
| <span className="text-muted" style={{ fontSize: 13 }}> | ||
| Selected: {fileSelections[itemId]?.name} | ||
| </span> | ||
| )} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
File upload only stores filename, actual file content is not uploaded.
buildScoresAttributes at line 354 stores fileSelections[itemId]?.name in commentsValue, but the actual File object is never uploaded. The user selects a file, but it won't be persisted to the server.
This needs either:
- A separate file upload endpoint call before/during submission
- Base64 encoding the file content into the payload (not recommended for large files)
- Disabling the UploadFile item type until backend support is implemented
| useEffect(() => { | ||
| if (resolvedAssignmentId) { | ||
| fetchAssignment({ url: `/assignments/${resolvedAssignmentId}`, method: "GET" }); | ||
| } | ||
| }, [resolvedAssignmentId, fetchAssignment]); |
There was a problem hiding this comment.
The unscoped /student_tasks/reviews flow loses questionnaire_id.
When this page is opened without :assignmentId, fetchAssignment never runs, so questionnaires is always empty. Every backend row then falls back to questionnaireId: undefined, and openReview navigates to /response/new without the questionnaire_id that src/pages/Student Teams/TeammateReview.tsx:113-129 expects. The new “Assigned Reviews” page will therefore open reviews with incomplete routing data unless you fetch questionnaire metadata per review.assignmentId or have /response_maps return the resolved questionnaire info directly.
Also applies to: 126-163, 338-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/StudentTasks/AssignedReviews.tsx` around lines 114 - 118, When the
page is opened without a scoped :assignmentId the useEffect that only calls
fetchAssignment when resolvedAssignmentId is set leaves questionnaires empty;
update AssignedReviews.tsx to, when resolvedAssignmentId is falsy, collect
unique assignmentIds from the reviews list and call fetchAssignment({ url:
`/assignments/${assignmentId}`, method: "GET" }) for each unique
review.assignmentId so questionnaire metadata is populated (apply same logic
where reviews are mapped — reference the fetchAssignment call and the openReview
path that expects questionnaire_id). Ensure you dedupe assignmentIds before
fetching and keep fetchAssignment in the dependency array.
| const buildRows = ( | ||
| maps: { | ||
| id: number; | ||
| reviewee_id: number; | ||
| reviewed_object_id?: number; | ||
| assignment_name?: string; | ||
| reviewee_team_id?: number | null; | ||
| team_name?: string; | ||
| latest_response?: any; | ||
| }[], | ||
| currentUserTeamId?: number | ||
| ): AssignedReviewRow[] => | ||
| maps.map((map) => { | ||
| const teamId = Number(map.reviewee_team_id ?? map.reviewee_id); | ||
| const teamName = map.team_name ?? `Team #${teamId}`; | ||
| const latestResponse = map.latest_response; | ||
| const isTeammateReview = | ||
| Boolean(currentUserTeamId) && Number(currentUserTeamId) === Number(teamId); | ||
| const mapAssignmentId = (map as any).reviewed_object_id | ||
| ? Number((map as any).reviewed_object_id) | ||
| : resolvedAssignmentId; | ||
|
|
||
| const selectedQuestionnaire = isTeammateReview | ||
| ? teammateQuestionnaire ?? normalReviewQuestionnaire | ||
| : normalReviewQuestionnaire ?? teammateQuestionnaire; | ||
|
|
||
| let status: AssignedReviewRow["status"] = "Not saved"; | ||
| if (latestResponse) { | ||
| const submitted = | ||
| typeof latestResponse.is_submitted === "boolean" | ||
| ? latestResponse.is_submitted | ||
| : Number(latestResponse.is_submitted) === 1; | ||
| status = submitted ? "Submitted" : "Saved"; | ||
| } | ||
|
|
||
| return { | ||
| mapId: Number(map.id), | ||
| responseId: latestResponse ? Number(latestResponse.id) : undefined, | ||
| teamName, | ||
| revieweeTeamId: | ||
| (map as any)._revieweeTeamId ?? Number(map.reviewee_team_id ?? map.reviewee_id), | ||
| assignmentId: mapAssignmentId, | ||
| assignmentName: (map as any).assignment_name, | ||
| status, | ||
| questionnaireType: isTeammateReview ? "Teammate Review" : "Review", | ||
| questionnaireId: selectedQuestionnaire?.id ? Number(selectedQuestionnaire.id) : undefined, | ||
| questionnaireName: | ||
| selectedQuestionnaire?.name || | ||
| (isTeammateReview ? "Teammate Review Questionnaire" : "Review Questionnaire"), | ||
| // E2619: per-row quiz state from the API — quiz_questionnaire_id and quiz_taken are | ||
| // included per review map so that students reviewing multiple teams are gated per team. | ||
| quizQuestionnaireId: (map as any).quiz_questionnaire_id != null | ||
| ? Number((map as any).quiz_questionnaire_id) | ||
| : undefined, | ||
| quizTaken: Boolean((map as any).quiz_taken), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Backend rows are always treated as normal reviews.
isTeammateReview depends on currentUserTeamId, but the source-of-truth fetch path calls buildRows(maps) without that argument. That means teammate reviews from /response_maps are always labeled "Review" and pick the normal review questionnaire, so the student can be routed into the wrong form.
Also applies to: 313-319
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/StudentTasks/AssignedReviews.tsx` around lines 179 - 235, buildRows
is computing isTeammateReview from the optional currentUserTeamId but call sites
invoke buildRows(maps) without that argument, so teammate reviews get
misclassified; update every call site (including the source-of-truth fetch that
currently calls buildRows(maps) and the other place noted around the second
call) to pass the caller's current user team id (e.g., currentUserTeamId or
currentUser.team_id) as the second parameter: buildRows(maps,
currentUserTeamId), ensuring the variable is available in scope before the call
so isTeammateReview logic uses the real team id and the correct questionnaire
and label are chosen.
| * Merges backend-loaded assignment data with the hardcoded `initialValues` | ||
| * defaults. Any field that is `null` or `undefined` in the backend response | ||
| * falls back to the corresponding default so the form is never partially | ||
| * initialised. |
…front-end # Conflicts: # package-lock.json # src/App.tsx # src/pages/Assignments/AssignmentEditor.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Assignments/AssignmentUtil.ts`:
- Around line 295-299: The current condition allows non-quiz rows with falsy
aq.used_in_round to overwrite quiz restoration values; update the conditional in
AssignmentUtil (the block using qType, aq.used_in_round,
assignmentValues.selected_quiz_questionnaire, and
assignmentValues.assignment_questionnaire_quiz_id) to only run for explicit quiz
questionnaire types (i.e. test qType with /quizquestionnaire|quiz/i) and remove
the permissive "!aq.used_in_round" branch so only quiz-type questionnaires set
selected_quiz_questionnaire and assignment_questionnaire_quiz_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ddb6a338-64ef-4fc2-836d-a88351dd2900
📒 Files selected for processing (3)
src/App.tsxsrc/pages/Assignments/AssignmentEditor.tsxsrc/pages/Assignments/AssignmentUtil.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/App.tsx
| const qType = String(aq.questionnaire?.questionnaire_type || ""); | ||
| if (!aq.used_in_round || /quizquestionnaire|quiz/i.test(qType)) { | ||
| assignmentValues.selected_quiz_questionnaire = aq.questionnaire_id ?? aq.questionnaire?.id; | ||
| assignmentValues.assignment_questionnaire_quiz_id = aq.id; | ||
| } |
There was a problem hiding this comment.
Quiz restoration can be set from non-quiz questionnaire rows.
Line 296 uses !aq.used_in_round || /quiz.../, so any non-round row without used_in_round can overwrite selected_quiz_questionnaire. Restrict this branch to explicit quiz types.
Suggested fix
- } else {
- // Quiz questionnaire has no used_in_round — restore to the quiz select field
- const qType = String(aq.questionnaire?.questionnaire_type || "");
- if (!aq.used_in_round || /quizquestionnaire|quiz/i.test(qType)) {
- assignmentValues.selected_quiz_questionnaire = aq.questionnaire_id ?? aq.questionnaire?.id;
- assignmentValues.assignment_questionnaire_quiz_id = aq.id;
- }
- }
+ } else {
+ const qType = String(aq.questionnaire?.questionnaire_type || "");
+ if (/quizquestionnaire|quiz/i.test(qType)) {
+ assignmentValues.selected_quiz_questionnaire = aq.questionnaire_id ?? aq.questionnaire?.id;
+ assignmentValues.assignment_questionnaire_quiz_id = aq.id;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Assignments/AssignmentUtil.ts` around lines 295 - 299, The current
condition allows non-quiz rows with falsy aq.used_in_round to overwrite quiz
restoration values; update the conditional in AssignmentUtil (the block using
qType, aq.used_in_round, assignmentValues.selected_quiz_questionnaire, and
assignmentValues.assignment_questionnaire_quiz_id) to only run for explicit quiz
questionnaire types (i.e. test qType with /quizquestionnaire|quiz/i) and remove
the permissive "!aq.used_in_round" branch so only quiz-type questionnaires set
selected_quiz_questionnaire and assignment_questionnaire_quiz_id.
Implements the student quiz workflow in the Expertiza React/TypeScript frontend. Students on assignments that require a quiz must now complete the quiz before they can start their peer review. Instructors and submitting teams can author quiz questions with correct answers directly in the questionnaire editor.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Changes