Warn delete options multiple branches#2853
Conversation
thomasiles
force-pushed
the
warn-delete-options-multiple-branches
branch
from
a1516ef to
315e698
Compare
When changing the options in a selection question, we need to remove any conditions which no longer have a valid answer value. We add a method which will only be called when the draft question is saved.
We'll need to check if draft question supports none of the above outside of the input object.
When changing options, we want to show a banner which tells the user if they are removing or changing an option which is in a route. The text for the banner varies depending on how many options are in routes. We refer to options in routes as either the option number or the option's answer value. The bulk options page uses the answer value, the regular options page uses the option number.
thomasiles
force-pushed
the
warn-delete-options-multiple-branches
branch
from
315e698 to
e2c350d
Compare
Add the banner to the normal options page.
Add the banner warning users about options in conditions to the bulk options page.
thomasiles
force-pushed
the
warn-delete-options-multiple-branches
branch
from
e2c350d to
fa29e8b
Compare
| def remove_conditions_without_valid_answer_values(page) | ||
| return unless answer_type == "selection" | ||
|
|
||
| valid_answer_values = answer_settings[:selection_options].map { it[:name] } |
There was a problem hiding this comment.
Shouldn't we be using the :value rather than the :name here?
|
|
||
| valid_answer_values = answer_settings[:selection_options].map { it[:name] } | ||
|
|
||
| if answer_settings[:none_of_the_above_question].present? |
There was a problem hiding this comment.
What if the selection option allows "none of the above" as an answer but doesn't ask for a different answer?
There was a problem hiding this comment.
yes, I don't know how I didn't trigger that when testing. I've also added a guard clause to only change it when it's being treated as a selection option as the other cases are handled elsewhere or in other tickets.
| end | ||
|
|
||
| # option_indexes is either :number or :answer_value | ||
| def selection_options_in_routes_banner(draft_question, selection_options, include_none_of_the_above, option_indexes: :number) |
There was a problem hiding this comment.
I wonder if it would be easier to use this method if if was on the input object rather than a standalone helper... did you consider that?
There was a problem hiding this comment.
I did consider this and it's how I started off. I didn't want to bloat the input object with something so dedicated to the presentation. I'm not super keen on helper methods but it seemed the right place.
I'll happily change it back to the input object if you think thats a better place for it. It would make it easier to call and possibly easier to test too.
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2853.admin.review.forms.service.gov.uk/ 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. |
2 participants
Delete conditions which reference options which don't exist and add banner to options page about routes
Trello card: https://trello.com/c/vaePaWB5/3141-delete-routes-when-changing-selection-options-makes-it-invalid-when-multiple-branches-is-enabled
This PR adds a banner to the selection options page to warn the user that options are being used in routes. The banner changes the wording based on how many options are involved in routes and whether the "none of the above" is activated for the page.
When options are changed or removed, it removes any conditions which have answer_values which no longer have matching options.
Things to consider when reviewing