Skip to content

Conversation

@cemreinanc
Copy link
Contributor

@cemreinanc cemreinanc commented Feb 9, 2026

problem: deleting group subquestions changes the dates/bounds of the remaining subquestions
Fixes #3571

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability and consistency in the question form when adding, duplicating, loading, or managing multiple sub-questions by ensuring each sub-question receives a persistent unique identifier.
    • Prevented list rendering and duplication issues during draft load, cloning, and append operations for sub-questions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Adds a stable per-subquestion clientId (via crypto.randomUUID()) and uses it for React keys. Ensures clientId is assigned on initial render, when loading drafts, cloning, or appending sub-questions to maintain consistent identity.

Changes

Cohort / File(s) Summary
Sub-question Identifier Stabilization
front_end/src/app/(main)/questions/components/group_form.tsx
Introduces an optional clientId on draft sub-questions, generates crypto.randomUUID() for initial, cloned, appended, and draft-loaded sub-questions, and replaces index-based React list keys with subQuestion.clientId. Ensures preserved or newly-created clientId when populating from drafts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • lsabor
  • ncarazon

Poem

🐰 I hop and tag each question true,
A tiny UUID for me and you,
No shifting keys, no scrambled dates,
Stable hops across all states—
Hop hop! 🎉

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: introducing a clientId mechanism to fix data drift issues when adding, moving, or deleting subquestions.
Linked Issues check ✅ Passed The PR implements a clientId-based tracking system to fix the data drift problem where deleting subquestions caused remaining ones to inherit deleted data, directly addressing issue #3571.
Out of Scope Changes check ✅ Passed All changes are focused on adding clientId tracking to subquestions and replacing index-based keys with clientId, staying within the scope of fixing issue #3571.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/3571-subquestion-creator-bounds

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
front_end/src/app/(main)/questions/components/group_form.tsx (2)

513-520: Draft backward-compatibility handled well.

The sq.clientId || crypto.randomUUID() fallback ensures older drafts saved without clientId are gracefully upgraded. One minor note: the type cast (sq: QuestionWithForecasts & { clientId?: string }) is loose — the draft's subquestions were originally saved as SubQuestionDraft (via the as unknown as QuestionWithForecasts[] cast on line 547), not true QuestionWithForecasts. This works at runtime but the type isn't quite accurate.


788-797: Pre-existing: in-place mutation of state objects inside map callbacks.

Throughout the subquestion update handlers (here and at lines 813-818, 838-844, 877-883, 915-921, 933-939), the code mutates the existing subQuestion object rather than creating a new one:

subQuestion["label"] = e.target.value;
return subQuestion;

This mutates React state in-place. It works today because setSubQuestions is called with a new array, triggering re-render. However, with stable keys, React may skip re-rendering children whose reference hasn't changed. Consider using immutable updates (spread into a new object) for safety. This is pre-existing and not introduced by this PR, so fine to defer.

Example immutable update pattern
 setSubQuestions(
   subQuestions.map((subQuestion, iter_index) => {
-    if (index === iter_index) {
-      subQuestion["label"] = e.target.value;
-    }
-    return subQuestion;
+    if (index === iter_index) {
+      return { ...subQuestion, label: e.target.value };
+    }
+    return subQuestion;
   })
 );

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2026

🧹 Preview Environment Cleaned Up

The preview environment for this PR has been destroyed.

Resource Status
🌐 Preview App ✅ Deleted
🗄️ PostgreSQL Branch ✅ Deleted
⚡ Redis Database ✅ Deleted
🔧 GitHub Deployments ✅ Removed
📦 Docker Image ⚠️ Retained (auto-cleanup via GHCR policies)

Cleanup triggered by PR close at 2026-02-10T09:27:50Z

@cemreinanc cemreinanc merged commit 799d799 into main Feb 10, 2026
11 of 12 checks passed
@cemreinanc cemreinanc deleted the fix/3571-subquestion-creator-bounds branch February 10, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting group subquestions changes the dates/bounds of the remaining subquestions

4 participants