jilladams commented 1 month ago

Description

A campaign landing page has a draft change in the first promo block under "What you can do" did roll out to production last night despite being saved as draft: https://www.va.gov/initiatives/prepare-for-vas-secure-sign-in-changes/

Technical notes

CLP promos use the Custom block type: Promo. Blocks don't have an editorial workflow.

Acceptance criteria

[ ] CLPs in the front-end only display published content
[ ] We understand the mechanism by which this occurred, and comment with an assessment of what else may need to be reviewed

omahane commented 1 month ago

@jilladams From what I understand of this comment, it was determined that we don't have a Drupal-sourced issue at play. I'm I reading this wrong?

If that's the case, I'm not sure what I'm supposed to be looking at.

omahane commented 1 month ago

I found this conversation and see the issue: https://dsva.slack.com/archives/C07SFHRM745/p1729352044450959

omahane commented 1 month ago

Based on conversation with @edmund-dunn and my own analysis, we've determined that the inconsistencies with publishing appear to be related to entity reference fields and the way they are referenced.

jilladams commented 1 month ago

Noting: these promos use Blocks, which do not have an Editorial workflow, and can't be saved independently as Draft.

https://dsva.slack.com/archives/C07SFHRM745/p1729550800092079?thread_ts=1729287978.261209&cid=C07SFHRM745

Campaign Landing Pages > What you can do section: What you can do promos: This section of the CLP form is using Blocks, which do not have an Editorial workflow. So for example:

https://prod.cms.va.gov/node/63686/edit - this contains 3 promos that were created from within the context of the CLP form
You can also find those promos / prove they are blocks by looking at them here:

When you are creating a new CLP with new promos this probably doesn't matter, but once a CLP is live, blocks don't provide a way to do drafts. So even though you might edit a block-in-the-context-of-a-CLP, and save the CLP as a Draft: Blocks themselves don't have a draft state. I dunno how that works in terms of publishing (are blocks Published by default?)

jilladams commented 1 month ago

First: I am wrong. Blocks do have editorial workflow and revisions. I swear they didn't used to -- that may be new in latest Drupal core? In any case it exists now.

What had happened

The case study here:

From within the CLP: CAIA created a draft edit to the Promo block, and saved it as draft (both CLP / block). However: the content they added, ended up in the FE. It's still present in production.

Observations from testing

On the CLP node form, you can create a block. You can also see the revision state of the block. From within the node edit form on CLP, the moderation state seems to always say Published, regardless of the actual state of the block. We think it's reporting on the default revision, not the latest revision. (So it probably won't show you that there's a latest revision in draft, if that exists.)

If you're editing a block from within the CLP node edit for (as IEF): Changes on a block (including status) won't save to the block until you save the parent CLP node. The revision log from the parent CLP node does not get saved as the revision log on the Block itself, but a new block revision is created (just without the message).

You can preserve the draft status on a block, when you publish the CLP where it lives. In other words: publishing a CLP does not auto-publish Draft revisions on a promo block.

The graphQL query for CLPs / promos within CLPs is here: https://github.com/department-of-veterans-affairs/content-build/blob/main/src/site/stages/build/drupal/graphql/nodeCampaignLandingPage.graphql.js. We think: the next step is that FE needs to adjust this query to pull only the default revision for a block.

jilladams commented 1 month ago

We may also need to update the CLP KB related to promos.

don't set promos as published until you're read for the changes to go to the front-end
You will have to separately adjust the state of the Promo block (draft/published)

omahane commented 1 month ago

@edmund-dunn I have tested the configuration changes you made and have captured the test results in this spreadsheet. I think you're on the right path. Blocks in draft don't show on the front end. The node:edit of the block is showing the default revision, which actually may be right or may be wrong. It's a toss up. I can see how as an editor, if I haven't saved the block as published then maybe I shouldn't see the changed on the node:edit of the CLP on the premise that what is displayed is the default revision and that to see the latest requires clicking "Edit". That would be a change to note and communicate to the editors, though.

CLP node:edit

Entity browser form for Promo block, accessed via CLP node:edit

jilladams commented 1 month ago

The folks who create CLPs are mostly digital comms folks from other VA bureaus, and sometimes CAIA, so they're not Drupal pro users. If we change the behavior for previewing / seeing Draft changes, we need to update the related CLPs (and probably need technical notes on what exactly changed). Imo it's not a dealbreaker if it solves this, but It's ultimately a product call for @FranECross / @mmiddaugh .

If we were to say that this Node View showing Latest Revision (instead of draft revisions) is ok: would Edmund's solution be ready to PR?

jilladams commented 3 weeks ago

@edmund-dunn Christian noted that you were checking on your change with CAIA and may have news. We are tracking this ticket in our sprint, and wondering if it should be reassigned to you / tracked in the CMS board, if you have a fix in the works?

cc @gracekretschmer-metrostar

edmund-dunn commented 3 weeks ago

@jilladams I will be following up with them today.

edmund-dunn commented 3 weeks ago

@jilladams I spoke with Randi Hecht and showed her the changes. She thinks they are good to go. Also ran them past @MDomngz who approved of them from a UX perspective.

@omahane the necessary patches and changes to composer.json are pushed in your working banch.

@gracekretschmer-metrostar CC

omahane commented 3 weeks ago

I'm testing this locally. Should I not see any issues, I'll put up a PR.

omahane commented 2 weeks ago

@laflannery @davidmpickett @thejordanwood We are using a different widget to solve this problem, moving from Inline Entity Form to Entity Browser. @edmund-dunn has patched the module to allow us to show the latest revision in the form widget. He has reviewed the solution with UX from his team, but I wanted to run this by you to make sure you don't see any issue. I'm doing this now, as opposed to when the PR is ready to review because the UX changes are breaking Cypress. If I make those changes, but it turns out we wanted a different solution to this anyway, it's going to mean rewriting the Cypress tests again.

Here's a page to test: https://pr19640-4ikhsvndhlpcaaab3uhii2cuoabehwrm.ci.cms.va.gov/node/39911/edit

Go to What you can do
Click to expand it
Note the differences from production
Click the Place promo button
Note the use of a tabbed interface to either add an existing or make a new promo

You can also create a new CLP, of course.

laflannery commented 2 weeks ago

I took a look and while there are definitely accessibility issues here, the majority are not new to Drupal, they are existing within the overall product already (e..g. focus management, missing table headers, etc.). There is only 1 that is specific to this situation that we should address here before shipping:

The "show row weights" button is missing so the only way I have to reorder the promos is by dragging and dropping.

omahane commented 1 week ago

@edmund-dunn I am looking at this and have found the JS that is breaking the modal for non-admins, but I don't understand why/how. What's weird is that Entity Browser uses 2 custom AJAX callbacks. One is working but the other isn't.

Drupal.AjaxCommands.prototype.select_entities in entity_browser/js/entity_browser.modal.js: ❌ not working
Drupal.AjaxCommands.prototype.entity_browser_value_updated in entity_browser/js/entity_browser.common.js: ✅ working

Because the entity_browser.modal.js is not working, it is causing the improperly sized modal and the inability to place an existing promo block:

They both appear to be called only when the page is loaded by a non-admin. But in the case of the broken one, the AjaxCommands object doesn't appear to exist:

While I've found the offending JS, I don't know why it's offending.

omahane commented 1 week ago

The other remaining issue is the "Place Block" button, which does not show when the 3rd block has been removed. It only shows after the node has been saved and is reloaded.

omahane commented 1 week ago

Proposed fix for the breaking AJAX

When I added to the contrib module the AJAX library in the dependencies, Drupal.AjaxCommands.prototype.select_entities in entity_browser/js/entity_browser.modal.js is ✅ working. Admins don't see any regressions, either.

I changed this file:

/contrib/entity_browser/entity_browser.libraries.yml
Adding - core/drupal.ajax on line 65

Now, lines 58 - 69 are as follows:

modal:
css:
component:
  css/entity_browser.modal.css: {}
js:
js/entity_browser.modal.js: {}
dependencies:
- core/drupal.ajax
- core/drupalSettings
- core/drupal
- core/once
- core/jquery

@edmund-dunn This will need to be added to the patch. (But could you hold off until we have a solution to the following issue?)

Place Promo button does not show after 3rd promo block is removed

When loading a CLP with 3 promo blocks in What You Can Do, removing one does not show the Place Promo button.
You have to save and reload the edit form to see the Place Promo
This is a regression from the IEF

omahane commented 1 week ago

Continuing saga of the remove button

On prod

We have Events on Campaign landing pages using Entity Browser.

Cardinality: 2
After adding 2 events, the "Select Event" button goes away.
When we remove an event, the button reappears.

On PR env

We have Events on Campaign landing pages using Entity Browser.

Cardinality: 2
After adding 2 events, the "Select Event" button goes away.
When we remove an event, the button is still absent.

I have a video that shows this: https://www.loom.com/share/78d0572df9034d5293a0df5b47bf1bda?sid=b14b270f-5a25-47ce-bf1a-815976081618

Conclusion

I believe that the patch on https://www.drupal.org/project/entity_browser/issues/2851580 is breaking the expected remove functionality. I'm testing this hypothesis now.

omahane commented 1 week ago

Remove button, part deux

It turns out that without the following:

"drupal/entity_browser": {
    "2851580 - Re-order + remove broken with the Entity Reference (and File) widget": "patches/2851580-fix-remove-button-entity-browser.patch",
},
    "drupal/entity_browser_table": {
    "3395042 - Re-order + remove broken with the Entity Reference (and Table) widget": "patches/3395042-fix-remove-button-entity-browser-table.patch"
}

then the patch for the latest revision:

"drupal/entity_browser": {
    "3483265 - Make it possible to show latest revision in form widget": "patches/3483265-make-it-possible-to-show-latest-revision-in-fomr-widget.patch"
}

doesn't apply.

omahane commented 1 week ago

Row weight issue

From @laflannery via Slack

Everything worked fine for me, there are accessibility issues but only 1 that is not already existing within the greater Drupal situation that we should address here before shipping: The "show row weights" button is missing so the only way I have to reorder the promos is by dragging and dropping.

omahane commented 3 days ago

Remove block III: This time it's personal

I figured out the remove block failure on Promos.

/va_gov_workflow/src/EventSubscriber/EntityEventSubscriber.php

getIefTypeFields() has a call to isNodeIef(), which bypasses Revision log validation on IEFs that target nodes but not content blocks.
So I'm making a very minor change to make it true for blocks, too. (May need some biggish testing, though).

Following the data

In the Promos area, using Inline Entity Browser, when clicking the Remove button, this is the result in the browser's developer tools network tab. xhr-call-events-remove-edit.json

That last bit gets parsed to:

<div class="item-list--comma-list item-list">
  <ul class="item-list__comma-list">
    <li>
      <a href="#edit-revision-log-0-value--_b5kQ3IIxkM">Revision log message</a>
    </li>
  </ul>
</div>

Meaning that it's requiring a Revision log message during the removal process, which is unnecessary. Plus, that message never shows on the front end.

jilladams commented 3 days ago

Nice progress! If biggish testing means you need help, holler in scrum and we can sort out help.

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

CLP: draft content going to prod #19551

Description

Technical notes

Acceptance criteria

What had happened

Observations from testing

CLP node:edit

Entity browser form for Promo block, accessed via CLP node:edit

Proposed fix for the breaking AJAX

Place Promo button does not show after 3rd promo block is removed

Continuing saga of the remove button

On prod

On PR env

Conclusion

Remove button, part deux

Row weight issue

Remove block III: This time it's personal

Following the data