From 36006808280431a4ff2ff35fdebc6863fe9795f2 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Mon, 15 Apr 2024 10:07:18 +0100
Subject: [PATCH] Issue #3323317 by amateescu, vladimir_kriukov, smustgrave,
 alexpott: Revision metadata is not updated when a workspace is merged into
 its parent

---
 .../workspaces/src/WorkspaceMerger.php        | 27 ++++------
 .../src/WorkspaceOperationFactory.php         | 49 +++++++++++++------
 .../src/Kernel/WorkspaceAssociationTest.php   | 23 ++++++++-
 .../workspaces/workspaces.services.yml        |  2 +-
 4 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/core/modules/workspaces/src/WorkspaceMerger.php b/core/modules/workspaces/src/WorkspaceMerger.php
index 2500ea157748..2734efbb9aea 100644
--- a/core/modules/workspaces/src/WorkspaceMerger.php
+++ b/core/modules/workspaces/src/WorkspaceMerger.php
@@ -2,7 +2,6 @@
 
 namespace Drupal\workspaces;
 
-use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Drupal\Core\Utility\Error;
@@ -50,13 +49,6 @@ class WorkspaceMerger implements WorkspaceMergerInterface {
    */
   protected $workspaceAssociation;
 
-  /**
-   * The cache tag invalidator.
-   *
-   * @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface
-   */
-  protected $cacheTagsInvalidator;
-
   /**
    * Constructs a new WorkspaceMerger.
    *
@@ -66,8 +58,6 @@ class WorkspaceMerger implements WorkspaceMergerInterface {
    *   Database connection.
    * @param \Drupal\workspaces\WorkspaceAssociationInterface $workspace_association
    *   The workspace association service.
-   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator
-   *   The cache tags invalidator service.
    * @param \Drupal\workspaces\WorkspaceInterface $source
    *   The source workspace.
    * @param \Drupal\workspaces\WorkspaceInterface $target
@@ -75,11 +65,10 @@ class WorkspaceMerger implements WorkspaceMergerInterface {
    * @param \Psr\Log\LoggerInterface|null $logger
    *   The logger.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, Connection $database, WorkspaceAssociationInterface $workspace_association, CacheTagsInvalidatorInterface $cache_tags_invalidator, WorkspaceInterface $source, WorkspaceInterface $target, protected ?LoggerInterface $logger = NULL) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, Connection $database, WorkspaceAssociationInterface $workspace_association, WorkspaceInterface $source, WorkspaceInterface $target, protected ?LoggerInterface $logger = NULL) {
     $this->entityTypeManager = $entity_type_manager;
     $this->database = $database;
     $this->workspaceAssociation = $workspace_association;
-    $this->cacheTagsInvalidator = $cache_tags_invalidator;
     $this->sourceWorkspace = $source;
     $this->targetWorkspace = $target;
     if ($this->logger === NULL) {
@@ -103,21 +92,23 @@ public function merge() {
     try {
       $transaction = $this->database->startTransaction();
       foreach ($this->getDifferringRevisionIdsOnSource() as $entity_type_id => $revision_difference) {
+        $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
         $revisions_on_source = $this->entityTypeManager->getStorage($entity_type_id)
           ->loadMultipleRevisions(array_keys($revision_difference));
 
-        /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
+        /** @var \Drupal\Core\Entity\ContentEntityInterface $revision */
         foreach ($revisions_on_source as $revision) {
           // Track all the differing revisions from the source workspace in
           // the context of the target workspace. This will automatically
           // update all the descendants of the target workspace as well.
           $this->workspaceAssociation->trackEntity($revision, $this->targetWorkspace);
-        }
 
-        // Since we're not saving entity objects, we need to invalidate the list
-        // cache tags manually.
-        $entity_type = $this->entityTypeManager->getDefinition($entity_type_id);
-        $this->cacheTagsInvalidator->invalidateTags($entity_type->getListCacheTags());
+          // Set the workspace in which the revision was merged.
+          $field_name = $entity_type->getRevisionMetadataKey('workspace');
+          $revision->{$field_name}->target_id = $this->targetWorkspace->id();
+          $revision->setSyncing(TRUE);
+          $revision->save();
+        }
       }
     }
     catch (\Exception $e) {
diff --git a/core/modules/workspaces/src/WorkspaceOperationFactory.php b/core/modules/workspaces/src/WorkspaceOperationFactory.php
index 4346763c62f4..d5bb15b12473 100644
--- a/core/modules/workspaces/src/WorkspaceOperationFactory.php
+++ b/core/modules/workspaces/src/WorkspaceOperationFactory.php
@@ -4,6 +4,7 @@
 
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
 use Drupal\Core\Database\Connection;
+use Drupal\Core\DependencyInjection\DeprecatedServicePropertyTrait;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
 use Psr\Log\LoggerInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
@@ -18,6 +19,17 @@
  */
 class WorkspaceOperationFactory {
 
+  use DeprecatedServicePropertyTrait;
+
+  /**
+   * Defines deprecated injected properties.
+   *
+   * @var array
+   */
+  protected array $deprecatedProperties = [
+    'cacheTagInvalidator' => 'cache_tags.invalidator',
+  ];
+
   /**
    * The entity type manager.
    *
@@ -47,18 +59,18 @@ class WorkspaceOperationFactory {
   protected $workspaceAssociation;
 
   /**
-   * The cache tags invalidator.
+   * An event dispatcher instance to use for configuration events.
    *
-   * @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface
+   * @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface
    */
-  protected $cacheTagsInvalidator;
+  protected $eventDispatcher;
 
   /**
-   * An event dispatcher instance to use for configuration events.
+   * The logger service.
    *
-   * @var \Symfony\Contracts\EventDispatcher\EventDispatcherInterface
+   * @var \Psr\Log\LoggerInterface
    */
-  protected $eventDispatcher;
+  protected $logger;
 
   /**
    * Constructs a new WorkspaceOperationFactory.
@@ -71,28 +83,33 @@ class WorkspaceOperationFactory {
    *   The workspace manager service.
    * @param \Drupal\workspaces\WorkspaceAssociationInterface $workspace_association
    *   The workspace association service.
-   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator
-   *   The cache tags invalidator service.
-   * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $event_dispatcher
+   * @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface|\Drupal\Core\Cache\CacheTagsInvalidatorInterface $event_dispatcher
    *   The event dispatcher.
    * @param \Psr\Log\LoggerInterface|null $logger
    *   The logger.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, Connection $database, WorkspaceManagerInterface $workspace_manager, WorkspaceAssociationInterface $workspace_association, CacheTagsInvalidatorInterface $cache_tags_invalidator, EventDispatcherInterface $event_dispatcher = NULL, protected ?LoggerInterface $logger = NULL) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, Connection $database, WorkspaceManagerInterface $workspace_manager, WorkspaceAssociationInterface $workspace_association, EventDispatcherInterface|CacheTagsInvalidatorInterface $event_dispatcher, LoggerInterface|EventDispatcherInterface $logger = NULL) {
     $this->entityTypeManager = $entity_type_manager;
     $this->database = $database;
     $this->workspaceManager = $workspace_manager;
     $this->workspaceAssociation = $workspace_association;
-    $this->cacheTagsInvalidator = $cache_tags_invalidator;
-    if (!$event_dispatcher) {
-      @trigger_error('Calling ' . __METHOD__ . '() without the $event_dispatcher argument is deprecated in drupal:10.1.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3242573', E_USER_DEPRECATED);
+
+    if ($event_dispatcher instanceof CacheTagsInvalidatorInterface) {
       $event_dispatcher = \Drupal::service('event_dispatcher');
+      @trigger_error('Calling ' . __METHOD__ . '() with the $cache_tags_invalidator argument is deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. See https://www.drupal.org/node/3440755', E_USER_DEPRECATED);
+    }
+    elseif (!$event_dispatcher instanceof EventDispatcherInterface) {
+      $event_dispatcher = \Drupal::service('event_dispatcher');
+      @trigger_error('Calling ' . __METHOD__ . '() without the $event_dispatcher argument is deprecated in drupal:10.1.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/3242573', E_USER_DEPRECATED);
     }
     $this->eventDispatcher = $event_dispatcher;
-    if ($this->logger === NULL) {
+
+    $logger = func_get_arg(5);
+    if (!$logger instanceof LoggerInterface) {
+      $logger = \Drupal::service('logger.channel.workspaces');
       @trigger_error('Calling ' . __METHOD__ . '() without the $logger argument is deprecated in drupal:10.1.0 and it will be required in drupal:11.0.0. See https://www.drupal.org/node/2932520', E_USER_DEPRECATED);
-      $this->logger = \Drupal::service('logger.channel.workspaces');
     }
+    $this->logger = $logger;
   }
 
   /**
@@ -120,7 +137,7 @@ public function getPublisher(WorkspaceInterface $source) {
    *   A workspace merger object.
    */
   public function getMerger(WorkspaceInterface $source, WorkspaceInterface $target) {
-    return new WorkspaceMerger($this->entityTypeManager, $this->database, $this->workspaceAssociation, $this->cacheTagsInvalidator, $source, $target, $this->logger);
+    return new WorkspaceMerger($this->entityTypeManager, $this->database, $this->workspaceAssociation, $source, $target, $this->logger);
   }
 
 }
diff --git a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php
index a790d3b1984f..e22e2bf026cb 100644
--- a/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php
+++ b/core/modules/workspaces/tests/src/Kernel/WorkspaceAssociationTest.php
@@ -132,9 +132,30 @@ public function testWorkspaceAssociation() {
       'dev' => [3, 4, 5, 6, 7, 8, 9],
     ];
     $expected_initial_revisions += [
-      'stage' => [7, 8],
+      'dev' => [7, 8],
     ];
     $this->assertWorkspaceAssociations('node', $expected_latest_revisions, $expected_all_revisions, $expected_initial_revisions);
+
+    // Merge 'dev' into 'stage' and check the workspace associations.
+    /** @var \Drupal\workspaces\WorkspaceMergerInterface $workspace_merger */
+    $workspace_merger = \Drupal::service('workspaces.operation_factory')->getMerger($this->workspaces['dev'], $this->workspaces['stage']);
+    $workspace_merger->merge();
+
+    // The latest revisions from 'dev' are now tracked in 'stage'.
+    $expected_latest_revisions['stage'] = $expected_latest_revisions['dev'];
+
+    // Two revisions (8 and 9) were created for 'Test article 6', but only the
+    // latest one (9) is being merged into 'stage'.
+    $expected_all_revisions['stage'] = [3, 4, 5, 6, 7, 9];
+
+    // Revision 7 was both an initial and latest revision in 'dev', so it is now
+    // considered an initial revision in 'stage'.
+    $expected_initial_revisions['stage'] = [4, 5, 7];
+
+    // Which leaves revision 8 as the only remaining initial revision in 'dev'.
+    $expected_initial_revisions['dev'] = [8];
+
+    $this->assertWorkspaceAssociations('node', $expected_latest_revisions, $expected_all_revisions, $expected_initial_revisions);
   }
 
   /**
diff --git a/core/modules/workspaces/workspaces.services.yml b/core/modules/workspaces/workspaces.services.yml
index 1c07e67988d7..9b5f3b97ce8d 100644
--- a/core/modules/workspaces/workspaces.services.yml
+++ b/core/modules/workspaces/workspaces.services.yml
@@ -13,7 +13,7 @@ services:
   Drupal\workspaces\WorkspaceInformationInterface: '@workspaces.information'
   workspaces.operation_factory:
     class: Drupal\workspaces\WorkspaceOperationFactory
-    arguments: ['@entity_type.manager', '@database', '@workspaces.manager', '@workspaces.association', '@cache_tags.invalidator', '@event_dispatcher', '@logger.channel.workspaces']
+    arguments: ['@entity_type.manager', '@database', '@workspaces.manager', '@workspaces.association', '@event_dispatcher', '@logger.channel.workspaces']
   Drupal\workspaces\WorkspaceOperationFactory: '@workspaces.operation_factory'
   workspaces.association:
     class: Drupal\workspaces\WorkspaceAssociation
-- 
GitLab