Issue #2940204 by plach, Wim Leers, effulgentsia, matsbla, Gábor Hojtsy,...

Issue #2940204 by plach, Wim Leers, effulgentsia, matsbla, Gábor Hojtsy, tacituseu: Translatable fields with synchronization enabled should behave as untranslatable fields with respect to pending revisions

core/modules/content_translation/content_translation.module

@@ -163,6 +163,8 @@ function content_translation_entity_type_alter(array &$entity_types) {
       }
       $entity_type->set('translation', $translation);
     }
+
+    $entity_type->addConstraint('ContentTranslationSynchronizedFields');
   }
 }
 

core/modules/content_translation/content_translation.services.yml

 services:
   content_translation.synchronizer:
     class: Drupal\content_translation\FieldTranslationSynchronizer
-    arguments: ['@entity.manager']
+    arguments: ['@entity.manager', '@plugin.manager.field.field_type']
 
   content_translation.subscriber:
     class: Drupal\content_translation\Routing\ContentTranslationRouteSubscriber

core/modules/content_translation/src/FieldTranslationSynchronizer.php

@@ -5,6 +5,8 @@
 use Drupal\Core\Config\Entity\ThirdPartySettingsInterface;
 use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
+use Drupal\Core\Field\FieldDefinitionInterface;
+use Drupal\Core\Field\FieldTypePluginManagerInterface;
 
 /**
  * Provides field translation synchronization capabilities.
@@ -18,14 +20,57 @@ class FieldTranslationSynchronizer implements FieldTranslationSynchronizerInterf
    */
   protected $entityManager;
 
+  /**
+   * The field type plugin manager.
+   *
+   * @var \Drupal\Core\Field\FieldTypePluginManagerInterface
+   */
+  protected $fieldTypeManager;
+
   /**
    * Constructs a FieldTranslationSynchronizer object.
    *
    * @param \Drupal\Core\Entity\EntityManagerInterface $entityManager
    *   The entity manager.
+   * @param \Drupal\Core\Field\FieldTypePluginManagerInterface $field_type_manager
+   *   The field type plugin manager.
    */
-  public function __construct(EntityManagerInterface $entityManager) {
+  public function __construct(EntityManagerInterface $entityManager, FieldTypePluginManagerInterface $field_type_manager) {
     $this->entityManager = $entityManager;
+    $this->fieldTypeManager = $field_type_manager;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFieldSynchronizedProperties(FieldDefinitionInterface $field_definition) {
+    $properties = [];
+    $settings = $this->getFieldSynchronizationSettings($field_definition);
+    foreach ($settings as $group => $translatable) {
+      if (!$translatable) {
+        $field_type_definition = $this->fieldTypeManager->getDefinition($field_definition->getType());
+        if (!empty($field_type_definition['column_groups'][$group]['columns'])) {
+          $properties = array_merge($properties, $field_type_definition['column_groups'][$group]['columns']);
+        }
+      }
+    }
+    return $properties;
+  }
+
+  /**
+   * Returns the synchronization settings for the specified field.
+   *
+   * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition
+   *   A field definition.
+   *
+   * @return string[]
+   *   An array of synchronized field property names.
+   */
+  protected function getFieldSynchronizationSettings(FieldDefinitionInterface $field_definition) {
+    if ($field_definition instanceof ThirdPartySettingsInterface && $field_definition->isTranslatable()) {
+      return $field_definition->getThirdPartySetting('content_translation', 'translation_sync', []);
+    }
+    return [];
   }
 
   /**
@@ -33,7 +78,6 @@ public function __construct(EntityManagerInterface $entityManager) {
    */
   public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode, $original_langcode = NULL) {
     $translations = $entity->getTranslationLanguages();
-    $field_type_manager = \Drupal::service('plugin.manager.field.field_type');
 
     // If we have no information about what to sync to, if we are creating a new
     // entity, if we have no translations for the current entity and we are not
@@ -43,21 +87,55 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
     }
 
     // If the entity language is being changed there is nothing to synchronize.
-    $entity_type = $entity->getEntityTypeId();
-    $entity_unchanged = isset($entity->original) ? $entity->original : $this->entityManager->getStorage($entity_type)->loadUnchanged($entity->id());
+    $entity_unchanged = $this->getOriginalEntity($entity);
     if ($entity->getUntranslated()->language()->getId() != $entity_unchanged->getUntranslated()->language()->getId()) {
       return;
     }
 
+    if ($entity->isNewRevision()) {
+      if ($entity->isDefaultTranslationAffectedOnly()) {
+        // If changes to untranslatable fields are configured to affect only the
+        // default translation, we need to skip synchronization in pending
+        // revisions, otherwise multiple translations would be affected.
+        if (!$entity->isDefaultRevision()) {
+          return;
+        }
+        // When this mode is enabled, changes to synchronized properties are
+        // allowed only in the default translation, thus we need to make sure this
+        // is always used as source for the synchronization process.
+        else {
+          $sync_langcode = $entity->getUntranslated()->language()->getId();
+        }
+      }
+      elseif ($entity->isDefaultRevision()) {
+        // If a new default revision is being saved, but a newer default
+        // revision was created meanwhile, use any other translation as source
+        // for synchronization, since that will have been merged from the
+        // default revision. In this case the actual language does not matter as
+        // synchronized properties are the same for all the translations in the
+        // default revision.
+        /** @var \Drupal\Core\Entity\ContentEntityInterface $default_revision */
+        $default_revision = $this->entityManager
+          ->getStorage($entity->getEntityTypeId())
+          ->load($entity->id());
+        if ($default_revision->getLoadedRevisionId() !== $entity->getLoadedRevisionId()) {
+          $other_langcodes = array_diff_key($default_revision->getTranslationLanguages(), [$sync_langcode => FALSE]);
+          if ($other_langcodes) {
+            $sync_langcode = key($other_langcodes);
+          }
+        }
+      }
+    }
+
     /** @var \Drupal\Core\Field\FieldItemListInterface $items */
     foreach ($entity as $field_name => $items) {
       $field_definition = $items->getFieldDefinition();
-      $field_type_definition = $field_type_manager->getDefinition($field_definition->getType());
+      $field_type_definition = $this->fieldTypeManager->getDefinition($field_definition->getType());
       $column_groups = $field_type_definition['column_groups'];
 
       // Sync if the field is translatable, not empty, and the synchronization
       // setting is enabled.
-      if ($field_definition instanceof ThirdPartySettingsInterface && $field_definition->isTranslatable() && !$items->isEmpty() && $translation_sync = $field_definition->getThirdPartySetting('content_translation', 'translation_sync')) {
+      if (($translation_sync = $this->getFieldSynchronizationSettings($field_definition)) && !$items->isEmpty()) {
         // Retrieve all the untranslatable column groups and merge them into
         // single list.
         $groups = array_keys(array_diff($translation_sync, array_filter($translation_sync)));
@@ -101,6 +179,26 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
     }
   }
 
+  /**
+   * Returns the original unchanged entity to be used to detect changes.
+   *
+   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   *   The entity being changed.
+   *
+   * @return \Drupal\Core\Entity\ContentEntityInterface
+   *   The unchanged entity.
+   */
+  protected function getOriginalEntity(ContentEntityInterface $entity) {
+    if (!isset($entity->original)) {
+      $storage = $this->entityManager->getStorage($entity->getEntityTypeId());
+      $original = $entity->isDefaultRevision() ? $storage->loadUnchanged($entity->id()) : $storage->loadRevision($entity->getLoadedRevisionId());
+    }
+    else {
+      $original = $entity->original;
+    }
+    return $original;
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -174,9 +272,7 @@ public function synchronizeItems(array &$values, array $unchanged_items, $sync_l
           // items and the other columns from the existing values. This only
           // works if the delta exists in the language.
           elseif ($created && !empty($original_field_values[$langcode][$delta])) {
-            $item_columns_to_sync = array_intersect_key($source_items[$delta], array_flip($columns));
-            $item_columns_to_keep = array_diff_key($original_field_values[$langcode][$delta], array_flip($columns));
-            $values[$langcode][$delta] = $item_columns_to_sync + $item_columns_to_keep;
+            $values[$langcode][$delta] = $this->createMergedItem($source_items[$delta], $original_field_values[$langcode][$delta], $columns);
           }
           // If the delta doesn't exist, copy from the source language.
           elseif ($created) {
@@ -190,13 +286,37 @@ public function synchronizeItems(array &$values, array $unchanged_items, $sync_l
             // If the value has only been reordered we just move the old one in
             // the new position.
             $item = isset($original_field_values[$langcode][$old_delta]) ? $original_field_values[$langcode][$old_delta] : $source_items[$new_delta];
-            $values[$langcode][$new_delta] = $item;
+            // When saving a default revision starting from a pending revision,
+            // we may have desynchronized field values, so we make sure that
+            // untranslatable properties are synchronized, even if in any other
+            // situation this would not be necessary.
+            $values[$langcode][$new_delta] = $this->createMergedItem($source_items[$new_delta], $item, $columns);
           }
         }
       }
     }
   }
 
+  /**
+   * Creates a merged item.
+   *
+   * @param array $source_item
+   *   An item containing the untranslatable properties to be synchronized.
+   * @param array $target_item
+   *   An item containing the translatable properties to be kept.
+   * @param string[] $properties
+   *   An array of properties to be synchronized.
+   *
+   * @return array
+   *   A merged item array.
+   */
+  protected function createMergedItem(array $source_item, array $target_item, array $properties) {
+    $property_keys = array_flip($properties);
+    $item_properties_to_sync = array_intersect_key($source_item, $property_keys);
+    $item_properties_to_keep = array_diff_key($target_item, $property_keys);
+    return $item_properties_to_sync + $item_properties_to_keep;
+  }
+
   /**
    * Computes a hash code for the specified item.
    *

core/modules/content_translation/src/FieldTranslationSynchronizerInterface.php

@@ -3,6 +3,7 @@
 namespace Drupal\content_translation;
 
 use Drupal\Core\Entity\ContentEntityInterface;
+use Drupal\Core\Field\FieldDefinitionInterface;
 
 /**
  * Provides field translation synchronization capabilities.
@@ -54,4 +55,15 @@ public function synchronizeFields(ContentEntityInterface $entity, $sync_langcode
    */
   public function synchronizeItems(array &$field_values, array $unchanged_items, $sync_langcode, array $translations, array $columns);
 
+  /**
+   * Returns the synchronized properties for the specified field definition.
+   *
+   * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition
+   *   A field definition.
+   *
+   * @return string[]
+   *   An array of synchronized field property names.
+   */
+  public function getFieldSynchronizedProperties(FieldDefinitionInterface $field_definition);
+
 }

core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraint.php

+<?php
+
+namespace Drupal\content_translation\Plugin\Validation\Constraint;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * Validation constraint for the entity changed timestamp.
+ *
+ * @internal
+ *
+ * @Constraint(
+ *   id = "ContentTranslationSynchronizedFields",
+ *   label = @Translation("Content translation synchronized fields", context = "Validation"),
+ *   type = {"entity"}
+ * )
+ */
+class ContentTranslationSynchronizedFieldsConstraint extends Constraint {
+
+  // In this case "elements" refers to "field properties", in fact it is what we
+  // are using in the UI elsewhere.
+  public $defaultRevisionMessage = 'Non-translatable field elements can only be changed when updating the current revision.';
+  public $defaultTranslationMessage = 'Non-translatable field elements can only be changed when updating the original language.';
+
+}

core/modules/content_translation/src/Plugin/Validation/Constraint/ContentTranslationSynchronizedFieldsConstraintValidator.php

+<?php
+
+namespace Drupal\content_translation\Plugin\Validation\Constraint;
+
+use Drupal\content_translation\ContentTranslationManagerInterface;
+use Drupal\content_translation\FieldTranslationSynchronizerInterface;
+use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\Core\Entity\ContentEntityInterface;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Field\FieldDefinitionInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+
+/**
+ * Checks that synchronized fields are handled correctly in pending revisions.
+ *
+ * As for untranslatable fields, two modes are supported:
+ * - When changes to untranslatable fields are configured to affect all revision
+ *   translations, synchronized field properties can be changed only in default
+ *   revisions.
+ * - When changes to untranslatable fields affect are configured to affect only
+ *   the revision's default translation, synchronized field properties can be
+ *   changed only when editing the default translation. This may lead to
+ *   temporarily desynchronized values, when saving a pending revision for the
+ *   default translation that changes a synchronized property. These are
+ *   actually synchronized when saving changes to the default translation as a
+ *   new default revision.
+ *
+ * @see \Drupal\content_translation\Plugin\Validation\Constraint\ContentTranslationSynchronizedFieldsConstraint
+ * @see \Drupal\Core\Entity\Plugin\Validation\Constraint\EntityUntranslatableFieldsConstraintValidator
+ *
+ * @internal
+ */
+class ContentTranslationSynchronizedFieldsConstraintValidator extends ConstraintValidator implements ContainerInjectionInterface {
+
+  /**
+   * The entity type manager.
+   *
+   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
+   */
+  protected $entityTypeManager;
+
+  /**
+   * The content translation manager.
+   *
+   * @var \Drupal\content_translation\ContentTranslationManagerInterface
+   */
+  protected $contentTranslationManager;
+
+  /**
+   * The field translation synchronizer.
+   *
+   * @var \Drupal\content_translation\FieldTranslationSynchronizerInterface
+   */
+  protected $synchronizer;
+
+  /**
+   * ContentTranslationSynchronizedFieldsConstraintValidator constructor.
+   *
+   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
+   *   The entity type manager.
+   * @param \Drupal\content_translation\ContentTranslationManagerInterface $content_translation_manager
+   *   The content translation manager.
+   * @param \Drupal\content_translation\FieldTranslationSynchronizerInterface $synchronizer
+   *   The field translation synchronizer.
+   */
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, ContentTranslationManagerInterface $content_translation_manager, FieldTranslationSynchronizerInterface $synchronizer) {
+    $this->entityTypeManager = $entity_type_manager;
+    $this->contentTranslationManager = $content_translation_manager;
+    $this->synchronizer = $synchronizer;
+  }
+
+  /**
+   * [@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('entity_type.manager'),
+      $container->get('content_translation.manager'),
+      $container->get('content_translation.synchronizer')
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validate($value, Constraint $constraint) {
+    /** @var \Drupal\content_translation\Plugin\Validation\Constraint\ContentTranslationSynchronizedFieldsConstraint $constraint */
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
+    $entity = $value;
+    if ($entity->isNew() || !$entity->getEntityType()->isRevisionable()) {
+      return;
+    }
+    // When changes to untranslatable fields are configured to affect all
+    // revision translations, we always allow changes in default revisions.
+    if ($entity->isDefaultRevision() && !$entity->isDefaultTranslationAffectedOnly()) {
+      return;
+    }
+    $entity_type_id = $entity->getEntityTypeId();
+    if (!$this->contentTranslationManager->isEnabled($entity_type_id, $entity->bundle())) {
+      return;
+    }
+    $synchronized_properties = $this->getSynchronizedPropertiesByField($entity->getFieldDefinitions());
+    if (!$synchronized_properties) {
+      return;
+    }
+
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $original */
+    $original = $this->getOriginalEntity($entity);
+    $original_translation = $this->getOriginalTranslation($entity, $original);
+    if ($this->hasSynchronizedPropertyChanges($entity, $original_translation, $synchronized_properties)) {
+      if ($entity->isDefaultTranslationAffectedOnly()) {
+        foreach ($entity->getTranslationLanguages(FALSE) as $langcode => $language) {
+          if ($entity->getTranslation($langcode)->hasTranslationChanges()) {
+            $this->context->addViolation($constraint->defaultTranslationMessage);
+            break;
+          }
+        }
+      }
+      else {
+        $this->context->addViolation($constraint->defaultRevisionMessage);
+      }
+    }
+  }
+
+  /**
+   * Checks whether any synchronized property has changes.
+   *
+   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   *   The entity being validated.
+   * @param \Drupal\Core\Entity\ContentEntityInterface $original
+   *   The original unchanged entity.
+   * @param string[][] $synchronized_properties
+   *   An associative array of arrays of synchronized field properties keyed by
+   *   field name.
+   *
+   * @return bool
+   *   TRUE if changes in synchronized properties were detected, FALSE
+   *   otherwise.
+   */
+  protected function hasSynchronizedPropertyChanges(ContentEntityInterface $entity, ContentEntityInterface $original, array $synchronized_properties) {
+    foreach ($synchronized_properties as $field_name => $properties) {
+      foreach ($properties as $property) {
+        $items = $entity->get($field_name)->getValue();
+        $original_items = $original->get($field_name)->getValue();
+        if (count($items) !== count($original_items)) {
+          return TRUE;
+        }
+        foreach ($items as $delta => $item) {
+          // @todo This loose comparison is not fully reliable. Revisit this
+          //   after https://www.drupal.org/project/drupal/issues/2941092.
+          if ($items[$delta][$property] != $original_items[$delta][$property]) {
+            return TRUE;
+          }
+        }
+      }
+    }
+    return FALSE;
+  }
+
+  /**
+   * Returns the original unchanged entity to be used to detect changes.
+   *
+   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   *   The entity being changed.
+   *
+   * @return \Drupal\Core\Entity\ContentEntityInterface
+   *   The unchanged entity.
+   */
+  protected function getOriginalEntity(ContentEntityInterface $entity) {
+    if (!isset($entity->original)) {
+      $storage = $this->entityTypeManager->getStorage($entity->getEntityTypeId());
+      $original = $entity->isDefaultRevision() ? $storage->loadUnchanged($entity->id()) : $storage->loadRevision($entity->getLoadedRevisionId());
+    }
+    else {
+      $original = $entity->original;
+    }
+    return $original;
+  }
+
+  /**
+   * Returns the original translation.
+   *
+   * @param \Drupal\Core\Entity\ContentEntityInterface $entity
+   *   The entity being validated.
+   * @param \Drupal\Core\Entity\ContentEntityInterface $original
+   *   The original entity.
+   *
+   * @return \Drupal\Core\Entity\ContentEntityInterface
+   *   The original entity translation object.
+   */
+  protected function getOriginalTranslation(ContentEntityInterface $entity, ContentEntityInterface $original) {
+    $langcode = $entity->language()->getId();
+    if ($original->hasTranslation($langcode)) {
+      $original_langcode = $langcode;
+    }
+    else {
+      $metadata = $this->contentTranslationManager->getTranslationMetadata($entity);
+      $original_langcode = $metadata->getSource();
+    }
+    return $original->getTranslation($original_langcode);
+  }
+
+  /**
+   * Returns the synchronized properties for every specified field.
+   *
+   * @param \Drupal\Core\Field\FieldDefinitionInterface[] $field_definitions
+   *   An array of field definitions.
+   *
+   * @return string[][]
+   *   An associative array of arrays of field property names keyed by field
+   *   name.
+   */
+  public function getSynchronizedPropertiesByField(array $field_definitions) {
+    $synchronizer = $this->synchronizer;
+    $synchronized_properties = array_filter(array_map(
+      function (FieldDefinitionInterface $field_definition) use ($synchronizer) {
+        return $synchronizer->getFieldSynchronizedProperties($field_definition);
+      },
+      $field_definitions
+    ));
+    return $synchronized_properties;
+  }
+
+}

core/modules/content_translation/tests/modules/content_translation_test/content_translation_test.module

@@ -5,7 +5,10 @@
  * Helper module for the Content Translation tests.
  */
 
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Session\AccountInterface;
 
 /**
  * Implements hook_entity_bundle_info_alter().
@@ -23,6 +26,19 @@ function content_translation_test_entity_bundle_info_alter(&$bundles) {
   }
 }
 
+/**
+ * Implements hook_entity_access().
+ */
+function content_translation_test_entity_access(EntityInterface $entity, $operation, AccountInterface $account) {
+  $access = \Drupal::state()->get('content_translation.entity_access.' . $entity->getEntityTypeId());
+  if (!empty($access[$operation])) {
+    return AccessResult::allowed();
+  }
+  else {
+    return AccessResult::neutral();
+  }
+}
+
 /**
  * Implements hook_form_BASE_FORM_ID_alter().
  *

core/modules/content_translation/tests/src/Kernel/ContentTranslationSyncUnitTest.php

@@ -59,7 +59,7 @@ class ContentTranslationSyncUnitTest extends KernelTestBase {
   protected function setUp() {
     parent::setUp();
 
-    $this->synchronizer = new FieldTranslationSynchronizer($this->container->get('entity.manager'));
+    $this->synchronizer = new FieldTranslationSynchronizer($this->container->get('entity.manager'), $this->container->get('plugin.manager.field.field_type'));
     $this->synchronized = ['sync1', 'sync2'];
     $this->columns = array_merge($this->synchronized, ['var1', 'var2']);
     $this->langcodes = ['en', 'it', 'fr', 'de', 'es'];