fix depends for array builder #33108

rhasselle-oddball · 2024-11-19T19:33:23Z

Summary

Fix depends for array builder and add index property

depends: (formData, index) => ...

Fix forward navigation not respecting newly updated values for all arrays

Related issue(s)

Link to ticket created in va.gov-team repo
department-of-veterans-affairs/va.gov-team-forms#1800
department-of-veterans-affairs/va.gov-team-forms#1888

Testing done

Unit, cypress, regression, browser

Screenshots

n/a

What areas of the site does it impact?

array builder depends.

Acceptance criteria

Quality Assurance & Testing

I fixed|updated|added unit tests and integration tests for each feature (if applicable).
No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
Linting warnings have been addressed
Documentation has been updated (link to documentation *if necessary)
Screenshot of the developed feature is added
Accessibility testing has been performed

Error Handling

Browser console contains no warnings or errors.
Events are being sent to the appropriate logging solution
Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

Did you login to a local build and verify all authenticated routes work as expected with a test user

Requested Feedback

(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

mkerns1

This looks good to me! Thanks so much for adding this in!

src/platform/forms-system/src/js/patterns/array-builder/README.md

src/platform/forms-system/src/js/patterns/array-builder/arrayBuilder.jsx

ToddWebDev

Works well for me!

williamphelps13 · 2024-11-21T17:09:51Z

I switched out the onNavForward and onNavBack conditional page solution in this file, for the depends solution locally and it is working nicely!

The issue I am still encountering is that on edit if the page becomes no longer shown the data is still present. See this in the following video when "wrist fracture" triggers the sideOfBody question but then on edit when "wrist fracture" is updated to "asthma" which does not trigger the sideOfBody the sideOfBody data is still present and "asthma" is displayed as "asthma, right":

Screen.Recording.2024-11-21.at.9.00.34.AM.mov

Could be out of scope for this fix or could be I need to handle it in a different way.

williamphelps13

Just found a potential bug. I had the page that was shown conditionally as a middle page of the loop (that is what the above video demos). With a recent request the conditional page became the last page of the loop. On the summary page I am now not able to click the "no" option and and continue on. The following video shows the conditional page as a middle page first then as the last page:

Screen.Recording.2024-11-21.at.10.13.33.AM.mov

Here is my branch with your changes merged in and the conditional page as the last in the loop (select the "Cherry" demo when in localhost): conditions-639-refine-prototypes

rhasselle-oddball · 2024-11-21T19:52:17Z

Thanks @Midge-dev for helping with the tests on 686

Status update: Working on bug that @williamphelps13 found. Need to fix that before merging.

src/platform/forms-system/src/js/patterns/array-builder/arrayBuilder.jsx

src/applications/disability-benefits/686c-674-v2/config/form.js

rhasselle-oddball · 2024-11-22T17:51:48Z

...simple-forms/mock-simple-forms-patterns/tests/e2e/mock-array-builder-depends.cypress.spec.js

@@ -0,0 +1,289 @@
+import path from 'path';


Added new cypress test in latest commit.

rhasselle-oddball

PR ready for review

rhasselle-oddball · 2024-11-25T16:32:51Z

src/applications/simple-forms/21-0845/tests/e2e/0845-auth-disclose.cypress.spec.js

@@ -42,6 +42,14 @@ const testConfig = createTestConfig(
          });
        });
      },
+      'veteran-identification-information': ({ afterHook }) => {


included this additional unrelated change - because it was blocking PR

williamphelps13

Previous bug: when dependent page is the last page and its the last item added the user could not continue on from the list loop
This is now resolved.

Potential new issue: the recent changes appear to have changed formData to contain all formData for onNavForward when it previously only contained the itemData.

This may be an enhancement *(in the long term) because it seems desirable that formData always contain all formData and never just contain itemData. *Though in this PR it will cause regressions in some forms.

rhasselle-oddball · 2024-11-26T20:25:59Z

Previous bug: when dependent page is the last page and its the last item added the user could not continue on from the list loop This is now resolved.

Potential new issue: the recent changes appear to have changed formData to contain all formData for onNavForward when it previously only contained the itemData.

This may be an enhancement actually because it seems desirable that formData always contain all formData and never just contain itemData.

Appreciate the thorough testing @williamphelps13 - I'll spend some more time on this, to make sure the formData discrepancies don't cause regressions in existing forms.

rhasselle-oddball · 2024-11-27T20:33:01Z

...sentative-portal/accreditation/21a/pages/02-military-service-chapter/militaryServicePages.js

      onNavForward: props => {
-        return !props.formData.currentlyServing
+        const item =


since onNavForward accepts different formData now, I went through the code, and updated all places that were affected

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 19, 2024 20:12 Inactive

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 19, 2024 21:23 Inactive

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch from ebcd01d to fd28800 Compare November 19, 2024 21:33

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 19, 2024 21:59 Inactive

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch from 000397b to 780c3d2 Compare November 20, 2024 16:11

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 20, 2024 16:24 Inactive

rhasselle-oddball marked this pull request as ready for review November 20, 2024 16:46

rhasselle-oddball requested review from a team as code owners November 20, 2024 16:46

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 20, 2024 19:21 Inactive

mkerns1 previously approved these changes Nov 20, 2024

View reviewed changes

Midge-dev previously approved these changes Nov 20, 2024

View reviewed changes

ToddWebDev reviewed Nov 20, 2024

View reviewed changes

src/platform/forms-system/src/js/patterns/array-builder/README.md Show resolved Hide resolved

powellkerry previously approved these changes Nov 20, 2024

View reviewed changes

Midge-dev reviewed Nov 20, 2024

View reviewed changes

src/platform/forms-system/src/js/patterns/array-builder/arrayBuilder.jsx Show resolved Hide resolved

ToddWebDev previously approved these changes Nov 21, 2024

View reviewed changes

williamphelps13 self-requested a review November 21, 2024 18:15

williamphelps13 reviewed Nov 21, 2024

View reviewed changes

Midge-dev mentioned this pull request Nov 21, 2024

Dependents 81671: Remove custom routing #33164

Closed

13 tasks

Midge-dev dismissed stale reviews from ToddWebDev, powellkerry, mkerns1, and themself via 6c64fba November 21, 2024 19:11

Midge-dev requested a review from a team as a code owner November 21, 2024 19:11

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch 2 times, most recently from e87d395 to 504c78f Compare November 22, 2024 17:44

rhasselle-oddball commented Nov 22, 2024

View reviewed changes

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch 2 times, most recently from c236024 to 6f43684 Compare November 25, 2024 15:04

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 25, 2024 15:42 Inactive

rhasselle-oddball commented Nov 25, 2024

View reviewed changes

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch from 6f43684 to 441f2f5 Compare November 25, 2024 23:26

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 26, 2024 01:54 Inactive

Midge-dev previously approved these changes Nov 26, 2024

View reviewed changes

williamphelps13 reviewed Nov 26, 2024

View reviewed changes

rhasselle-oddball and others added 12 commits November 27, 2024 14:27

fix depends for array builder

cb41d47

remove unnecessary code

2a70cee

fix failing tests

47786fa

cleanup

ac605ba

update types

d43b852

fix undefined formData

5e55c81

add try catch wrapper to depends

1c5e3cd

Temp remove custom routing

db05c87

fix last path navigation

b8d1793

rework summary nav forward logic

ec1ec68

new tests + comment 686c-674-v2 conditionals

b65a603

update all affected code + TODOs

52fabce

rhasselle-oddball dismissed Midge-dev’s stale review via 52fabce November 27, 2024 20:28

rhasselle-oddball force-pushed the 1800-fix-depends-array-builder branch from 441f2f5 to 52fabce Compare November 27, 2024 20:28

rhasselle-oddball requested review from a team as code owners November 27, 2024 20:28

rhasselle-oddball commented Nov 27, 2024

View reviewed changes

va-vfs-bot temporarily deployed to master/1800-fix-depends-array-builder/main November 27, 2024 20:40 Inactive

use latest formData

1f8b7a5

va-vfs-bot deployed to master/1800-fix-depends-array-builder/main November 27, 2024 21:14 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix depends for array builder #33108

fix depends for array builder #33108

rhasselle-oddball commented Nov 19, 2024 •

edited

Loading

mkerns1 left a comment

ToddWebDev left a comment

williamphelps13 commented Nov 21, 2024 •

edited

Loading

williamphelps13 left a comment •

edited

Loading

rhasselle-oddball commented Nov 21, 2024

rhasselle-oddball Nov 22, 2024

rhasselle-oddball left a comment

rhasselle-oddball Nov 25, 2024

williamphelps13 left a comment •

edited

Loading

rhasselle-oddball commented Nov 26, 2024

rhasselle-oddball Nov 27, 2024

fix depends for array builder #33108

Are you sure you want to change the base?

fix depends for array builder #33108

Conversation

rhasselle-oddball commented Nov 19, 2024 • edited Loading

Summary

Related issue(s)

Testing done

Screenshots

What areas of the site does it impact?

Acceptance criteria

Quality Assurance & Testing

Error Handling

Authentication

Requested Feedback

mkerns1 left a comment

Choose a reason for hiding this comment

ToddWebDev left a comment

Choose a reason for hiding this comment

williamphelps13 commented Nov 21, 2024 • edited Loading

williamphelps13 left a comment • edited Loading

Choose a reason for hiding this comment

rhasselle-oddball commented Nov 21, 2024

rhasselle-oddball Nov 22, 2024

Choose a reason for hiding this comment

rhasselle-oddball left a comment

Choose a reason for hiding this comment

rhasselle-oddball Nov 25, 2024

Choose a reason for hiding this comment

williamphelps13 left a comment • edited Loading

Choose a reason for hiding this comment

rhasselle-oddball commented Nov 26, 2024

rhasselle-oddball Nov 27, 2024

Choose a reason for hiding this comment

rhasselle-oddball commented Nov 19, 2024 •

edited

Loading

williamphelps13 commented Nov 21, 2024 •

edited

Loading

williamphelps13 left a comment •

edited

Loading

williamphelps13 left a comment •

edited

Loading