Problem/Motivation

Workspaces structure: develop -> stage -> live.

After merging the develop workspace into stage, the workspace revision metadata field is not updated to point to the new workspace ID.

Steps to reproduce

  • Create a node in develop workspace
  • Merge the develop workspace into stage in /admin/config/workflow/workspaces/manage/develop/stage/merge
  • Either load the latest revision of that node and check the value of the workspace revision metadata field, or look it up in the node_revision table.

Proposed resolution

Update the workspace revision metadata field for all the revisions involved when merging two workspaces.

CommentFileSizeAuthor
#10 3323317-nr-bot.txt1.68 KBneeds-review-queue-bot
#3 3323317-fix-contraint-error-on-deploy-from-develop-to-stage.patch970 bytesvladimir_kriukov
fix-contraint-error-on-deploy-from-playground-to-quality.patch970 bytesvladimir_kriukov

Issue fork drupal-3323317

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:

About issue forks

Comments

vladimir_kryukov created an issue. See original summary.

vladimir_kriukov’s picture

vladimir_kriukov commented
vladimir_kriukov’s picture

vladimir_kriukov commented
StatusFileSize
new 3323317-fix-contraint-error-on-deploy-from-develop-to-stage.patch970 bytes
antonnavi’s picture

antonnavi
Primary language Ukrainian
Location Leipzig
commented
Status: Active » Needs review
amateescu’s picture

amateescu commented
Status: Needs review » Needs work
Issue tags: +Needs tests

I agree with the solution, since the workspace merger behaves like the publisher and it's not creating new revisions when a workspace is merged into its parent. However, we need test coverage for this, somewhere in \Drupal\Tests\workspaces\Functional\WorkspaceConcurrentEditingTest.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

amateescu opened merge request !7365

amateescu’s picture

amateescu commented

Improved the patch a bit and converted to a MR, still needs tests so leaving at NW for now.

amateescu’s picture

amateescu commented
Title: Contraint validation error after merge: The content is being edited in the develop workspace » Revision metadata is not updated when a workspace is merged into its parent
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs tests

Added test coverage and revamped the issue summary since this can no longer be reproduced via the workspace conflict constraint.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3323317-nr-bot.txt1.68 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

amateescu’s picture

amateescu commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Hiding patches for clarity

Test only feature was already ran

1) Drupal\Tests\workspaces\Kernel\WorkspaceAssociationTest::testWorkspaceAssociation
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
     1 => 4
     2 => 5
     3 => 6
-    4 => 7
-    5 => 9
 )
/builds/issue/drupal-3323317/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/drupal-3323317/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php:183
/builds/issue/drupal-3323317/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php:157
/builds/issue/drupal-3323317/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

So change to the Merger looks good.

michelle’s picture

michelle
she/her
Primary language English
Location Wisconsin, USA
commented

My issue, https://www.drupal.org/project/drupal/issues/3438769 was marked as a duplicate of this. However, this patch is not fixing the issue of the develop workspace not clearing. Is there something not working here, or is my issue not actually a duplicate?

amateescu’s picture

amateescu commented

Hm.. that's right, I thought #3438769: Sub workspace does not clear was about the value of the workspace metadata field, but it's actually about WorkspaceAssociation::onPostPublish() not deleting the associations of sub-workspaces. I'll reopen it :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Added a comment to the MR. It's a bit of a pain to address but this change makes a service that's injected unused.

amateescu’s picture

amateescu commented
Status: Needs work » Needs review

Oops, I didn't realize we don't need that service anymore. Since the Merger class is not a service and it's also tagged @internal, I only removed the argument from its constructor, but I had to do the whole deprecation dance anyway for WorkspaceOperationFactory because that's an actual service :/

Also added a CR for it: https://www.drupal.org/node/3440755

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Wasn't fully aware of DeprecatedServicePropertyTrait neat!

Deprecation is all green. Know the stable ticket for workspace is RTBC but maybe this can get in at the same time.

quietone’s picture

quietone commented

I checked the change record and it is correct and easy to understand.

  • alexpott committed 028e0937 on 10.3.x 
    Issue #3323317 by amateescu, vladimir_kriukov, smustgrave, alexpott:...

  • alexpott committed 36006808 on 11.x 
    Issue #3323317 by amateescu, vladimir_kriukov, smustgrave, alexpott:...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Whoops! Forgot to mark this fixed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Status: Fixed » Closed (fixed)

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