Conversation
SamJamCul
force-pushed
the
multiple-branches-questions-page
branch
2 times, most recently
from
ab7d922 to
b8da23b
Compare
This is based on the Group's multiple_branches_enabled feature flag - so if this is toggled off, then it will use the original add and edit page view. When the flag is turned on for the group, the display logic assumes that the form has been created using the new Routes process, and will not be particularly compatible with forms that have been created using the old routing pages (although it might work if the routing is relatively simple). There is no validation displayed for these changes, as we're not ready to start deciding how to validate the new multiple-branch routes.
SamJamCul
force-pushed
the
multiple-branches-questions-page
branch
from
b8da23b to
1562c70
Compare
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2744.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. |
| <% condition_page = self.condition_page(condition) %> | ||
| <% condition_page_position = self.condition_page_position(condition) %> | ||
| <div id="<%= PageListComponent::ErrorSummary::View.error_id(condition.id) %>" class="govuk-summary-list__row app-page-list__row <%= class_names( "govuk-form-group--error": condition.has_routing_errors?) %>"> | ||
| <% if @form.group.multiple_branches_enabled? && page.routing_conditions.present? %> |
There was a problem hiding this comment.
I'd suggest using the feature service to check whether the feature is enabled, rather than looking at the form record directly.
| <% if @form.group.multiple_branches_enabled? && page.routing_conditions.present? %> | |
| <% if FeatureService.new(@form.group).enabled?(:multiple_branches) && page.routing_conditions.present? %> |
Then in the tests whether the flag is enabled or not can be changed by mocking the feature service with the helpers in spec/support/features.rb, removing the need to add a group to the form in each test.
| <div id="<%= PageListComponent::ErrorSummary::View.error_id(condition.id) %>" class="govuk-summary-list__row app-page-list__row <%= class_names( "govuk-form-group--error": condition.has_routing_errors?) %>"> | ||
| <% if @form.group.multiple_branches_enabled? && page.routing_conditions.present? %> | ||
| <div class="govuk-summary-list__row"> | ||
| <dt class="govuk-summary-list__key govuk-summary-list__key app-page-list__key"> |
There was a problem hiding this comment.
| <dt class="govuk-summary-list__key govuk-summary-list__key app-page-list__key"> | |
| <dt class="govuk-summary-list__key app-page-list__key"> |
| end | ||
| end | ||
|
|
||
| def branch_condition_description(condition) |
There was a problem hiding this comment.
What about calling this condition_description2 or something like that? branch_condition_description sounds to me like it's only for branch conditions.
2 participants
This is based on the Group's multiple_branches_enabled feature flag - so if this is toggled off, then it will use the original add and edit page view.
When the flag is turned on for the group, the display logic assumes that the form has been created using the new Routes process, and will not be particularly compatible with forms that have been created using the old routing pages (although it might work if the routing is relatively simple).
There is no validation displayed for these changes, as we're not ready to start deciding how to validate the new multiple-branch routes.
I've removed the edit links for now, we'll need to add them in once we've got the questions listed out on the routes page, as they should be anchored links.
What problem does this pull request solve?
https://trello.com/c/IgUz33Ml/3070-make-edit-questions-list-display-with-new-routes
Things to consider when reviewing