Published entities created in a workspace should return the workspace-specific revision

Created on 19 March 2024, 3 months ago
Updated 8 April 2024, 3 months ago

Problem/Motivation

When a published entity is added in a workspace, two revisions are created:
1. a default revision, unpublished, needed by the entity storage so database queries in the Live site continue to work
2. a pending revision, published, workspace-specific, so it can be displayed and queried in that workspace

Any code that runs after the entity save process and needs to track or reference the revision ID the new entity, has to work with the second revision object.

Steps to reproduce

Switch to a workspace, create and save a published entity:

$entity = EntityClass::create();
$entity->save();

After the entity is saved by the storage, $entity->getRevisionId(); should be the ID of the second (workspace-specific) revision that was created automatically.

Proposed resolution

Change the workspaces_entity_insert() implementation to re-save the passed-in $entity object directly instead of operating on a clone.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Since workspaces_entity_insert() is executed last, there should be no behavior change for other hook implementations. However, code that runs after the save process is completed will have access to the second (workspace-specific) revision.

Data model changes

Nope.

Release notes snippet

Nope.

πŸ› Bug report
Status

Fixed

Version

10.3 ✨

Component
WorkspacesΒ  β†’

Last updated 18 days ago

No maintainer
Created by

πŸ‡·πŸ‡΄Romania amateescu

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @amateescu
  • Merge request !7096Fix double-revision saving in a workspace. β†’ (Open) created by amateescu
  • Status changed to Needs review 3 months ago
  • Pipeline finished with Success
    3 months ago
    Total: 837s
    #123628
  • First commit to issue fork.
  • Status changed to RTBC 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Applied 1 nitpicky change for :void

    1) Drupal\Tests\workspaces\Kernel\WorkspaceIntegrationTest::testWorkspaceAssociationDataIntegrity
    Failed asserting that '5' matches expected 6.
    /builds/issue/drupal-3432228/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
    /builds/issue/drupal-3432228/core/modules/workspaces/tests/src/Kernel/WorkspaceIntegrationTest.php:414
    /builds/issue/drupal-3432228/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
    FAILURES!
    Tests: 20, Assertions: 1568, Failures: 1.
    

    Shows the test coverage

    Change make sense to me. Since this is a workspace critical will mark now.

  • Pipeline finished with Success
    3 months ago
    Total: 663s
    #124288
    • catch β†’ committed 53c02df1 on 10.3.x
      Issue #3432228 by amateescu, smustgrave: Published entities created in a...
    • catch β†’ committed a39f6541 on 11.x
      Issue #3432228 by amateescu, smustgrave: Published entities created in a...
  • Status changed to Fixed 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Discussed this one with @amateescu in slack. There was an alternative option to reverse the order of entity creation - workspace-specific first, unpublished default revision - but this would have re-introduced the race condition we worked hard to eliminate in Drupal 7 contrib where there's a published default revision before the unpublished one, which can leak into the live site if it's busy enough.

    Removing the cloning here is also a bit alarming, but I don't see another way, and the hook_module_implements_alter() ensures to the best extent that we can that it only happens after everything else has run.

    Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024