fix(SurveyFormBuilder): use nanoid for new option values and remove duplicate onChange call

[nataliapatrucco]

Summary

Fixes silent deletion failure for newly added answer options in SelectQuestion (SurveyFormBuilder).

Problem

When a user adds options, deletes one, then adds another, the counter-based value generation (new-option-${options.length + 1}) recycles values, causing collisions. This triggers the someOptionsWithSameValue dedup useEffect, which rewrites ALL option values to random nanoid() strings. The delete handler's closure still holds the pre-rewrite value, so the filter options.filter(o => o.value !== params.value) matches nothing — the option silently survives.

Additionally, the dedup useEffect fallback branch called onQuestionChange twice with identical data, causing redundant state updates.

Changes

• handleAddOption: Use nanoid() instead of new-option-${optionsLength + 1} to generate unique option values that won't collide after add-delete-add cycles.
• Dedup useEffect: Remove the duplicate onQuestionChange call in the nanoid fallback branch (lines 74-76 were redundant since line 78 always executed).

The dedup useEffect is kept as a safety net for legacy data that may arrive with duplicate values, but with the nanoid() fix, newly created options will never trigger it.

Related

• Previous f0 PR: fix: prevent answer selection in edit mode for SelectOption #3811 (fixed inverted click guard in SelectOption)
• Factorial PR: https://github.com/factorialco/factorial/pull/94028 (content builder bug fixes)

---

Note

Low Risk
Low risk UI/state fix limited to SelectQuestion, changing how new option IDs are generated and removing a redundant onQuestionChange call.

Overview
Fixes SelectQuestion option value collisions by generating new option values with nanoid() instead of counter-based strings, preventing add/delete/add cycles from reusing IDs.

Also removes a redundant onQuestionChange invocation in the duplicate-value dedup useEffect, avoiding unnecessary duplicate state updates when normalizing option values.

^{Written by Cursor Bugbot for commit 5fff09d. This will update automatically on new commits. Configure here.}

[Copilot]

Pull request overview

Fixes a SurveyFormBuilder SelectQuestion edge case where newly added options could reuse a previously deleted option’s value, leading to collisions and silent deletion failures.

Changes:

• Generate new option values with nanoid() to avoid add/delete/add collisions.
• Remove a redundant duplicate onQuestionChange call in the “dedup values” useEffect fallback path.

[github-actions]

✅ No New Circular Dependencies

No new circular dependencies detected. Current count: 0

[github-actions]

📦 Alpha Package Version Published

Use pnpm i github:factorialco/f0#npm/alpha-pr-3818 to install the package

Use pnpm i github:factorialco/f0#17144f2396653a31494ca31f285c72bd9f3f9139 to install this specific commit

[github-actions]

🔍 Visual review for your branch is published 🔍

Here are the links to:

• the Visual Review Page

• the latest build on chromatic

• the full storybook

[github-actions]

Coverage Report for packages/react

Status	Category	Percentage	Covered / Total
🔵	Lines	45.41%	10940 / 24089
🔵	Statements	44.69%	11277 / 25233
🔵	Functions	37.34%	2468 / 6608
🔵	Branches	37.04%	7090 / 19137

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/react/src/sds/surveys/SurveyFormBuilder/QuestionTypes/SelectQuestion/index.tsx	1.81%	0%	0%	2.12%	26-200

Generated in workflow #12452 for commit a158ca4 by the Vitest Coverage Report Action

[sauldom102]

Nope, we can't do this, in frontend we use new-option- to detect when these options are new and need to be stored on backend

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

nanoid breaks new-option- prefix backend convention

High Severity

Replacing new-option-${optionsLength + 1} with nanoid() in handleAddOption removes the new-option- prefix that the frontend relies on to signal the backend which options are newly created and need to be persisted. As confirmed by a reviewer, this convention is load-bearing — without it, newly added options won't be stored on the backend, causing silent data loss.

 

[cursor]

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[nataliapatrucco]

Updated to use new-option-${nanoid()} which preserves the new-option- prefix for backward compatibility while ensuring uniqueness.

Analysis of the new-option- prefix flow:

I traced the full round-trip through Factorial's helpers.ts:

• createQuestionWithOptions (line 105): maps value: option.id || option.value || '' — f0's value field receives the backend option id for existing options
• getQuestionOptions (line 301): maps back value: option.value.toString() — no id field is returned
• FromQuestionaireToMutationVariables (line 393): nullifyIdIfNew(option.id) checks option.id (from the questionnaire schema), not option.value

After the f0 round-trip, option.id is always undefined (since getQuestionOptions doesn't include it). So shouldNullifyId technically never sees the new-option- prefix. But keeping the prefix is safer for any other potential consumers, so new-option-${nanoid()} is the right approach.

What this PR fixes:

1. Value collision: Old code used new-option-${options.length + 1} — after a delete-then-add cycle, values collide (e.g., delete option 2 of 3, add another → two new-option-3). This triggers the someOptionsWithSameValue dedup useEffect which rewrites ALL option values to nanoid(), making the delete handler's captured value stale → silent deletion failure.
2. Duplicate onChange call: The dedup useEffect had a redundant onQuestionChange call in the fallback branch, removed.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.