department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
98 stars 68 forks source link

Bug: CLP will not save with only Reusable Q&As associated #16575

Closed jilladams closed 9 months ago

jilladams commented 10 months ago

Description

We added Reusable Q&A to CLPs in #10938. However, currently you cannot successfully save a CLP with only Reusable Q&As associated. The CLP form throws an error related to Page-specific Q&As, and 3 of those must be added before the node form will save.

To reproduce:

On Staging / local / Tugboat:

Current behavior: Screen shows an error at form top, "1 error has been found: [Page-Specific Q&A]". No error message is present in the FAQ > Page-Specific Q&A section of the page to indicate what the error is. Form does not save.

Expected behavior: We need to define it, with input from Drupal engineering on what validations are possible / reasonable between Page-specific & Reusable Q&As.

KB Article

https://prod.cms.va.gov/help/campaign-landing-pages/how-to-manage-campaign-landing-pages KB specifies: "FAQs: Add the FAQ segment when you have a minimum of three (max 10) frequently asked questions related to your campaign that will provide answers and additional context for your audience."

We need to update this language to reflect validation changes.

ACs

jilladams commented 10 months ago

@DanielleThierryUSDSVA reported in Slack here: https://dsva.slack.com/archives/C52CL1PKQ/p1703712906383219

Noting: we need to keep her posted on timing for the fix, for a January CLP timeline that will either benefit from fix or need to work around the bug if we can't get it fixed in time.

jilladams commented 9 months ago

Re: these 2 ACs, remove them from ticket body:

The FE creates 2 separate accordions for page-specific vs. reusable Q&As: https://github.com/department-of-veterans-affairs/content-build/blob/main/src/site/layouts/campaign_landing_page.drupal.liquid#L463. So those ACs have been moved to #10940

dsasser commented 9 months ago

Status Update 1/8/23

I discovered that the hidden error message is: "0 Q&A entries created. Minimum of 3 required.". This error exists anytime there are no Page-Specific Q&A's (field field_clp_faq_paragraphs) added to the CLP, and is being thrown by our custom constraint, RequiredParagraph in the va_gov_backend module.

in va_gov_backend_entity_bundle_field_info_alter():

    // Add range check on faq panel.
    if (isset($fields['field_clp_faq_paragraphs'])) {
      $fields['field_clp_faq_paragraphs']->addConstraint('RequiredParagraph', [
        'toggle' => 'field_clp_faq_panel',
        'readable' => 'Q&A',
        'min' => 3,
        'max' => 10,
      ]);
    }

The validation code then requires a minimum of 3 and a max of 10 Page-Specific Q&A's on the CLP.

In order to solve for this, we'll need to replace the use of the RequiredParagraph constraint with a new one programmed to meet our needs. Fortunately Drupal doesn't have too many limitations on how we validate fields, even if they are being validated together.

dsasser commented 9 months ago

Per scrum 16th minute discussion on 1/9/24:

We decided that the verbiage for the error messages should be updated. @thejordanwood provided the following:

dsasser commented 9 months ago

Mid-Sprint Update 1/10/24

This issue is at risk for closing this sprint, unless @Becapa can pick carry it over the line (I'm on PTO for most of the remainder of the sprint).

Where we stand: ✅ The code to fix the problem has has been written. ❌ Tests have not been written ❌ KB Article has not been updated

Tests might be completed by COB today. If so, Michael or whomever can make KB article updates and ensure the PR is tested and merged.

jilladams commented 9 months ago

Reported the mid-sprint risk (here). The d10 patch upgrade takes priority, so this may roll, depending on how that goes.

dsasser commented 9 months ago

Status Udate

I was not able to completely finish the cypress tests today. I've had to various unexpected challenges getting things previously untested to be testable, which has included adding additional cypress commands. I think I'm nearly there with the tests but just finishing up final bugs. All tests have been written, but a couple steps are failing still and I am in the middle of working those out. Here is a screenshot of the error I'm currently debugging:

Screenshot 2024-01-10 at 4 21 21 PM

My current code, should Michael need to pick up where I left off, is in the branch VACMS-16575-unable-to-save-clp-with-only-reusable-q

As I said, I'm currently just debugging the last few (hopefully) cypress test bugs and this should be ready to go after those are resolved.

jilladams commented 9 months ago

From scrum this am: We've asked Michael to pick this up next and see if we can get it wrapped by sprint end, given that we got an extension on the Drupal 10.2 work.

jilladams commented 9 months ago

Noting: this PR merged 6 hours ago. CMS daily deploy theoretically ran after that, but today's daily deploy was aborted bc the git commit sha for main matched what's already in prod, so: maybe this merged after that check? Hard to tell. Should deploy tomorrow and be verifiable in prod.

dsasser commented 9 months ago

Verified in prod:

Screenshot 2024-01-24 at 8 16 08 AM

jilladams commented 9 months ago

Verified with Daniel that he did also confirm: any combo of page-specific / Reusable works as well.