Resolve #3587526 "Detect conflicts during publishing"
Moved MR
Moved the changes from MR1193 as we have reverted MR1107. This new MR is made against the new MR1213 that is re-introducing reverted changes.
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" (project/canvas#3587587).
It is BE implementation of project/canvas#3587526 and counter-part of Draft: Prevent publishing pending changes with ... (canvas-3587526!1).
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/publishendpoint definition by addingHTTP 409response toPOSTmethod - Updates
ApiAutoSaveControllerto ensure conflicts are detected during the publishing of auto-save item(s) - Adds overrides (
OverriddenEntityChangedConstraintandOverriddenEntityChangedConstraintValidator) forEntityChangedconstraint and it's validator for Page entities as this constraint triggers in the same conditions asHTTP 409during auto-save item publishing and is redundant - Adds tests to
ApiAutoSaveControllerTestcovering "publishing if auto-save items is prevented on conflict detection"
Warning
As per project/canvas!1107 (comment 1072278) must also include the explicit coverage for changes project/canvas!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 CanvasPage, coreNodeor anything other entity type extending coreEditorialContentEntityBase) would perform conflict detection twice. Feels redundant and unsuitable for config entities. -
Current
HTTP 409error message from/canvas/api/v0/auto-saves/publish(provided byErrorCodesEnum::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
-
Create Page entity in Canvas, make a change, publish the changes
-
Modify same Page entity, but do not publish your changes Outside of Canvas resave the Page entity (using
drush esav canvas_page <entity id> -yshould work just fine)Caution
⚠️ The following step must be performed before the next automatic call toauto-saves/pending(every 10s) as that will returnHTTP 409and ruin the test: -
Attempt publishing your changes and observe HTTP
POSTto/canvas/api/v0/auto-saves/pendingreturnHTTP 409:- Response should have
errorsproperty with item for the Page entity modified in step 2. - Error item should have property
codewith value4, identifying theHTTP 409response as "resolvable in Canvas UI" as described in project/canvas#3587587 - Error item must have
metaproperty and it must containconflict_idproperty set to string value of the current latest revision of Page entity updated in step 3 that is stored in db.
- Response should have
One of two MRs that will close #3587526
