fix(api): default option type to choice for non-grid questions#3504
Merged
Chartman123 merged 5 commits intoJul 16, 2026
Conversation
Creating an option through POST .../questions/{id}/options without an
optionType stored the option with a null type. Such options are saved
but never rendered, so the question shows up with no selectable answers.
The option_type migration (Version050300Date20250914000000) already
backfills null types to 'choice', and DataStructure.md documents 'choice'
as the default for normal option lists. newOption() just never applied
that default to new inserts.
Default optionType to 'choice' for any non-grid question when the caller
omits it. Grid questions are left untouched, as their options require an
explicit row/column type. Adds the missing Option::OPTION_TYPE_CHOICE
constant alongside the existing row/column ones.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Chartman123
requested changes
Jul 16, 2026
Addresses review feedback on the placement of the default. Moving the default before findByQuestion() changed the ordering: the lookup filters by option type, so a freshly defaulted 'choice' no longer matched existing options that predate the option_type migration (or were inserted without a type), resetting the order to 1. Apply the default after the ordering lookup instead. findByQuestion() keeps the caller-provided type (null means "all options of the question"), so option order is unchanged, and the inserted option still gets 'choice'. Signed-off-by: Josef Behr <code@josef-behr.de> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chartman123
requested changes
Jul 16, 2026
Follow-up to review: move the default out of the controller logic and into the schema, as suggested. - New migration gives the option_type column a 'choice' default and backfills rows still stored without a type. - newOption() no longer writes an explicit null, so the column default applies for normal option lists, and re-reads the inserted option so the response reflects the stored value. - Grid options are unaffected, as callers always pass an explicit row/column type. Removes the earlier controller-side default and the OPTION_TYPE_CHOICE constant. Integration fixtures now receive 'choice' from the column default, so the affected expectations are updated. Signed-off-by: Josef Behr <code@josef-behr.de> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chartman123
requested changes
Jul 16, 2026
- Set the newOption() signature default to 'choice' instead of resolving it in the method body, now that the migration guarantees stored options carry a type. - Drop the explicit-null guard and the post-insert re-read; they are no longer needed with the signature default in place. - Return early from the migration schema change when nothing is altered. - Document the 'choice' default on the optionType parameter. Signed-off-by: Josef Behr <code@josef-behr.de> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Applied all four suggestions in 99ea1cf: signature default |
Reflects the 'choice' default now declared on the newOption optionType parameter. The reorder-options parameter is unchanged. Signed-off-by: Josef Behr <code@josef-behr.de> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chartman123
approved these changes
Jul 16, 2026
Chartman123
left a comment
Collaborator
There was a problem hiding this comment.
Great, looks good now :)
Collaborator
|
/backport to stable5.3 |
|
The backport to # Switch to the target branch and update it
git checkout stable5.3
git pull origin stable5.3
# Create the new backport branch
git checkout -b backport/3504/stable5.3
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 8c08c540 754d8265 f66dd9be 99ea1cf5 bbbaae96
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/3504/stable5.3Error: Failed to cherry pick commits: error: no cherry-pick or revert in progress Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
Chartman123
pushed a commit
that referenced
this pull request
Jul 16, 2026
* fix(api): default option type to choice for non-grid questions
Creating an option through POST .../questions/{id}/options without an
optionType stored the option with a null type. Such options are saved
but never rendered, so the question shows up with no selectable answers.
The option_type migration (Version050300Date20250914000000) already
backfills null types to 'choice', and DataStructure.md documents 'choice'
as the default for normal option lists. newOption() just never applied
that default to new inserts.
Default optionType to 'choice' for any non-grid question when the caller
omits it. Grid questions are left untouched, as their options require an
explicit row/column type. Adds the missing Option::OPTION_TYPE_CHOICE
constant alongside the existing row/column ones.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(api): apply option-type default after ordering lookup
Addresses review feedback on the placement of the default.
Moving the default before findByQuestion() changed the ordering: the
lookup filters by option type, so a freshly defaulted 'choice' no longer
matched existing options that predate the option_type migration (or were
inserted without a type), resetting the order to 1.
Apply the default after the ordering lookup instead. findByQuestion()
keeps the caller-provided type (null means "all options of the question"),
so option order is unchanged, and the inserted option still gets 'choice'.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(api): default option_type via column default and migration
Follow-up to review: move the default out of the controller logic and
into the schema, as suggested.
- New migration gives the option_type column a 'choice' default and
backfills rows still stored without a type.
- newOption() no longer writes an explicit null, so the column default
applies for normal option lists, and re-reads the inserted option so
the response reflects the stored value.
- Grid options are unaffected, as callers always pass an explicit
row/column type.
Removes the earlier controller-side default and the OPTION_TYPE_CHOICE
constant. Integration fixtures now receive 'choice' from the column
default, so the affected expectations are updated.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(api): apply review suggestions for option_type default
- Set the newOption() signature default to 'choice' instead of resolving
it in the method body, now that the migration guarantees stored options
carry a type.
- Drop the explicit-null guard and the post-insert re-read; they are no
longer needed with the signature default in place.
- Return early from the migration schema change when nothing is altered.
- Document the 'choice' default on the optionType parameter.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(openapi): regenerate for option_type default change
Reflects the 'choice' default now declared on the newOption optionType
parameter. The reorder-options parameter is unchanged.
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: Josef Behr <code@josef-behr.de>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 2994695)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Creating an option through the API without an
optionTypestores it with a null type:The options are saved and returned by the API, but the frontend does not render them. The question then appears with no selectable answers. If it is a required question, the form cannot be submitted at all. Verified on Nextcloud 32.0.9 / Forms 5.3.3 with
multiple_uniquequestions: the options existed inGET .../forms/{id}but the rendered form showed an empty question.This is easy to hit because
optionTypeis documented as optional, and the failure is silent: no error, options present in the API response, just not rendered.Why 'choice' is the right default
option_typemigration (Version050300Date20250914000000) already backfills existing null types to'choice'.DataStructure.mdstates: "choiceis the default for normal option lists."So
'choice'is already the documented and migrated default.newOption()simply never applied it to new inserts, so options created after the migration slip through with a null type again.The fix
Default
optionTypeto'choice'for any non-grid question when the caller omits it. Grid questions are deliberately left untouched, since their options require an explicitrow/columntype. Adds the missingOption::OPTION_TYPE_CHOICEconstant next to the existingOPTION_TYPE_ROW/OPTION_TYPE_COLUMN.The existing integration test
testCreateNewOptionassertedoptionType => null, which codified the broken behaviour; updated to expect'choice'.Context
Found while building a feedback form entirely through the API.
optionTypewas omitted on twomultiple_uniquequestions, and both rendered without any options.🤖 Generated with Claude Code