Problem/Motivation

When changes are made in a sub-workspace and pushed to the live workspace, the sub-workspace does not clear old changes as expected. This is due to the WorkspaceAssociation::onPostPublish() method not deleting the associations of sub-workspaces when published to the live workspace. The issue persists as old changes continue to appear after merging and publishing.

Steps to reproduce

1. Set up Drupal with the Workspaces module enabled.
2. Create a parent workspace and multiple sub-workspaces.
3. Make changes in a sub-workspace.
4. Merge the sub-workspace changes into the Stage workspace.
5. Publish changes from the Stage workspace to the Live workspace.
6. Verify that the sub-workspace has been cleared and old changes are no longer present.

Proposed resolution

To resolve this issue, we need to implement a cleanup logic to ensure that the associations between sub-workspaces and their changes are properly removed when changes are published to the live workspace. This will prevent old changes from persisting and ensure that the workspace accurately reflects the latest changes.

Issue fork drupal-3438769

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Michelle created an issue. See original summary.

amateescu’s picture

Status: Active » Closed (duplicate)
michelle’s picture

@amateescu,

Thanks for finding that. I did search quite a bit before filing these issues but I guess I just wasn't using the right terms. I was calling it "sub workspace" not "develop workspace". If I hit any more problems, I'll search on that.

damienmckenna’s picture

amateescu’s picture

Version: 10.1.x-dev » 11.x-dev
Status: Closed (duplicate) » Active

Turns out I was wrong in #2, the problem here is WorkspaceAssociation::onPostPublish() not deleting the associations of sub-workpaces.

erik_mc’s picture

Bumping for visibility.

amitvin’s picture

As the workspace association is not getting deleted, One is not able to edit the content on the live workspace even if the content is published. Right now getting the message that content can not be edited as the content is edited in the sub workspace.
A temporary workaround is to delete the association manually (from the database) after merging the sub-workspace with the parent workspace.

antonnavi’s picture

Hello all here,

I had the same bug and fixed it in the way of adding additional cleanup on publication to Live workspace.
See details in attached patch file.

antonnavi’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests, +Needs title update

Hello, all fixes should be in MRs against 11.x so tests will run.

Also issue summary should be complete and a clear title. Tagging for both.

With most bugs will also need a test case showing the issue

vinmayiswamy made their first commit to this issue’s fork.

vinmayiswamy’s picture

Issue summary: View changes
vinmayiswamy’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update, -Needs tests

Hi, I have created an MR incorporating the patch changes from comment #9 and added a test case to validate these changes as outlined in comment #11. The issue summary has been updated accordingly.

I am removing the "Needs issue summary update" and "Needs tests" tags. If anyone believes these tags should still be applied, please feel free to re-add them.

Kindly review the changes and let me know if any further adjustments or updates are needed. Your feedback and suggestions for further improvements are greatly appreciated.

Thanks!

scott weston’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed code of #13 (and #9) and tested on Drupal 11.0.1 clean Standard installation with default and custom content types, taxonomy terms, and menu links. The bug experienced is no longer happening. Updating status to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs work

One question on the MR.

vinmayiswamy’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs title update +Needs Review Queue Initiative

Feedback appears to be addressed.

  • catch committed 52a4ee46 on 10.3.x
    Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu: Sub...

  • catch committed f5e2c4ea on 10.4.x
    Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu: Sub...

  • catch committed 5a08a6e3 on 11.0.x
    Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu: Sub...

  • catch committed c1f7b85d on 11.x
    Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu: Sub...
catch’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks that looks better.

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

antonnavi’s picture

Status: Fixed » Needs work

Hello all,

The code:

    if (!$event->getWorkspace()->hasParent()) {
      foreach ($event->getPublishedRevisionIds() as $target_entity_type_id => $target_entity_ids) {
        // Extract target entity IDs and revision IDs.
        $target_entity_ids_list = array_keys($target_entity_ids);
        $target_entity_revision_ids = array_keys($target_entity_ids);

        $this->deleteAssociations(NULL, $target_entity_type_id, $target_entity_ids_list, $target_entity_revision_ids);
      }
    }

which was committed to the core, there is a bug in this line:

        $target_entity_ids_list = array_keys($target_entity_ids);

The $target_entity_ids variable is an array where keys - revision IDs and values - entity IDs. So instead of:

        $target_entity_ids_list = array_keys($target_entity_ids);

should be:

        $target_entity_ids_list = array_values($target_entity_ids);

to get entity IDs instead of revisions.

  • catch committed 09612a68 on 10.3.x
    Revert "Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu:...

  • catch committed 38e4bc43 on 10.4.x
    Revert "Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu:...

  • catch committed 678bebde on 11.0.x
    Revert "Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu:...

  • catch committed 2b536b6c on 11.x
    Revert "Issue #3438769 by vinmayiswamy, antonnavi, michelle, amateescu:...
catch’s picture

Version: 10.3.x-dev » 11.x-dev
Issue tags: +Needs tests

Thanks that's a good spot!

I've rolled this back from all four branches, let's re-add it with some additional test coverage.

vinmayiswamy’s picture

Status: Needs work » Needs review

Thank you, @antonnavi, for highlighting the incorrect usage of $target_entity_ids_list = array_keys($target_entity_ids);.

I’ve updated the code to use $target_entity_ids_list = array_values($target_entity_ids);, which correctly retrieves the entity IDs instead of revision IDs. Additionally, I've added test coverage to verify this change, ensuring that workspace associations are properly cleaned up when publishing the workspace.

Kindly review the changes and please let me know if any further adjustments are needed. Your feedback and suggestions for improvements are greatly appreciated.

Thanks!

catch’s picture

In this context is array_values actually useful? i.e. do we need to replace the revision ID keys with indexed keys or does it not matter?

smustgrave’s picture

Status: Needs review » Needs work

#34 seems worth exploring.

amateescu’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Reviewed the MR and fixed all my comments, sorry for taking so long to get to this folks.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Feedback appears to be addressed!

  • catch committed 301c19da on 10.3.x
    Issue #3438769 by vinmayiswamy, amateescu, catch, antonnavi, michelle:...

  • catch committed b4fdd3fe on 10.4.x
    Issue #3438769 by vinmayiswamy, amateescu, catch, antonnavi, michelle:...

  • catch committed b5def768 on 11.0.x
    Issue #3438769 by vinmayiswamy, amateescu, catch, antonnavi, michelle:...

  • catch committed 2cfb0738 on 11.x
    Issue #3438769 by vinmayiswamy, amateescu, catch, antonnavi, michelle:...
catch’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

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

Status: Fixed » Closed (fixed)

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