Draft: Detect conflicts during publishing [BE]

Stale MR

We might end up closing this MR as we have now reverted !1107 (merged) in !1205 (merged).

There is a new MR !1213 to re-introduce conflict detection changes. This MR might get closed in favour of the issue/canvas-3589076!4 as it targets the new MR1213.

TL;DR: Adds HTTP 409 responses to the "publish auto-save items" endpoint for auto-save items; Prevents publishing if a conflict is detected;

For people with attention span or AI summary tools:

This MR adds "Prevent publishing of auto-save items with detected conflicts on BE" capability described in the "[META] Review of changes and Conflict resolution" (#3587587).

It is BE implementation of #3587526 and counter-part of <FE MR link goes here>.

To keep the scope manageable, changes are limited to Page (canvas_page) entities. Effort is made to leave path open for other entity types.

List of changes:

• Updates the /canvas/api/v0/auto-saves/publish endpoint definition by adding HTTP 409 response to POST method
• Updates ApiAutoSaveController to ensure conflicts are detected during the publishing of auto-save item(s)
• Adds overrides (OverriddenEntityChangedConstraint and OverriddenEntityChangedConstraintValidator) for EntityChanged constraint and it's validator for Page entities as this constraint triggers in the same conditions as HTTP 409 during auto-save item publishing and is redundant
• Adds tests to ApiAutoSaveControllerTest covering "publishing if auto-save items is prevented on conflict detection"

Warning

As per !1107 (comment 1072278) must also include the explicit coverage for changes !1107 (merged) made to the AutoSaveManager::getAllAutoSaveList() added to AutoSaveManagerTest

What's missing (opinions and suggestions are welcome):

• Override could be refactored to perform the conflict detection and not just skip validation on "no conflict". But his would mean that on publishing of auto-save items for entities implementing Drupal\Core\Entity\EntityChangedInterface (such as Canvas Page, core Node or anything other entity type extending core EditorialContentEntityBase) would perform conflict detection twice. Feels redundant and unsuitable for config entities.

• Current HTTP 409 error message from /canvas/api/v0/auto-saves/publish (provided by ErrorCodesEnum::AutoSaveItemEntityConflict->getMessage()) is a simple "Conflict detected.", so maybe something more elaborate could be used? On the other hand, such response will create "Conflict detected CTA" in the selective publishing dropdown (design attached bellow), so perhaps showing the error in the same style as other validation errors isn't even necessary? 🤔

  

Testing instructions

Quick fingers are required 😅

1. Create Page entity in Canvas, make a change, publish the changes

2. Modify same Page entity, but do not publish your changes Outside of Canvas resave the Page entity (using drush esav canvas_page <entity id> -y should work just fine)

   Caution

   ⚠️ The following step must be performed before the next automatic call to auto-saves/pending (every 10s) as that will return HTTP 409 and ruin the test:

3. Attempt publishing your changes and observe HTTP POST to /canvas/api/v0/auto-saves/pending return HTTP 409 :

   • Response should have errors property with item for the Page entity modified in step 2.
   • Error item should have property code with value 4 , identifying the HTTP 409 response as "resolvable in Canvas UI" as described in #3587587
   • Error item must have meta property and it must contain conflict_id property set to string value of the current latest revision of Page entity updated in step 3 that is stored in db.

One of two MRs that will close #3587526