Show none of the above question on a separate page for long list selection questions#1807
Merged
stephencdaly merged 12 commits intomainfrom Jan 8, 2026
Merged
Conversation
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
8 times, most recently
from
a6b38a2 to
c934cd2
Compare
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
from
c934cd2 to
aa23938
Compare
lfdebrux
reviewed
Jan 6, 2026
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
from
ea73e80 to
0fe7256
Compare
stephencdaly
commented
Jan 6, 2026
For a selection question with more than 30 options where we display an autocomplete component, we'll want to show the extra question if 'None of the above' is selected on a new page. In the case we show a question when none of the above is selected, we don't want users to be able to go to the next question in the form until it is answered. Override the `answered?` method to return false if there is a 'None of the above' question configured and it hasn't been answered.
The result of this was never checked. We validate the question when we call `Flow::Context#save_step` when submitting question pages.
For selection questions that use the autocomplete component, we want to validate differently when submitting the "None of the above" question page. Allow passing in a context that is passed to `Question::QuestionBase::valid?` so we can apply different validation.
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
from
0fe7256 to
44e685d
Compare
When an autocomplete component is shown for a selection question when it has more than 30 options, we show the question for when "None of the above" is selected on a different page. To account for this, we do not validate the presence of the none_of_the_above_answer on the selection page if the question is an autocomplete question. Allow passing in a validation_context to the validation method that will skip the validation of the none_of_the_above_answer. We want to validate the none_of_the_above_answer in the default context in case we have a future use for loading in questions and validating, rather than just the current case where we validate the model when a question is answered.
Rather than using a :with_selection_settings trait, add a dedicated factories for steps returned from forms-admin that are for selection questions. This allows us to add an additional trait for when the step also has a none of the above question configured.
We weren't showing the back link when rendering validation errors in response to the POST request to submit a question. Move setting the back_link to the common method for setting up the instance variables and make it more uniform with how we set the other instance variables. Also improve the readability of the method while we're here.
Add a separate /none-of-the-above route for asking the question when "None of the above" is selected for a selection question that uses the autocomplete component. Use the same `Question::Selection` model for the form on this page. We need to set the `selection` for the model to have "None of the above" selected as this won't be loaded automatically from the answer_store as `answered?` returns false for the question. We know what the selection is though, so rather than change the step loading logic, we can just pre-set. When the step is saved, pass in the context `none_of_the_above_page` so that the none_of_the_above answer is validated correctly.
This method makes more sense to be an instance method of Step, move it to de-clutter the controller.
When the page for a selection question is submitted, and it is an autocomplete component with "None of the above" selected, redirect to the dedicated "None of the above" question page. Do not call LogEventService#log_page_save in this case as we don't want 2 save events for the same question, and we also only want to send the "Started" metric to CloudWatch once for this form session if this is the first question in the form. Pass in the `skip_none_of_the_above_question_validation` validation context when saving the step so that we don't validate the presence of a value for the none of the above answer on the selection page.
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
from
44e685d to
98e664d
Compare
lfdebrux
reviewed
Jan 8, 2026
lfdebrux
previously approved these changes
Jan 8, 2026
Contributor
lfdebrux
left a comment
lfdebrux
left a comment
There was a problem hiding this comment.
Looks good to me! Thanks for doing the refactoring at the same time. It might be worth in future breaking some of the refactors out into separate PRs to be merged first, as this was a bit of a large PR to review.
For 'None of the above' questions that are shown on a separate page when an autocomplete component is shown for the selection question, link to the page to change the 'None of the above' question answer for the change link on the 'Check your answers' page.
Bang methods usually indicate that they perform some sort of "dangerous" operation, such as modifying the object in place or raising an exception on failure. In this case, the `update!` method is updating attributes on a question object, but not saving it anywhere so the bang seems unnecessary and misleading. The method name `update` didn't make much sense either, as "update" would indicate that we are saving the object. Rename to `assign_question_attributes` to better reflect the behaviour.
stephencdaly
force-pushed
the
show-none-of-the-above-question-for-autocomplete
branch
from
98e664d to
736eeb6
Compare
|
lfdebrux
approved these changes
Jan 8, 2026
Contributor
|
🎉 A review copy of this PR has been deployed! It is made of up two components Important Not all of the functionality of forms-runner is present in review apps. You should use the full dev environment to test the functionality which is disabled here. It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
stephencdaly
deleted the
show-none-of-the-above-question-for-autocomplete
branch
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
2 participants
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.
What problem does this pull request solve?
Trello card: https://trello.com/c/bfFa7B13/
Things to consider when reviewing