Entity queries for latest revisions should return the latest workspace-specific revisions

Created on 2 April 2019, almost 6 years ago
Updated 19 February 2024, 11 months ago

Problem/Motivation

Doing a 'latest revision' entity query should return the latest workspace-specific revisions, otherwise a user could edit a revision that belongs to a different workspace.

Proposed resolution

Make latest revision entity queries return the latest workspace-specific revisions instead.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Release notes snippet

Rntity queries that are looking for latest revisions of an entity type (by using \Drupal\Core\Entity\Query\QueryInterface::latestRevision()), are now returning the latest workspace-specific revision when running in a non-default workspace context, for example in the stage workspace.

πŸ› Bug report
Status

Postponed

Version

11.0 πŸ”₯

Component
WorkspacesΒ  β†’

Last updated about 3 hours 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

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida

    Darren Oh β†’ made their first commit to this issue’s fork.

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida
  • Status changed to Needs review 11 months ago
  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida

    catch was just asking a question. This change is necessary to ensure predictable behavior with code that is not workspace-aware. I am currently trying to set up workspaces for a major client, and a blocker is that the edit tab does not load the revision for the current workspace by default.

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida
  • Merge request !6680Draft: #3045177 "Test only" β†’ (Open) created by darren oh
  • Pipeline finished with Failed
    11 months ago
    Total: 173s
    #98905
  • Pipeline finished with Failed
    11 months ago
    Total: 184s
    #98908
  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida
  • Pipeline finished with Success
    11 months ago
    Total: 560s
    #98915
  • Pipeline finished with Failed
    11 months ago
    Total: 1002s
    #98916
  • Pipeline finished with Success
    11 months ago
    Total: 741s
    #98923
  • Status changed to Needs work 11 months ago
  • πŸ‡·πŸ‡΄Romania amateescu

    @Darren Oh, I think you're the first one to actually bump into this problem that was only theoretical so far, so I'm wondering if you can share any more info about the code that is not workspace-aware and it was loading the wrong revision :)

    As for the MR, let's switch the order in the condition and check for isEntityTypeSupported() before hasActiveWorkspace(), see πŸ› Fix workspace-support check in entity queries Fixed for a recent example why this matters.

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida

    Sure I can share more info. If you have multiple workspaces, the first workspace in which a node is edited blocks editing in all other workspaces because the edit tab always loads the most recent revision. You'll get a message saying the node is being edited in another workspace and changes cannot be saved. The expected behavior is that the most recent live revision is loaded and copied to the current workspace.

  • πŸ‡·πŸ‡΄Romania amateescu

    The expected behavior is that the most recent live revision is loaded and copied to the current workspace.

    That's not the expected behavior in core at the moment. Editing the same entity in different workspaces requires a conflict resolution solution, which doesn't exist in core (yet?), that's why we actively prevent it.

    If you'd like to give editors the ability to edit the same entity in different workspaces, with the (important!) caveat that whichever workspace is published last "wins" (i.e. the changes from the previously-published workspace will be lost), I'd suggest to override the plugin class of the EntityWorkspaceConflict constraint with a custom one.

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida

    I did override the EntityWorkspaceConflict constraint. Also had to apply the fix from this issue.

  • πŸ‡·πŸ‡΄Romania amateescu

    There must be something else at play then, because I tried the snippet below and the following steps:

    1. edit a node in a workspace, save
    2. edit the node in Live, save
    3. create a new workspace, and editing the node showed me the revision created in step 2.

    --- a/core/modules/workspaces/src/Plugin/Validation/Constraint/EntityWorkspaceConflictConstraintValidator.php
    +++ b/core/modules/workspaces/src/Plugin/Validation/Constraint/EntityWorkspaceConflictConstraintValidator.php
    @@ -79,6 +79,7 @@ public static function create(ContainerInterface $container) {
        * {@inheritdoc}
        */
       public function validate($entity, Constraint $constraint) {
    +    return;
         /** @var \Drupal\Core\Entity\EntityInterface $entity */
         if (isset($entity) && !$entity->isNew()) {
           $active_workspace = $this->workspaceManager->getActiveWorkspace();
    
  • Status changed to Postponed: needs info 9 months ago
  • πŸ‡·πŸ‡΄Romania amateescu

    amateescu β†’ changed the visibility of the branch 3045177-test-only to hidden.

  • Pipeline finished with Failed
    13 days ago
    Total: 348s
    #396836
  • Pipeline finished with Failed
    7 days ago
    Total: 145s
    #402135
  • πŸ‡·πŸ‡΄Romania amateescu

    While working on πŸ“Œ Allow for / implement simplified content workflow with workspaces Active , it turned out that this is very much needed for that issue, and for ✨ Ability to edit content in live workspace when it has been edited in other workspaces Active as well.

    Pushed an update to also handle latest translation affected revisions.

  • Pipeline finished with Failed
    7 days ago
    Total: 128s
    #402398
  • Pipeline finished with Success
    7 days ago
    Total: 516s
    #402406
  • Merge request !10971Handle latest revision queries. β†’ (Open) created by amateescu
  • Pipeline finished with Failed
    7 days ago
    Total: 148s
    #402444
  • Merge request !10973Handle latest revision queries. β†’ (Open) created by amateescu
  • Pipeline finished with Success
    7 days ago
    Total: 815s
    #402505
  • Pipeline finished with Success
    7 days ago
    Total: 977s
    #402508
  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • πŸ‡·πŸ‡΄Romania amateescu

    The bot is trying to apply the 11.1.x MR to 11.x

  • πŸ‡¬πŸ‡§United Kingdom catch

    The MR needs to be against 11.x since that's the target for commits, the bot isn't wrong there.

    This could use an issue summary about why it needs to be done and what the impact is.

  • πŸ‡·πŸ‡΄Romania amateescu

    The bot is confused when there are multiple MRs. 6681 (3045177-workspace-specific-latest-revision) is the one for 11.x. Updated the IS.

  • πŸ‡©πŸ‡ͺGermany Fabianx

    Looks great to me!

    Had just one question, but that is minor.

    Tests also make sense.

    FWIW, here is a test summary by AI:

    This is a kernel test for Drupal's Workspaces module, specifically testing how entity revisions are managed in different workspaces and languages. Let's break it down:

    **1. Key Components:**
    - **Workspace Context:** Tests how entities behave when edited in different workspaces ('ham', 'cheese') vs Live environment
    - **Multilingual Support:** Tests with English (en) and Romanian (ro) languages
    - **Two Entity Types:**
    - `entity_test_revpub`: Revisionable, non-translatable entity
    - `entity_test_mulrevpub`: Revisionable AND translatable entity

    **2. Test Flow:**

    **A. Non-translatable Entity Test:**
    1. Creates initial entity in Live
    2. Creates revisions in workspaces:
    - 'ham' workspace revision
    - 'cheese' workspace revision
    - New Live revision
    3. Verifications:
    - Each workspace sees its own revision
    - Live sees the latest Live revision
    - Workspaces maintain their revisions even after Live updates

    **B. Translatable Entity Test:**
    1. Creates multilingual entity with EN/RO translations in Live
    2. Tests workspace-specific translations:
    - Creates RO translation revision in 'ham'
    - Creates RO translation revision in 'cheese'
    - Creates new RO translation in Live
    3. Verifications:
    - Correct workspace-specific RO translations are returned
    - Language context (en/ro) is respected
    - Workspace revisions remain isolated from each other and Live

    **3. Key Technical Checks:**
    - `EntityRepository::getActive()` correctly resolves:
    - Workspace context (which revision is active in which workspace)
    - Language context (which translation to show)
    - Revision tracking across workspaces
    - Workspace switching mechanics (`switchToWorkspace()`/`switchToLive()`)
    - Proper isolation of workspace-specific revisions

    **4. Why This Matters:**
    - Ensures content editors in different workspaces don't interfere with each other
    - Validates proper version control for multilingual content
    - Maintains data integrity when publishing from workspaces to Live
    - Tests core Workspaces module functionality at the storage layer

    The test uses kernel-level testing (without full HTTP stack) to validate these low-level entity operations efficiently.

  • First commit to issue fork.
  • Pipeline finished with Success
    4 days ago
    Total: 6359s
    #404782
  • πŸ‡¬πŸ‡§United Kingdom oily Greater London

    Edited code to enhance clarity.

  • πŸ‡¬πŸ‡§United Kingdom oily Greater London

    Edited issue summary.

  • πŸ‡¬πŸ‡§United Kingdom oily Greater London

    Where it states 'The only impact on existing sites is that entity queries will function properly if the locking protection mentioned above was bypassed in custom code.' in the issue summary should that be '...entity queries will not function properly...'?

  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida

    No, Drupal was relying on the locking mechanism to hide this bug from users. Existing sites that bypass the locking mechanism in custom code will now work properly without needing additional workarounds.

  • πŸ‡¬πŸ‡§United Kingdom oily Greater London

    @darran oh Great! MR seems good to me.

  • Pipeline finished with Running
    4 days ago
    #405020
  • πŸ‡¬πŸ‡§United Kingdom oily Greater London

    Rebased.

  • πŸ‡¬πŸ‡§United Kingdom catch

    One question on the MR.

  • πŸ‡·πŸ‡΄Romania amateescu

    Replied to the MR comment. It seems that the number of MRs here causes some confusion, so here's a quick breakdown for them:

    11.x -> MR !6681
    11.1.x -> MR !10971
    10.5.x (and below) -> MR !10973

    And to reply to that comment here as well, note that in the 11.x MR the hook implementation is in the proper place: core/modules/workspaces/src/Hook/EntityOperations.php

    OOP hooks made it very "fun" to work on the Worskspaces module :)

  • πŸ‡¬πŸ‡§United Kingdom catch

    Sorry I completely missed that was the 11.1 MR. However I left one more question on the 11.x MR about an explanatory comment and possibly additional test coverage.

  • πŸ‡·πŸ‡΄Romania amateescu

    amateescu β†’ changed the visibility of the branch 3045177-10.5.x to hidden.

  • πŸ‡·πŸ‡΄Romania amateescu

    Updated both the 11.x and 11.1.x MRs to expand that comment, and added explicit test coverage.

  • Pipeline finished with Success
    about 11 hours ago
    Total: 9781s
    #408097
  • Pipeline finished with Failed
    about 11 hours ago
    Total: 9670s
    #408100
  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida
  • πŸ‡ΊπŸ‡ΈUnited States darren oh Lakeland, Florida
Production build 0.71.5 2024