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
97 stars 69 forks source link

Bug: CMS migrator only overwriting the last row in the Revisions database on Facilities products #17587

Closed xiongjaneg closed 5 months ago

xiongjaneg commented 5 months ago

Background

Public Websites discovered a migration problem that may also affect Facilities migrations: the nightly CMS migration for Forms is not correctly creating Revisions on Forms nodes.

The CMS migrator is always only overwriting the last row in the Revisions database, rather than writing a new row.

The new CMS Migrator revision squashes the most recent previous Revision into it (regardless whether previous is an Editor generated or a CMS Migrator revision). The new Revision contains both sets of changes.

This could snowball: every successive CMS migrator revision will squash the Revision before it. Over time, this could end up with a single Revision that represents several squashes, if only nightly changes are being made, if that makes sense. (Meaning; if a node gets edited 5 times in a day, only the last revision will get squashed into the nightly migration. But if a node is edited once a day, every day's edit will get rolled up into the next nightly CMS migration revision.)

The Forms migration is also no longer correctly applying all the Flags we expect to be applied to Forms. (Specifically our Changed Title flag is no longer being correctly applied.)

PW is going to work with CAIA since they're regularly editing, just to see if we can confirm that edited content is successfully capturing all the expected revisions.

There's also a chance this will trace back to Drupal core and become a CMS team problem.

Flags

Public Websites also has an instance of Flags not being correctly applied during migration. While checking migrations, it would be wise to test flags and attempt to verify if flags are being applied as expected.

Helpful links

Problem statement

Facilities needs to know if CMS migrator is not correctly creating revisions on Facilities products nodes.

Acceptance Criteria

jilladams commented 5 months ago

Recording of Daniel showing the problem description / demo between prod and Tugboat:

Becapa commented 5 months ago

Migrator made changes to the East Los Angeles Vet Center node on 03/19. It also made changes on 02/02 at 8:00am. 'The Last Saved by an Editor' is set to 02/02 at 11:36am, but there are no revisions in between the changes from the CMS Migrator:

Revisions Tab For the Node Screenshot 2024-03-21 at 3 37 55 PM

When comparing the last revision with its previous revision there are Hours changes, but there are also Local Spotlight changes that probably shouldn't have been coming from the CMS Migrator:

Comparison of latest revision Screenshot 2024-03-21 at 3 25 27 PM
xiongjaneg commented 5 months ago

There's a known defect with Vet Center last saved by an editor #15124

davidmpickett commented 5 months ago

Also I just found a Tugboat that built a few days ago and checked Hopi. It has a revision dated March 13 that seems to be overridden by the March 19th one

screencapture-pr17341-rygqrpifzaff3s9hwbystopqmiykvuuu-ci-cms-va-gov-farmington-vet-center-hopi-outstation-2024-03-21-15_03_34

Becapa commented 5 months ago

So I think this is something that happened in Drupal core. It looks like in 10.2 they made it so that when an entity is updating from a migrate there is an isSyncing flag that is being set. I guess this keeps the migrate from being able to set the update as a new Revision (see https://www.drupal.org/node/3052115). There is a workaround that's documented here: https://www.drupal.org/node/3426397

Becapa commented 5 months ago

Plan for moving forward with code fixes:

jilladams commented 5 months ago

Proposal: since we're on a roll and have time in sprint, let's update this as the implementation, not just spike? Also ticket doesn't have points, so we've already spent prob 3-5, and let's add more for the rest? @Becapa any concerns about handling it that way? If not, I'll update ACs.

Becapa commented 5 months ago

@jilladams That sounds good to me!

omahane commented 5 months ago

I spent time on Friday and today making a view to make sure that facility name changes have not slipped by without our notice. The good news is that it appears none have happened since 10.2 came out. Do we want this view added to prod? @xiongjaneg

xiongjaneg commented 5 months ago

The more views the better? It sounds like it's been helpful for you so may be so again in the future. Thanks!

omahane commented 5 months ago

After a little more discovery, I found that we already have a view that does this: https://prod.cms.va.gov/admin/content/audit?title_op=contains&title=&type_op=in&type%5B%5D=nca_facility&type%5B%5D=health_care_local_facility&type%5B%5D=vba_facility&type%5B%5D=vet_center&type%5B%5D=vet_center_mobile_vet_center&type%5B%5D=vet_center_outstation&moderation_state_op=or&section=All

No need for an additional view.

jilladams commented 5 months ago

@omahane @dsasser any thoughts on how we might verify this to close it?

I took a look to see if Forms would make it easy. PR merged after 3/27 nightly migration. 3/28 nightly migration only modified one form from what I can see:

Not totally obvious to me that anything's wrong, but I'm not sure what would make me think that other than another stomped edit.

jilladams commented 5 months ago

End of sprint: discussed with Christian / Daniel. Forms looks ok. We'd like to monitor migrations for a little bit just to make sure nothing looks amiss before we declare success. Will keep ticket open for follow up in next sprint.

omahane commented 5 months ago

OK, we do have some strangeness going on, all circling around March 2, 2024, 7:28 am. (CT)

Updated date not matching the revision log

Content view (/admin/content)

Palm Bay VA Clinic was updated on April 8, 2024

Screenshot 2024-04-10 at 17 42 17

Recent changes

No April 8 revision is shown. Screenshot 2024-04-10 at 17 42 35

Comparing revisions

The only difference is the addition of a health service. Screenshot 2024-04-10 at 17 43 28

VAMC facility health service creation

The time at which the health service was created is the same time at which /admin/content lists the facility as being updated. The facility is updated when a new service is created that is related to it by way of the corresponding entity reference.

Screenshot 2024-04-10 at 17 50 17

I suspect that the creation of health services is somehow bypassing the normal process and is creating a new regression, not squashing revisions but using the same time as the last revision when creating a new one.

jilladams commented 5 months ago

Ah, ok. From forms perspective, we have evidence of the migrator making multiple revisions since we pushed this change on 3/27: https://prod.cms.va.gov/find-forms/about-form-28-10289 - has updates from 4/2 and 4/4 both from the migrator.

So: I think your notes belong in a new ticket, and we can call the "multiple revisions squashed to a single revision" bug fixed. Let me know if you agree, @omahane and I'll close and put together the new ticket.

jilladams commented 5 months ago

https://github.com/department-of-veterans-affairs/va.gov-cms/issues/17808 tracks the new report