Bugfix - order conditions in next attribute according to order their fields appear on the page to help avoid page skipping weirdness#199
Draft
wjrm500 wants to merge 1 commit into
Conversation
…fields appear on the page to help avoid page skipping weirdness
wjrm500
force-pushed
the
bugfix/condition-ordering-causing-skipping
branch
from
c97b8c6 to
de6adf8
Compare
|
Comment on lines
+58
to
+65
| save = async (toUpdate, callback = () => {}) => { | ||
| try { | ||
| await this.designerApi.save(this.id, toUpdate); | ||
| const sortedData = sortConditionsBySourceFieldOrder(toUpdate); | ||
|
|
||
| await this.designerApi.save(this.id, sortedData); | ||
| // @ts-ignore | ||
| this.setState({data: toUpdate, updatedAt: new Date().toLocaleTimeString(), error: undefined,}, callback()); | ||
| return toUpdate; | ||
| this.setState({data: sortedData, updatedAt: new Date().toLocaleTimeString(), error: undefined,}, callback()); | ||
| return sortedData; |
Contributor
There was a problem hiding this comment.
v unfamiliar with this codebase sorry. Is the broken behaviour coming from XGovFormBuilder, or from something we've layered on top of it?
If it's broken in XGFB and we're patching a fix in here - should we instead be trying to fix it upstream, get that merged, and then pull it in ourselves?
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.
WIP - Please note that the code and PR description have been AI-generated as a rapid prototype for testing and validation
Description
We encountered a bug where users could skip pages in forms when selecting multiple checkboxes. This happened because the conditions that control page navigation were stored in the wrong order. When a user selected both "feasibility study" and "call for projects" checkboxes, the system would jump directly to the "call for projects" page and skip the "feasibility study" page entirely, because the "call for projects" condition was evaluated first in the list.
Our fix automatically sorts the navigation conditions to match the order of the checkboxes that trigger them. When a form is saved, the system now looks at the order of options in the checkbox field and reorders the corresponding conditions to match. This ensures that if "feasibility study" appears before "call for projects" in the checkbox list, its condition will also be evaluated first, preventing pages from being skipped.
This approach has some limitations. It only works for forms that use checkboxes or radio buttons to control navigation flow. If conditions are based on other types of form fields or complex logic, the sorting may not work as expected. Additionally, the fix assumes that the visual order of options should match the navigation order, which may not always be the desired behavior for all forms.
Testing