Use initial values for content translation metadata fields and fix existing data

Just walk by review.

1. +++ b/core/modules/content_translation/content_translation.install
   @@ -62,3 +64,64 @@ function content_translation_update_8002() {
   +      $entity_type = \Drupal::entityDefinitionUpdateManager()->getEntityType($entity_type_id);
   

   EDUM can be stored in a variable outside of the loop.

2. +++ b/core/modules/content_translation/content_translation.install
   @@ -62,3 +64,64 @@ function content_translation_update_8002() {
   +            ->fields(['content_translation_outdated' => 0])
   ...
   +            ->fields(['content_translation_status' => 1])
   
   +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -117,14 +117,16 @@ public function getFieldDefinitions() {
   +      ->setInitialValue(0);
   
   @@ -143,7 +145,8 @@ public function getFieldDefinitions() {
   +        ->setInitialValue(1);
   

   Should we use constants here?

Thank you for addressing the feedback.
#8.2: I think EntityPublishedInterface should add those constants instead of NodeInterface and should be used here. Toughts?

#2856808: Break out the 'entity_test_update' entity type into its own module and add additional test db dumps is fixed.

I think it is only blocked on #2346019: Handle initial values when creating a new field storage definition which is blocked on #2841291: Fix NOT NULL handling in the entity storage and 'primary key' changes when updating the storage definition of an identifier field so postpone by 2.

Struggling to find any issues.

Yeah, I don't know this code, but I have to say that this patch looks super solid :)

Awesome, thanks! Needs work for 3. only.

1. +++ b/core/modules/content_translation/content_translation.install
   @@ -44,3 +46,57 @@ function content_translation_update_8001() {
   +      $storage_definitions = $last_installed_schema_repository->getLastInstalledFieldStorageDefinitions($entity_type_id);
   ...
   +        if (isset($storage_definitions['content_translation_source'])) {
   ...
   +        if (isset($storage_definitions['content_translation_outdated'])) {
   ...
   +        if (isset($storage_definitions['content_translation_status'])) {
   

   Minor, but you could avoid having to fetch the schema repository manually by doing $entity_definition_update_manager->getFieldStorageDefinition(). I don't feel strongly about this, though, so feel free to leave as is.

2. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -117,14 +117,16 @@ public function getFieldDefinitions() {
          ->setDefaultValue(LanguageInterface::LANGCODE_NOT_SPECIFIED)
          ->setRevisionable(TRUE)
   -      ->setTranslatable(TRUE);
   +      ->setTranslatable(TRUE)
   +      ->setInitialValue(LanguageInterface::LANGCODE_NOT_SPECIFIED);
   

   Minor, but any reason not to put the initial value directly after the default value?

3. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -117,14 +117,16 @@ public function getFieldDefinitions() {
   +      ->setTranslatable(TRUE)
   +      ->setInitialValue(0);
   
   @@ -143,7 +145,8 @@ public function getFieldDefinitions() {
   +        ->setTranslatable(TRUE)
   +        ->setInitialValue(1);
   

   Let's use TRUE for the initial value, as well.

Perfect, thank you!

That seems to not work:

General error: 1366 Incorrect integer value: '' for column 'content_translation_outdated'

+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
@@ -1712,7 +1712,7 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st
       if ($initial_value && isset($initial_value[$field_column_name])) {
...
+        $schema['fields'][$schema_field_name]['initial'] = drupal_schema_get_field_value($column_schema, $initial_value[$field_column_name]);
       }
       elseif (!empty($initial_value_from_field)) {
         $schema['fields'][$schema_field_name]['initial_from_field'] = $initial_value_from_field[$field_column_name];

Nice fix. I thought whether the elseif-branch should get the same treatment, and I think it should. But I also found it strange that the conditions are set up differently - i.e. I would have expected the elseif to be $initial_valu_from_field && isset($initial_value_from_field[$field_column_name]). But that's a preexisting issue so shouldn't be tackled here, and, thus, maybe we should leave that entire branch alone. Not sure, so leaving at Needs review.

It looks as though we're ready to go back to RTBC.

Wow, thank you for #30!!! That makes a lot of sense, totally missed that the two are inherently different beasts. Awesome explanation. RTBC++

Back to RTBC then!

+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
@@ -1712,7 +1712,7 @@ protected function getSharedTableFieldSchema(FieldStorageDefinitionInterface $st
+        $schema['fields'][$schema_field_name]['initial'] = drupal_schema_get_field_value($column_schema, $initial_value[$field_column_name]);

Had to double check drupal_schema_get_field_value() wasn't deprecated, it isn't. Issue for that is #2124069: Convert schema.inc to the update.update_hook_registry service (UpdateHookRegistry).

Patch looks great, couldn't find anything to complain about. Committed/pushed to 8.4.x, thanks!