Specify permitted next nodes for all flows #2010
Merged
Conversation
This file contains 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
This allows us to define the permitted next nodes by passing an option to next_node, rather than having to use `permitted_next_nodes`.
|
Looks good to me. Maybe we should run all the regression tests before deploying these changes. |
Move the `widow_pension_amount?` question before `widowed_mother_amount?`. Responses to `multiple_choice` questions are always ordered alphabetically. This flow iterates over the responses to the `:receiving_non_exemption_benefits?` question and uses that to decide which node to display next. The alphabetical ordering of responses means that `widow_pension_amount?` will always be asked before `widowed_mother_amount?`.
I've also removed the unused `response` block argument to avoid the following Rubocop warning: "Unused block argument - response".
I've also removed the unused `response` block argument to avoid the following Rubocop warning: "Unused block argument - response".
We no longer have any graphs that can not be visualised
When calling `next_node` with a block. This isn't foolproof as it only checks that we pass something as the `permitted` option, and not that the permitted options actually make sense. It's probably good enough to prevent people accidentally forgetting to specify the `permitted` option.
This should help ensure that we supply the correct set of permitted next nodes when calling `next_node` with a block.
chrisroos
force-pushed
the
specify-permitted-next-nodes-for-all-flows
branch
from
cee30eb to
8c785e4
Compare
Good shout! This caught some problems in my changes to benefit-cap-calculator. I've fixed the changes and force pushed. |
|
The regression tests have all passed locally so I'm going to get this merged to master! |
chrisroos
added a commit
that referenced
this pull request
Oct 14, 2015
…or-all-flows Specify permitted next nodes for all flows
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.
This supersedes PR #2002.
Trello card - https://trello.com/c/jtjRNOW9/97-update-all-flows-to-use-permitted-next-nodes
Based on feedback from @floehopper, I've added the
permittednamed option toQuestion::Base#next_node.I've updated all flows that use
next_nodewith a block to additionally supply the permitted next nodes.I've updated
Question::Base#next_nodeto raise an exception if it's called with a block but thepermittednext nodes aren't specified.I've updated
Question::Base#next_node_forto raise an exception if thenext_nodeisn't found in the list ofpermitted_next_nodes.NOTE. Using
next_node :symbolornext_node_ifautomatically add the nodes topermitted_next_nodesso I've only updated the flows/questions that usenext_nodewith a block.The advantage of using
permitted_next_nodesis that it allows us to visualise the flows.Expected user-visible changes
Visualisation should now be working for the following Smart Answers.
Example: Visualising additional-commodity-code
Before
After