Issue #2982759 by amateescu: EntityUpdateToRevisionableAndPublishableTest wrongly assumes entities should be converted to revisionable and publishable in a single step

core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php

@@ -333,11 +333,6 @@ class Schema extends DatabaseSchema {
       $this->connection->query($query);
 
       // Apply the initial value if set.
-      if (isset($specification['initial'])) {
-        $this->connection->update($table)
-          ->fields([$field => $specification['initial']])
-          ->execute();
-      }
       if (isset($specification['initial_from_field'])) {
         if (isset($specification['initial'])) {
           $expression = 'COALESCE(' . $specification['initial_from_field'] . ', :default_initial_value)';
@@ -351,6 +346,11 @@ class Schema extends DatabaseSchema {
           ->expression($field, $expression, $arguments)
           ->execute();
       }
+      elseif (isset($specification['initial'])) {
+        $this->connection->update($table)
+          ->fields([$field => $specification['initial']])
+          ->execute();
+      }
     }
     else {
       // We cannot add the field directly. Use the slower table alteration
@@ -363,14 +363,7 @@ class Schema extends DatabaseSchema {
 
       // Build the mapping between the old fields and the new fields.
       $mapping = [];
-      if (isset($specification['initial'])) {
-        // If we have a initial value, copy it over.
-        $mapping[$field] = [
-          'expression' => ':newfieldinitial',
-          'arguments' => [':newfieldinitial' => $specification['initial']],
-        ];
-      }
-      elseif (isset($specification['initial_from_field'])) {
+      if (isset($specification['initial_from_field'])) {
         // If we have a initial value, copy it over.
         if (isset($specification['initial'])) {
           $expression = 'COALESCE(' . $specification['initial_from_field'] . ', :default_initial_value)';
@@ -385,6 +378,13 @@ class Schema extends DatabaseSchema {
           'arguments' => $arguments,
         ];
       }
+      elseif (isset($specification['initial'])) {
+        // If we have a initial value, copy it over.
+        $mapping[$field] = [
+          'expression' => ':newfieldinitial',
+          'arguments' => [':newfieldinitial' => $specification['initial']],
+        ];
+      }
       else {
         // Else use the default of the field.
         $mapping[$field] = NULL;

core/modules/system/tests/modules/entity_test_update/update/entity_rev_pub_updates_8400.inc

@@ -8,41 +8,17 @@
 use Drupal\Core\Field\BaseFieldDefinition;
 
 /**
- * Implements hook_update_dependencies().
- */
-function entity_test_update_update_dependencies() {
-  // The update function that adds the status field must run after
-  // content_translation_update_8400() which fixes NULL values for the
-  // 'content_translation_status' field.
-  $dependencies['entity_test_update'][8400] = [
-    'content_translation' => 8400,
-  ];
-
-  return $dependencies;
-}
-
-/**
- * Add the 'published' and revisionable metadata fields to entity_test_update.
+ * Add the 'published' entity key to entity_test_update.
  */
 function entity_test_update_update_8400() {
   $definition_update_manager = \Drupal::entityDefinitionUpdateManager();
 
-  // Add the published entity key and revisionable metadata fields to the
-  // entity_test_update entity type.
+  // Add the published entity key to the entity_test_update entity type.
   $entity_type = $definition_update_manager->getEntityType('entity_test_update');
 
   $entity_keys = $entity_type->getKeys();
   $entity_keys['published'] = 'status';
   $entity_type->set('entity_keys', $entity_keys);
-
-  $revision_metadata_keys = [
-    'revision_user' => 'revision_user',
-    'revision_created' => 'revision_created',
-    'revision_log_message' => 'revision_log_message',
-    'revision_default' => 'revision_default',
-  ];
-  $entity_type->set('revision_metadata_keys', $revision_metadata_keys);
-
   $definition_update_manager->updateEntityType($entity_type);
 
   // Add the status field.
@@ -55,41 +31,13 @@ function entity_test_update_update_8400() {
 
   $has_content_translation_status_field = \Drupal::moduleHandler()->moduleExists('content_translation') && $definition_update_manager->getFieldStorageDefinition('content_translation_status', 'entity_test_update');
   if ($has_content_translation_status_field) {
-    $status->setInitialValueFromField('content_translation_status');
+    $status->setInitialValueFromField('content_translation_status', TRUE);
   }
   else {
     $status->setInitialValue(TRUE);
   }
   $definition_update_manager->installFieldStorageDefinition('status', 'entity_test_update', 'entity_test_update', $status);
 
-  // Add the revision metadata fields.
-  $revision_created = BaseFieldDefinition::create('created')
-    ->setLabel(t('Revision create time'))
-    ->setDescription(t('The time that the current revision was created.'))
-    ->setRevisionable(TRUE);
-  $definition_update_manager->installFieldStorageDefinition('revision_created', 'entity_test_update', 'entity_test_update', $revision_created);
-
-  $revision_user = BaseFieldDefinition::create('entity_reference')
-    ->setLabel(t('Revision user'))
-    ->setDescription(t('The user ID of the author of the current revision.'))
-    ->setSetting('target_type', 'user')
-    ->setRevisionable(TRUE);
-  $definition_update_manager->installFieldStorageDefinition('revision_user', 'entity_test_update', 'entity_test_update', $revision_user);
-
-  $revision_log_message = BaseFieldDefinition::create('string_long')
-    ->setLabel(t('Revision log message'))
-    ->setDescription(t('Briefly describe the changes you have made.'))
-    ->setRevisionable(TRUE)
-    ->setDefaultValue('')
-    ->setDisplayOptions('form', [
-      'type' => 'string_textarea',
-      'weight' => 25,
-      'settings' => [
-        'rows' => 4,
-      ],
-    ]);
-  $definition_update_manager->installFieldStorageDefinition('revision_log_message', 'entity_test_update', 'entity_test_update', $revision_log_message);
-
   // Uninstall the 'content_translation_status' field if needed.
   $database = \Drupal::database();
   if ($has_content_translation_status_field) {

core/modules/system/tests/src/Functional/Update/EntityUpdateToPublishableTest.php

@@ -7,14 +7,12 @@ use Drupal\FunctionalTests\Update\UpdatePathTestBase;
 use Drupal\Tests\system\Functional\Entity\Traits\EntityDefinitionTestTrait;
 
 /**
- * Tests the upgrade path for making an entity revisionable and publishable.
- *
- * @see https://www.drupal.org/node/2841291
+ * Tests the upgrade path for making an entity publishable.
  *
  * @group Update
  * @group legacy
  */
-class EntityUpdateToRevisionableAndPublishableTest extends UpdatePathTestBase {
+class EntityUpdateToPublishableTest extends UpdatePathTestBase {
 
   use EntityDefinitionTestTrait;
   use DbUpdatesTrait;
@@ -73,23 +71,21 @@ class EntityUpdateToRevisionableAndPublishableTest extends UpdatePathTestBase {
   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 and publishable.
+   * Tests the conversion of an entity type to be publishable.
    *
    * @see entity_test_update_update_8400()
    */
-  public function testConvertToRevisionableAndPublishable() {
-    // Check that entity type is not revisionable nor publishable prior to
-    // running the update process.
+  public function testConvertToPublishable() {
+    // Check that entity type is not publishable prior to running the update
+    // process.
     $entity_test_update = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
-    $this->assertFalse($entity_test_update->isRevisionable());
     $this->assertFalse($entity_test_update->getKey('published'));
 
-    // Make the entity type revisionable, translatable and publishable.
+    // Make the entity type translatable and publishable.
     $this->updateEntityTypeDefinition();
 
     $this->enableUpdates('entity_test_update', 'entity_rev_pub_updates', 8400);
@@ -97,84 +93,65 @@ class EntityUpdateToRevisionableAndPublishableTest extends UpdatePathTestBase {
 
     /** @var \Drupal\Core\Entity\EntityTypeInterface $entity_test_update */
     $entity_test_update = $this->lastInstalledSchemaRepository->getLastInstalledDefinition('entity_test_update');
-    $this->assertTrue($entity_test_update->isRevisionable());
-    $this->assertEqual('status', $entity_test_update->getKey('published'));
+    $this->assertEquals('status', $entity_test_update->getKey('published'));
 
     /** @var \Drupal\Core\Entity\Sql\SqlEntityStorageInterface $storage */
     $storage = \Drupal::entityTypeManager()->getStorage('entity_test_update');
-    $this->assertEqual(count($storage->loadMultiple()), 102, 'All test entities were found.');
-
-    // The conversion to revisionable is already tested by
-    // \Drupal\system\Tests\Entity\Update\SqlContentEntityStorageSchemaConverterTest::testMakeRevisionable()
-    // so we only need to check that some special cases are handled.
-    // All the checks implemented here are taking into consideration the special
-    // conditions in which the test database was created.
-    // @see _entity_test_update_create_test_entities()
+    $this->assertCount(102, $storage->loadMultiple(), 'All test entities were found.');
 
     // The test entity with ID 50 was created before Content Translation was
     // enabled, which means it didn't have a 'content_translation_status' field.
     // content_translation_update_8400() added values for that field which
     // should now be reflected in the entity's 'status' field.
-    /** @var \Drupal\Core\Entity\ContentEntityInterface $revision */
-    $revision = $storage->loadRevision(50);
-    $this->assertEqual(1, $revision->status->value);
+    /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
+    $entity = $storage->load(50);
+    $this->assertEquals(1, $entity->status->value);
 
-    $translation = $revision->getTranslation('ro');
-    $this->assertEqual(1, $translation->status->value);
+    $translation = $entity->getTranslation('ro');
+    $this->assertEquals(1, $translation->status->value);
 
     // The test entity with ID 100 was created with Content Translation enabled
     // and it should have the same values as entity 50.
-    $revision = $storage->loadRevision(100);
-    $this->assertEqual(1, $revision->status->value);
+    $entity = $storage->load(100);
+    $this->assertEquals(1, $entity->status->value);
 
-    $translation = $revision->getTranslation('ro');
-    $this->assertEqual(1, $translation->status->value);
+    $translation = $entity->getTranslation('ro');
+    $this->assertEquals(1, $translation->status->value);
 
     // The test entity 101 had 'content_translation_status' set to 0 for the
     // English (source) language.
-    $revision = $storage->loadRevision(101);
-    $this->assertEqual(0, $revision->status->value);
+    $entity = $storage->load(101);
+    $this->assertEquals(0, $entity->status->value);
 
-    $translation = $revision->getTranslation('ro');
-    $this->assertEqual(1, $translation->status->value);
+    $translation = $entity->getTranslation('ro');
+    $this->assertEquals(1, $translation->status->value);
 
     // The test entity 102 had 'content_translation_status' set to 0 for the
     // Romanian language.
-    $revision = $storage->loadRevision(102);
-    $this->assertEqual(1, $revision->status->value);
+    $entity = $storage->load(102);
+    $this->assertEquals(1, $entity->status->value);
 
-    $translation = $revision->getTranslation('ro');
-    $this->assertEqual(0, $translation->status->value);
+    $translation = $entity->getTranslation('ro');
+    $this->assertEquals(0, $translation->status->value);
   }
 
   /**
-   * Updates the 'entity_test_update' entity type to revisionable,
-   * translatable, publishable and adds revision metadata keys.
+   * Updates the 'entity_test_update' entity type to translatable and
+   * publishable.
    */
   protected function updateEntityTypeDefinition() {
     $entity_type = clone $this->entityTypeManager->getDefinition('entity_test_update');
 
     $keys = $entity_type->getKeys();
-    $keys['revision'] = 'revision_id';
     $keys['published'] = 'status';
     $entity_type->set('entity_keys', $keys);
 
-    $revision_metadata_keys = [
-      'revision_user' => 'revision_user',
-      'revision_created' => 'revision_created',
-      'revision_log_message' => 'revision_log_message',
-      'revision_default' => 'revision_default',
-    ];
-    $entity_type->set('revision_metadata_keys', $revision_metadata_keys);
-
     $entity_type->set('translatable', TRUE);
     $entity_type->set('data_table', 'entity_test_update_data');
-    $entity_type->set('revision_table', 'entity_test_update_revision');
-    $entity_type->set('revision_data_table', 'entity_test_update_revision_data');
 
     $this->state->set('entity_test_update.entity_type', $entity_type);
 
-    // Also add the status and revision metadata base fields to the entity type.
+    // Add the status field to the entity type.
     $status = BaseFieldDefinition::create('boolean')
       ->setLabel(t('Publishing status'))
       ->setDescription(t('A boolean indicating the published state.'))
@@ -183,35 +160,8 @@ class EntityUpdateToRevisionableAndPublishableTest extends UpdatePathTestBase {
       ->setRequired(TRUE)
       ->setDefaultValue(TRUE);
 
-    $revision_created = BaseFieldDefinition::create('created')
-      ->setLabel(t('Revision create time'))
-      ->setDescription(t('The time that the current revision was created.'))
-      ->setRevisionable(TRUE);
-
-    $revision_user = BaseFieldDefinition::create('entity_reference')
-      ->setLabel(t('Revision user'))
-      ->setDescription(t('The user ID of the author of the current revision.'))
-      ->setSetting('target_type', 'user')
-      ->setRevisionable(TRUE);
-
-    $revision_log_message = BaseFieldDefinition::create('string_long')
-      ->setLabel(t('Revision log message'))
-      ->setDescription(t('Briefly describe the changes you have made.'))
-      ->setRevisionable(TRUE)
-      ->setDefaultValue('')
-      ->setDisplayOptions('form', [
-        'type' => 'string_textarea',
-        'weight' => 25,
-        'settings' => [
-          'rows' => 4,
-        ],
-      ]);
-
     $this->state->set('entity_test_update.additional_base_field_definitions', [
       'status' => $status,
-      'revision_created' => $revision_created,
-      'revision_user' => $revision_user,
-      'revision_log_message' => $revision_log_message,
     ]);
 
     $this->entityTypeManager->clearCachedDefinitions();