From 7096493514178116b29794103522e28251ba6126 Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Wed, 11 Apr 2018 16:13:49 +0100
Subject: [PATCH] Issue #2919157 by benjy, amateescu, Sam152: Unable to convert
 a non-translatable entity into a revisionable entity

---
 ...SqlContentEntityStorageSchemaConverter.php |   4 +-
 ...rageSchemaConverterNonTranslatableTest.php |  23 +++
 ...tEntityStorageSchemaConverterTestBase.php} | 147 +++---------------
 ...StorageSchemaConverterTranslatableTest.php | 123 +++++++++++++++
 4 files changed, 173 insertions(+), 124 deletions(-)
 create mode 100644 core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterNonTranslatableTest.php
 rename core/modules/system/tests/src/Functional/Entity/Update/{SqlContentEntityStorageSchemaConverterTest.php => SqlContentEntityStorageSchemaConverterTestBase.php} (56%)
 create mode 100644 core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTranslatableTest.php

diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchemaConverter.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchemaConverter.php
index 2d3edfc6d49a..aa92e0f37cc7 100644
--- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchemaConverter.php
+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchemaConverter.php
@@ -296,7 +296,9 @@ protected function copyData(array &$sandbox) {
         // Set the 'revision_translation_affected' flag to TRUE to match the
         // previous API return value: if the field was not defined the value
         // returned was always TRUE.
-        $entity->set($revision_translation_affected_key, TRUE);
+        if ($temporary_entity_type->isTranslatable()) {
+          $entity->set($revision_translation_affected_key, TRUE);
+        }
 
         // Treat the entity as new in order to make the storage do an INSERT
         // rather than an UPDATE.
diff --git a/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterNonTranslatableTest.php b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterNonTranslatableTest.php
new file mode 100644
index 000000000000..e5ac6ed62ac2
--- /dev/null
+++ b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterNonTranslatableTest.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\Entity\Update;
+
+/**
+ * Tests converting a non-translatable entity type with data to revisionable.
+ *
+ * @group Entity
+ * @group Update
+ */
+class SqlContentEntityStorageSchemaConverterNonTranslatableTest extends SqlContentEntityStorageSchemaConverterTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles = [
+      __DIR__ . '/../../../../fixtures/update/drupal-8.0.0-rc1-filled.standard.entity_test_update.php.gz',
+      __DIR__ . '/../../../../fixtures/update/drupal-8.entity-test-schema-converter-enabled.php',
+    ];
+  }
+
+}
diff --git a/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTest.php b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTestBase.php
similarity index 56%
rename from core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTest.php
rename to core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTestBase.php
index 625ee190edb9..a0acaef7571c 100644
--- a/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTest.php
+++ b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTestBase.php
@@ -7,12 +7,9 @@
 use Drupal\system\Tests\Entity\EntityDefinitionTestTrait;
 
 /**
- * Tests updating an entity type with existing data to be revisionable.
- *
- * @group Entity
- * @group Update
+ * Defines a class for testing the conversion of entity types to revisionable.
  */
-class SqlContentEntityStorageSchemaConverterTest extends UpdatePathTestBase {
+abstract class SqlContentEntityStorageSchemaConverterTestBase extends UpdatePathTestBase {
 
   use EntityDefinitionTestTrait;
 
@@ -64,16 +61,6 @@ protected function setUp() {
     $this->state = \Drupal::state();
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function setDatabaseDumpFiles() {
-    $this->databaseDumpFiles = [
-      __DIR__ . '/../../../../fixtures/update/drupal-8.0.0-rc1-filled.standard.entity_test_update_mul.php.gz',
-      __DIR__ . '/../../../../fixtures/update/drupal-8.entity-test-schema-converter-enabled.php',
-    ];
-  }
-
   /**
    * Tests the conversion of an entity type to revisionable.
    */
@@ -83,17 +70,24 @@ public function testMakeRevisionable() {
     $entity_test_update = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
     $this->assertFalse($entity_test_update->isRevisionable());
 
-    // Make the entity type revisionable and translatable and run the updates.
-    $this->updateEntityTypeToRevisionableAndTranslatable();
+    $translatable = $entity_test_update->isTranslatable();
+
+    // Make the entity type revisionable and run the updates.
+    if ($translatable) {
+      $this->updateEntityTypeToRevisionableAndTranslatable();
+    }
+    else {
+      $this->updateEntityTypeToRevisionable();
+    }
 
     $this->runUpdates();
 
     /** @var \Drupal\Core\Entity\EntityTypeInterface $entity_test_update */
     $entity_test_update = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
-    $this->assertTrue($entity_test_update->isRevisionable());
-
     $field_storage_definitions = $this->lastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions('entity_test_update');
-    $this->assertTrue(isset($field_storage_definitions['revision_translation_affected']));
+
+    $this->assertTrue($entity_test_update->isRevisionable());
+    $this->assertEquals($translatable, isset($field_storage_definitions['revision_translation_affected']));
 
     /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
     $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update');
@@ -107,10 +101,6 @@ public function testMakeRevisionable() {
       $this->assertEqual($i, $revision->id());
       $this->assertEqual($i, $revision->getRevisionId());
 
-      // Check that the correct initial value was provided for the
-      // 'revision_translation_affected' field.
-      $this->assertTrue($revision->revision_translation_affected->value);
-
       $this->assertEqual($i . ' - test single property', $revision->test_single_property->value);
 
       $this->assertEqual($i . ' - test multiple properties - value1', $revision->test_multiple_properties->value1);
@@ -131,7 +121,16 @@ public function testMakeRevisionable() {
 
       $this->assertEqual($i . ' - test entity base field info', $revision->test_entity_base_field_info->value);
 
-      // Do the same checks for translated field values.
+      // Do the same checks for translated field values if the entity type is
+      // translatable.
+      if (!$translatable) {
+        continue;
+      }
+
+      // Check that the correct initial value was provided for the
+      // 'revision_translation_affected' field.
+      $this->assertTrue($revision->revision_translation_affected->value);
+
       $translation = $revision->getTranslation('ro');
 
       $this->assertEqual($i . ' - test single property - ro', $translation->test_single_property->value);
@@ -168,102 +167,4 @@ public function testMakeRevisionable() {
     }
   }
 
-  /**
-   * Tests that a failed "make revisionable" update preserves the existing data.
-   */
-  public function testMakeRevisionableErrorHandling() {
-    $original_entity_type = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
-    $original_storage_definitions = $this->lastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions('entity_test_update');
-
-    $original_entity_schema_data = $this->installedStorageSchema->get('entity_test_update.entity_schema_data', []);
-    foreach ($original_storage_definitions as $storage_definition) {
-      $original_field_schema_data[$storage_definition->getName()] = $this->installedStorageSchema->get('entity_test_update.field_schema_data.' . $storage_definition->getName(), []);
-    }
-
-    // Check that entity type is not revisionable prior to running the update
-    // process.
-    $this->assertFalse($original_entity_type->isRevisionable());
-
-    // Make the update throw an exception during the entity save process.
-    \Drupal::state()->set('entity_test_update.throw_exception', TRUE);
-
-    // Since the update process is interrupted by the exception thrown above,
-    // we can not do the full post update testing offered by UpdatePathTestBase.
-    $this->checkFailedUpdates = FALSE;
-
-    // Make the entity type revisionable and run the updates.
-    $this->updateEntityTypeToRevisionableAndTranslatable();
-
-    $this->runUpdates();
-
-    // Check that the update failed.
-    $this->assertRaw('<strong>' . t('Failed:') . '</strong>');
-
-    // Check that the last installed entity type definition is kept as
-    // non-revisionable.
-    $new_entity_type = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
-    $this->assertFalse($new_entity_type->isRevisionable(), 'The entity type is kept unchanged.');
-
-    // Check that the last installed field storage definitions did not change by
-    // looking at the 'langcode' field, which is updated automatically.
-    $new_storage_definitions = $this->lastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions('entity_test_update');
-    $langcode_key = $original_entity_type->getKey('langcode');
-    $this->assertEqual($original_storage_definitions[$langcode_key]->isRevisionable(), $new_storage_definitions[$langcode_key]->isRevisionable(), "The 'langcode' field is kept unchanged.");
-
-    /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
-    $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update');
-
-    // Check that installed storage schema did not change.
-    $new_entity_schema_data = $this->installedStorageSchema->get('entity_test_update.entity_schema_data', []);
-    $this->assertEqual($original_entity_schema_data, $new_entity_schema_data);
-
-    foreach ($new_storage_definitions as $storage_definition) {
-      $new_field_schema_data[$storage_definition->getName()] = $this->installedStorageSchema->get('entity_test_update.field_schema_data.' . $storage_definition->getName(), []);
-    }
-    $this->assertEqual($original_field_schema_data, $new_field_schema_data);
-
-    // Check that temporary tables have been removed.
-    $schema = \Drupal::database()->schema();
-    foreach ($storage->getTableMapping()->getTableNames() as $table_name) {
-      $this->assertFalse($schema->tableExists(TemporaryTableMapping::getTempTableName($table_name)));
-    }
-
-    // Check that the original tables still exist and their data is intact.
-    $this->assertTrue($schema->tableExists('entity_test_update'));
-    $this->assertTrue($schema->tableExists('entity_test_update_data'));
-
-    $base_table_count = \Drupal::database()->select('entity_test_update')
-      ->countQuery()
-      ->execute()
-      ->fetchField();
-    $this->assertEqual($base_table_count, 102);
-
-    $data_table_count = \Drupal::database()->select('entity_test_update_data')
-      ->countQuery()
-      ->execute()
-      ->fetchField();
-    // There are two records for each entity, one for English and one for
-    // Romanian.
-    $this->assertEqual($data_table_count, 204);
-
-    $base_table_row = \Drupal::database()->select('entity_test_update')
-      ->fields('entity_test_update')
-      ->condition('id', 1, '=')
-      ->condition('langcode', 'en', '=')
-      ->execute()
-      ->fetchAllAssoc('id');
-    $this->assertEqual('843e9ac7-3351-4cc1-a202-2dbffffae21c', $base_table_row[1]->uuid);
-
-    $data_table_row = \Drupal::database()->select('entity_test_update_data')
-      ->fields('entity_test_update_data')
-      ->condition('id', 1, '=')
-      ->condition('langcode', 'en', '=')
-      ->execute()
-      ->fetchAllAssoc('id');
-    $this->assertEqual('1 - test single property', $data_table_row[1]->test_single_property);
-    $this->assertEqual('1 - test multiple properties - value1', $data_table_row[1]->test_multiple_properties__value1);
-    $this->assertEqual('1 - test multiple properties - value2', $data_table_row[1]->test_multiple_properties__value2);
-    $this->assertEqual('1 - test entity base field info', $data_table_row[1]->test_entity_base_field_info);
-  }
-
 }
diff --git a/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTranslatableTest.php b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTranslatableTest.php
new file mode 100644
index 000000000000..df734988873e
--- /dev/null
+++ b/core/modules/system/tests/src/Functional/Entity/Update/SqlContentEntityStorageSchemaConverterTranslatableTest.php
@@ -0,0 +1,123 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\Entity\Update;
+
+use Drupal\Core\Entity\Sql\TemporaryTableMapping;
+
+/**
+ * Tests converting a translatable entity type with data to revisionable.
+ *
+ * @group Entity
+ * @group Update
+ */
+class SqlContentEntityStorageSchemaConverterTranslatableTest extends SqlContentEntityStorageSchemaConverterTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles = [
+      __DIR__ . '/../../../../fixtures/update/drupal-8.0.0-rc1-filled.standard.entity_test_update_mul.php.gz',
+      __DIR__ . '/../../../../fixtures/update/drupal-8.entity-test-schema-converter-enabled.php',
+    ];
+  }
+
+  /**
+   * Tests that a failed "make revisionable" update preserves the existing data.
+   */
+  public function testMakeRevisionableErrorHandling() {
+    $original_entity_type = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
+    $original_storage_definitions = $this->lastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions('entity_test_update');
+
+    $original_entity_schema_data = $this->installedStorageSchema->get('entity_test_update.entity_schema_data', []);
+    foreach ($original_storage_definitions as $storage_definition) {
+      $original_field_schema_data[$storage_definition->getName()] = $this->installedStorageSchema->get('entity_test_update.field_schema_data.' . $storage_definition->getName(), []);
+    }
+
+    // Check that entity type is not revisionable prior to running the update
+    // process.
+    $this->assertFalse($original_entity_type->isRevisionable());
+
+    // Make the update throw an exception during the entity save process.
+    \Drupal::state()->set('entity_test_update.throw_exception', TRUE);
+
+    // Since the update process is interrupted by the exception thrown above,
+    // we can not do the full post update testing offered by UpdatePathTestBase.
+    $this->checkFailedUpdates = FALSE;
+
+    // Make the entity type revisionable and run the updates.
+    $this->updateEntityTypeToRevisionableAndTranslatable();
+
+    $this->runUpdates();
+
+    // Check that the update failed.
+    $this->assertRaw('<strong>' . t('Failed:') . '</strong>');
+
+    // Check that the last installed entity type definition is kept as
+    // non-revisionable.
+    $new_entity_type = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
+    $this->assertFalse($new_entity_type->isRevisionable(), 'The entity type is kept unchanged.');
+
+    // Check that the last installed field storage definitions did not change by
+    // looking at the 'langcode' field, which is updated automatically.
+    $new_storage_definitions = $this->lastInstalledSchemaRepository->getLastInstalledFieldStorageDefinitions('entity_test_update');
+    $langcode_key = $original_entity_type->getKey('langcode');
+    $this->assertEqual($original_storage_definitions[$langcode_key]->isRevisionable(), $new_storage_definitions[$langcode_key]->isRevisionable(), "The 'langcode' field is kept unchanged.");
+
+    /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
+    $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update');
+
+    // Check that installed storage schema did not change.
+    $new_entity_schema_data = $this->installedStorageSchema->get('entity_test_update.entity_schema_data', []);
+    $this->assertEqual($original_entity_schema_data, $new_entity_schema_data);
+
+    foreach ($new_storage_definitions as $storage_definition) {
+      $new_field_schema_data[$storage_definition->getName()] = $this->installedStorageSchema->get('entity_test_update.field_schema_data.' . $storage_definition->getName(), []);
+    }
+    $this->assertEqual($original_field_schema_data, $new_field_schema_data);
+
+    // Check that temporary tables have been removed.
+    $schema = \Drupal::database()->schema();
+    foreach ($storage->getTableMapping()->getTableNames() as $table_name) {
+      $this->assertFalse($schema->tableExists(TemporaryTableMapping::getTempTableName($table_name)));
+    }
+
+    // Check that the original tables still exist and their data is intact.
+    $this->assertTrue($schema->tableExists('entity_test_update'));
+    $this->assertTrue($schema->tableExists('entity_test_update_data'));
+
+    $base_table_count = \Drupal::database()->select('entity_test_update')
+      ->countQuery()
+      ->execute()
+      ->fetchField();
+    $this->assertEqual($base_table_count, 102);
+
+    $data_table_count = \Drupal::database()->select('entity_test_update_data')
+      ->countQuery()
+      ->execute()
+      ->fetchField();
+    // There are two records for each entity, one for English and one for
+    // Romanian.
+    $this->assertEqual($data_table_count, 204);
+
+    $base_table_row = \Drupal::database()->select('entity_test_update')
+      ->fields('entity_test_update')
+      ->condition('id', 1, '=')
+      ->condition('langcode', 'en', '=')
+      ->execute()
+      ->fetchAllAssoc('id');
+    $this->assertEqual('843e9ac7-3351-4cc1-a202-2dbffffae21c', $base_table_row[1]->uuid);
+
+    $data_table_row = \Drupal::database()->select('entity_test_update_data')
+      ->fields('entity_test_update_data')
+      ->condition('id', 1, '=')
+      ->condition('langcode', 'en', '=')
+      ->execute()
+      ->fetchAllAssoc('id');
+    $this->assertEqual('1 - test single property', $data_table_row[1]->test_single_property);
+    $this->assertEqual('1 - test multiple properties - value1', $data_table_row[1]->test_multiple_properties__value1);
+    $this->assertEqual('1 - test multiple properties - value2', $data_table_row[1]->test_multiple_properties__value2);
+    $this->assertEqual('1 - test entity base field info', $data_table_row[1]->test_entity_base_field_info);
+  }
+
+}
-- 
GitLab