Bug: Drupal: Events can be saved with Location type: VA facility, and empty Facility Location

[jilladams]

Description

#17424 found 2 Events that were published with the following location info:

• Location type: At a VA facility
• Facility location: Empty
  

That shouldn't be allowed in the content model. VA Facility should require a Facility Location.

FE consequences

Not proven yet, but it appears that the missing data broke FE templates, potentially, and led to Ui you see in #17424 .

Acceptance Criteria

On an Event node:

• Given Location type is set to At a VA facility, then Facility Location should be required
• Given Location type is set to At a non-VA location, then Street address, City, and State should be required

[dsasser]

PR is up: #17859

[dsasser]

Status update 4/16/24

This work is in PR. I got a review by Christian who noted a very odd browser behavior which is allowing submitting despite having empty, required fields when saving an Event. The behavior which prevents the form from submitting empty, required fields, is native to the browser itself, so this may be challenging to troubleshoot, not to mention unexpected.

[dsasser]

The bug mentioned here is pre-existing for Events, and possibly other forms in the CMS. Initial findings indicate it is a problem with a contributed module: Clientside Validation.

This may be an issue for the Platform CMS team, since that module isn't actively being used by the Event product. However, the opposite case could be made in that the Event product is the only product (known to be) experiencing the issue, and so the resolution should fall to us. @FranECross @jilladams any thoughts on that?

[FranECross]

@dsasser @jilladams I'm leaning toward asking the CMS team to handle it... reasoning is because we aren't actually using it for Events, I think it should fall on them to test to see if other products are actually (silently) having this issue, and then fix it for all product. But... I know they're a new team and don't know what their backlog is. Perhaps Jill can rock/paper/scissors this, or provide a different viewpoint? Thanks!

[jilladams]

That setting is enabled in Prod: https://prod.cms.va.gov/admin/config/user-interface/clientside-validation

I understand from the PR notes:

• With clientside validation ON: we experience the bug ( true in prod)
• With clientside validation OFF: the browser doesn't intervene, and Drupal validation works as expected ?

So: yep, I agree: this is a CMS-wide config that is affecting Events form and probably others, and we should at least talk to them about it. (It may be that we are asked to handle fixing or figuring out next steps, if we have capacity and the negative impact on Events is worrisome for us.) I can spin up a thread.

[jilladams]

https://dsva.slack.com/archives/CT4GZBM8F/p1713396251131369

[dsasser]

Following up on the Clientside Validation problem:

It was decided to let this PR through before fixing the Clientside Validation problem, since the changes here are benign for existing Events, but fixes the issue for new Events.

[jilladams]

@dsasser and it's true I think that our fix won't totally fix everything / this problem could recur until the Clientside validation fix is handled?

[dsasser]

@dsasser and it's true I think that our fix won't totally fix everything / this problem could recur until the Clientside validation fix is handled?

That is correct. Once the Clientside Validation problem is resolved the solution we merge as part of this PR will then begin to function correctly when editing existing Events.

[jilladams]

Noting: this work is done for all intents and purposes, but am keeping the ticket in Complete Pending Integration, to reflect that we will not be able to verify the final behavior until the blocking CMS ticket is complete (#17896). @FranECross @dsasser if you'd rather keep track of that some other way, I'm open to whatever, just let me know.

[dsasser]

Status Update 6/24/24

As previously noted, this work has been merged, but an upstream bug needs to be addressed in order to realize the changes.