Editors unable to save changes to Staff Profile pages

[TroyA6]

Describe the defect

Helpdesk has received three separate reports of Staff Profiles not saving changes when 'published' . I have seen this myself on VA Sioux Falls and VA Jackson health care in Production and in Staging - In the Recent Changes list, my changes do not appear -- the third report comes from VA Greater Los Angeles health care.

To Reproduce

Steps to reproduce the behavior:

1. Go to 'https://prod.cms.va.gov/va-jackson-health-care' and log in as log in as Camille.Ballentine@va.gov
2. Navigate to Staff Profiles (Jason C. Cain is the prime example)
3. Make a change, then Publish
4. See error - The change is not saved, and the edit does not appear in the 'Recent Changes' list.

AC / Expected behavior

When a change is made to a Staff profile by an editor, that change should be saved upon Publish and/or Draft

Screenshots

Camille has removed the 'Acting' from the title and the Phone number several times but the changes do not 'stick'

Additional context

Other reports are from

Amber.Bailey-Froke@va.gov at VA Sioux Falls health care Lauren.Bolanos@va.gov at VA Greater Los Angeles health care

Desktop (please complete the following information if relevant, or delete)

• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Labels

(You can delete this section once it's complete)

• [x] Issue type (red) (defaults to "Defect")
• [ ] CMS subsystem (green)
• [ ] CMS practice area (blue)
• [x] CMS workstream (orange) (not needed for bug tickets)
• [ ] CMS-supported product (black)

Team

Please check the team(s) that will do this work.

• [x] CMS Team
• [ ] Public Websites
• [ ] Facilities
• [ ] User support

[jilladams]

Tagged with Facilities and VAMC for visibility in our board to track the fix.

[jilladams]

CMS slack thread: https://dsva.slack.com/archives/CT4GZBM8F/p1687982246103479

[TroyA6]

@jilladams @stefaniefgray @EWashb Update: Resolution still in progress

[edmund-dunn]

@jilladams Please assign this to your developers to fix, ASAP.

[edmund-dunn]

From what I can tell, there was an issue with the validation of this form. It came up during automated testing of some contrib module updates that were pushed in preparation for the D10 upgrade. We thought we had fixed it, but we missed something, apparently.

After discussing with @ndouglas we came to the conclusion that this form never worked correctly.

The business logic needs to be contained in client-side validation.

[jilladams]

@edmund-dunn thanks for the flag -- I chatted with @swirtSJW and @BerniXiongA6 just now about this, and Berni's also had convos with @productmike / Erika about it, so she's gonna follow up and confirm whether we should take this back.

[BerniXiongA6]

Thanks @jilladams -- the next steps is that @EWashb will be meeting with @mmiddaugh tomorrow to discuss whether Facilities or CMS will take this on. In the meantime, please feel free to lean on @productmike and me with any questions you have -- he and I are happy to be helpful Product and Delivery partners! Thanks all.

[EWashb]

Hi @BerniXiongA6. @mmiddaugh is OOO today and I overlooked that when I mentioned I would speak to her! Apologies. I can still speak with her next week, but it seems like the fix CMS proposed (even if approved on the PR) will not make it by our last deploy and cannot be addressed until after the July 4 holiday.

[EWashb]

I also want to ask after reading all related threads: are we still not quite sure "why" this defect is occurring? Is there a way that Facilities can troubleshoot this item since staff profile seems to fall under Facilities product portfolio @jilladams @mmiddaugh? If it is found that the bug is an outcome of a larger CMS issue, I propose we have an interim solution for this issue so that profiles are able to update and then the CMS team can address the larger issue once it is ticketed? @BerniXiongA6 @productmike

[swirtSJW]

Hi @EWashb, I will definitely give a rundown of what broke this when I get it up and running again.

[jilladams]

@kmariepat-cityfriends added to Sprint 87 as unplanned: https://dsva.slack.com/archives/C03LFSPGV16/p1688155320662049

[swirtSJW]

We have a working instance using commit a5138c

Here are all the commits after that working instance » git log --oneline a5138c..HEAD --reverse 0a07167c1 Update cms-team-and-sitewide-crew-member-onboarding.md 2e945ff84 Typo cleanups 332ada49f Update cms-team-and-sitewide-crew-member-onboarding.md e912a1bd7 Replace ddev migrate-sync with ddev composer va:migrate:sync (#13833) 2a821f608 (tag: cms/v0.0.772) VACMS-13817: Replaces knplabs/php-github-api with custom code. (#13826) 01281a8b0 VACMS-0000: Ensure $clientPayload is not NULL. (#13861) 9b65b6625 Bump dependabot/fetch-metadata from 1.4.0 to 1.5.1 (#13871) 2a26d29d2 Fix (I think) some coverage warnings. (#13872) 76bf4c447 Bump github/codeql-action from 2.3.3 to 2.3.5 (#13870) 663a9346b (tag: cms/v0.0.774) Bump cypress-io/github-action from 5.7.2 to 5.8.0 (#13869) ba74203de VACMS-13769 Fix non page facilities urls pushed to lighthouse (#13814) 88252037c Bump drupal/raven from 4.0.15 to 4.0.16 (#13741) 481c54789 (tag: cms/v0.0.773) VACMS-13468: updated copy for vamc notification (#13859) a25bbbdb5 Bump drupal/address from 1.11.0 to 1.12.0 (#13943) 8a59cda46 Bump drupal/menu_item_extras from 2.19.0 to 3.0.2 (#13944) c39e54536 VACMS-11365: Adds GHA workflow to repair PR titles. (#13877) 850d9e047 VACMS-12559: Removes ability to sort on moderation_state on content view. (#13945) 6bd8c265a (tag: cms/v0.0.775) Bump datadog/dd-trace from 0.87.1 to 0.87.2 (#13958) f43e999f3 Set up UX languages changes for TMGMT. (#13894) 8cc33a2e6 Bump va-gov/content-build from 0.0.3261 to 0.0.3276 (#13977) 1936e366d (tag: cms/v0.0.776) VACMS-13362: Adds migration for NCA Facilities API data. (#13858) 516515636 VACMS-13526 Add timezone to Facilities (and hide flags) (#13953) d0e9ee52b VACMS-13815: pdf audit for VHA DM (#13991) 98ae723af Update cms-team-and-sitewide-crew-member-onboarding.md 685700bc7 Update cms-team-and-sitewide-crew-member-onboarding.md 3f3a45bb6 (tag: cms/v0.0.777) Bump mglaman/phpstan-drupal from 1.1.32 to 1.1.34 (#13992) c311e520a Update runbook-vba-facility-closed.md (#13972) 61b4a9869 Update runbook-vba-facility-name-change.md (#13973) 3eb5617d3 Update runbook-nca-facility-name-change.md (#13971) 7f2234955 VACMS-13672: Adds system banner image (#13988) e3b480cf8 Update cms-team-and-sitewide-crew-member-onboarding.md efa5d407d (tag: cms/v0.0.778) VACMS-0000: Fixes issue identified by Sentry (#14007) 2b61d3beb Update runbook-vba-facility-new.md (#13974) 6756aced6 Update cms-team-and-sitewide-crew-member-onboarding.md e2263ad14 (timcosgrove/main) Update cms-team-and-sitewide-crew-member-onboarding.md d639ea3f9 (tag: cms/v0.0.779) Bump drupal/allow_only_one from 1.0.1 to 1.0.2 (#14003) d722b0506 VACMS-13583: Unable to alter field values by Views Bulk Operation (#14039) 942e23a5c (tag: cms/v0.0.780) VACMS-13806: hide counters on text fields when editing translations. (#14043) 2ac53a0e5 VACMS-13649: Multi-time pattern passthrough (#14035) 6ce49268d [DOCS] Update vamc-facilities.md to move full width banner content. 861c03d45 Bump github/codeql-action from 2.3.5 to 2.13.4 (#14053) 231cb3768 Bump phpstan/phpstan from 1.10.15 to 1.10.18 (#14036) e228ec7a0 [DOC] Add links to content-model-centralized-content 75db185a6 Bump cypress-io/github-action from 5.8.0 to 5.8.1 (#13997) eae9490e6 Bump int128/datadog-actions-metrics from 1.48.0 to 1.49.0 (#13996) c3be547cc (tag: cms/v0.0.781) Bump aws-actions/configure-aws-credentials from 2.0.0 to 2.1.0 (#13994) 63782aeda [docs] Add govdelivery diagram to vamc-facilities 2566d4fb8 (tag: cms/v0.0.782) VACMS-13956: Updates office_hours, removes patch (#14058) 7b63ff575 Update runbook-vet-center-new.md (#14065) e4f1e39d6 VACMS-13806: defect, method missing (#14077) dee19e5d8 [docs] Update cms-team-and-sitewide-crew-member-onboarding.md 1e2686cab Bump drupal/blazy from 2.15.0 to 2.16.0 (#14062) 649e75dc5 (tag: cms/v0.0.783) Bump aws/aws-sdk-php from 3.271.0 to 3.273.0 (#14082) 890a94f43 Bump drupal/path_redirect_import from 2.0.6 to 2.0.7 (#14091) 45972d880 Create cms-design-system-dev-implementation.md 3723218ed Update cms-design-system-update-or-addition.md dac8b13e0 Update cms-design-system-update-or-addition.md cc364b560 Update cms-design-system-update-or-addition.md b6d5ad0e6 Update cms-design-system-dev-implementation.md d581f572f Bump mglaman/phpstan-drupal from 1.1.34 to 1.1.35 (#14096) 4ec580497 Bump datadog/dd-trace from 0.87.2 to 0.88.0 (#14097) 8397cfc8e Bump drupal/office_hours from 1.9.0 to 1.11.0 (#14098) b7eccbe60 [docs] Update cms-team-and-sitewide-crew-member-onboarding.md 9b6bd5df4 (tag: cms/v0.0.784) VACMS-13948: Record continuous delivery metrics for CMS. (#14089) 7f1972829 VACMS-0000: PR template small typo (#14105) 8a08d4262 (tag: cms/v0.0.785) Bump drupal/devel from 5.1.1 to 5.1.2 (#14107) 5771756f1 (dsasser/main) [docs] Update cms-team-and-sitewide-crew-member-onboarding.md ec8cf64c7 Bump va-gov/content-build from 0.0.3276 to 0.0.3288 (#14120) b6cca84f5 (tag: cms/v0.0.786) VACMS-000 upgrade migration tools module (#14122) c8ae143dc Bump cypress-io/github-action from 5.8.1 to 5.8.3 (#14111) 9ab040a53 (nate/main) Bump actions/checkout from 3.5.2 to 3.5.3 (#14110) 055b5325e Update runbook-vet-center-closed.md (#14068) 7d5150093 Bump datadog/dd-trace from 0.88.0 to 0.88.1 (#14132) ae3b925a7 Bump openssl from 0.10.52 to 0.10.55 in /tests/loadtest (#14131) e4eb78311 Bump aws-actions/configure-aws-credentials from 2.1.0 to 2.2.0 (#14109) 995d56e1d VACMS-13478: contrib module updates for d10 readiness (#14026) a91212747 VACMS-13592: Update Vet Center editor login redirect for outdated content (#14130) d3a53267c (tag: cms/v0.0.787) VACMS-10368: Uses WCAG 2.1A and 2.1AA standards. (#14133) 72d3e8bb5 VACMS-0000: A small change to the CD metrics script. (#14135) f6e438630 [DOC] Update runbook-nca-facility-closed.md to account for auto archiving. d7869c161 VACMS-00000: fix for patch on sitewide_alert (#14139) 0e1aa5956 (tag: cms/v0.0.788) VACMS-00000: update form simplesamlphp library (#14145) 14b39b87d [docs] Update runbook-vet-center-cap-to-outstation.md (#14067) 43e404d70 Update runbook-nca-facility-closed.md (#13970) bbb7e0e80 Create deploy-oob.md e197dceb6 Bump drupal/geofield_map from 3.0.8 to 3.0.10 (#14144) 608b7f4ae VACMS-13768: Updates method name after namespace change (#14102) 972266db1 Bump reviewdog/action-eslint from 1.18.2 to 1.19.1 (#14156) 7b71df201 Bump reviewdog/action-stylelint from 1.16.0 to 1.17.1 (#14155) 692db759d Bump drupal/libraries from 4.0.3 to 4.0.4 (#14154) f44770698 (tag: cms/v0.0.789) VACMS-13845: Creates va_gov_environment module and Environment Discovery service. (

14157)

2d64b1115 (tag: cms/v0.0.790) VACMS-14148: fix for facilites migration, patch migrate_plus (#14159) 4cbae9160 [docs] Update cms-team-and-sitewide-crew-member-onboarding.md a6ce60af3 [docs] Update cms-team-and-sitewide-crew-member-onboarding.md ee7c20cec Bump drupal/image_style_warmer from 1.2.0-rc2 to 1.2.0 (#14168) c18721c9d VACMS-13847: Create ContentReleaseStrategy plugin system. (#14160) 0554436c4 Bump va-gov/content-build from 0.0.3288 to 0.0.3292 (#14186) 34a489521 (tag: cms/v0.0.791) VACMS-00000: disable flysystem modules (#14193) a89168aa6 VACMS-11921: Deprecate old homepage cms elements (#14192) a2d241d9b Bump phpstan/phpstan from 1.10.19 to 1.10.21 (#14208) 9faf423e1 VACMS-0000: Allow connection reuse in ExistingSiteBase tests. (#14204) 552c77cfe (tag: cms/v0.0.792) VACMS-13848: Creates strategyresolver service. (#14191) c434714ce Bump mglaman/phpstan-drupal from 1.1.35 to 1.1.36 (#14227) f5334895c VACMS-0000: Enhancements to OOB deploy documentation. (#14151) a88d99846 VACMS-13849: Create form resolver service. (#14212) 2a6d8915b (tag: cms/v0.0.793) VACMS-0000: Enable syntax highlighting for .module and .install files (#14229) a4699d407 VACMS-00000: remove drupal/flysystem modules (#14230) 8d89c8bde VACMS-0000: Dependabot alerts. (#14228) 9b21b61b8 VACMS 9839: Debug and reënable failing PHPUnit test. (#9985) bf0f33a57 Create cms-design-system-documentation.md e560e90c2 Update cms-design-system-update-or-addition.md 68cdc6435 Update cms-design-system-documentation.md ef6f6efd2 VACMS-14032: Autoarchive Outstation and Mobiles and clears data (#14211) 62a37face [DOCS] Update runbook-vet-center-closed.md cfbcc6d31 (upstream/main, main) Bump drupal/geofield_map from 3.0.10 to 3.0.11 (#14234)

[swirtSJW]

Confirmed that I as admin can use and unuse the profile page checkbox as it is designed https://main-1ouqufuiwhqohaqnvgjjoaidpjpensj4.demo.cms.va.gov/node/56774/revisions

Testing as RyanS in Lebannon also works as expected

Conclusion: This was working as intended as of this commit a5138c from a month ago.

[swirtSJW]

I also want to ask after reading all related threads: are we still not quite sure "why" this defect is occurring? Is there a way that Facilities can troubleshoot this item since staff profile seems to fall under Facilities product portfolio

@EWashb In terms of root cause. There were several colliding factors: 1) The code was a bit fragile to begin with and took advantage of loopholes in drupal's handling of conditional requiredness that were also at odds with how browsers enforce html5 validation (validation that can't be controlled by js or drupal, it is baked into the browser). 2) Only a wee bit of test coverage and none that specifically targetted the conditionally required fields. 3) This D10 contrib readiness PR removed a lot of the related code and the QA steps only accounted for testing 2 of the possible combinations on that person_profile content type

The new implementation is more robust in terms of proper handling of conditionally required fields and in terms of automated testing the possible points of failure

Takeaways:

• More complete QA steps to hunt for points of failure.
• Less use of PRs that try to update many things all at once. (one pr per D10 module fix) It makes it a lot easier to define the QA steps and spot regressions.
• Better communication across teams when updates are being made to product team related code.