Problem/Motivation

As @timmillwood and @alexpott have consistently warned, content moderation is experimental and has no upgrade path. Installing and using any experimental version of this module in production is discouraged and probably a really bad idea… however those of us who understood the risks and did it anyway will need an upgrade path.

Issues tagged content_moderation upgrade path

Proposed resolution

I don’t think an upgrade path should be added to the module. It would require fixing and updating every time a schema break was introduced, which would slow down the path to beta and probably create bad expectations that it would “just work".

Instead I propose using this issue to host possible upgrade path patches to allow those who need an upgrade path to collaborate. A bit like HEAD to HEAD for CM, use at your own risk etc.

Using this patch

  1. Upgrade core from 8.2.x to 8.3.x and apply the patch.
  2. Run drush updb.
  3. Export your sites configuration (if applicable).
  4. Remove the patch and dial back the content_moderation schema version using: drush ev "drupal_set_installed_schema_version('content_moderation', 8000);"

Some of your sites code and configuration will need some updates to work with the changes made to content moderation:

  • You'll need to reassign the content moderation permissions for the transitions each role is allowed to use.
  • Custom code references to the moderation state in the form of $entity->moderation_state->entity->id() will need to be updated to be in the form of $entity->moderation_state->value
  • Loading custom states and transitions like ModerationState::loadMultiple() will need to be updated to $workflow = Workflow::load('content_moderation_workflow'); $workflow->getStates();
CommentFileSizeAuthor
#29 unofficial-2846618-29.patch629.21 KBnlisgo
#29 interdiff-2846618-22-29.txt1 KBnlisgo
#23 intertiff-2846618-6-16.txt3.3 KBbmcclure
#22 2846618-82-83-upgrade-path-22.patch628.95 KBSam152
#22 interdiff.txt1.1 KBSam152
#16 2846618-82-83-upgrade-path-16.patch628.61 KBbmcclure
#12 2846618-82-83-upgrade-path-12.patch629 KBbmcclure
#7 2846618-82-83-upgrade-path-6.patch628.88 KBSam152
#7 interdiff.txt2.09 KBSam152
#4 2846618-82-83-upgrade-path-4.patch628.44 KBSam152
#4 interdiff.txt803 bytesSam152
#2 2846618-82-83-upgrade-path-2.patch627.65 KBSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Active » Needs review
FileSize
2846618-82-83-upgrade-path-2.patch627.65 KB

Working:

  • Upgrades the state and transition config to a workflow entity.
  • Upgrade preserves the state of all content revisions.

Not working or @todo:

  • Changes to views relationships and fields.
  • Batching the mass content update.
  • Multiple workflow entities based on different combinations of enabled states on bundles.

Status: Needs review » Needs work

The last submitted patch, 2: 2846618-82-83-upgrade-path-2.patch, failed testing.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs work » Needs review
FileSize
interdiff.txt803 bytes
2846618-82-83-upgrade-path-4.patch628.44 KB

Did hit that error a few times locally, but not towards the end, will see if this fixes it. Given this isn't going to be committed, I don't mind putting hacks in there to make the upgrade possible. The patch will be reverted as soon as the updb runs.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs review » Needs work 
+++ b/core/modules/content_moderation/content_moderation.install
@@ -0,0 +1,149 @@
+      list($entity_type_id,,$bundle_id) = explode('.', $bundle_config_id);

Ran this upgrade path on a project. Worked as expected but $entity_type_id here is actually the module providing the entity type, not the entity type ID itself. Works for for "node" obviously, but not for modules providing entity types where entity type ID !== module name.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs work » Needs review
FileSize
interdiff.txt2.09 KB
2846618-82-83-upgrade-path-6.patch628.88 KB

Fix attached.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue summary: View changes
bmcclure’s picture

bmcclure CreditAttribution: bmcclure commented

Just wanted to say thanks for these patches, this is incredibly helpful for us! It worked for me so far, but I'll report back if I notice anything.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure commented

One issue I ran into:

In my site, I was unable to get to the point of being able to run database updates without first enabling the workflows module. I didn't realize it at the time, but the fact that workflows was already enabled completely prevented the update hook from doing anything.

I ended up commenting out the first few lines in the update hook because I needed it to run even though workflows was enabled.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure commented

What I've been working on for a little while now is that, during this update hook, I'm receiving the error:

Unable to delete a field (moderation_state in content_moderation_state entity) with data that cannot be purged.

It's strange, since there's code in the update hook that removes those values before trying to delete the field, but somehow there must be left-over data.

I even tried deleting it from the tables directly right before deleting the field:

// Force remove any left over moderation state field data
  /** @var \Drupal\content_moderation\ModerationInformation $moderation_information */
  $moderation_information = \Drupal::service('content_moderation.moderation_information');
  $tables_to_update = [];
  foreach (\Drupal::entityTypeManager()->getDefinitions() as $entity_type) {
    if ($moderation_information->canModerateEntitiesOfEntityType($entity_type)) {
      $tables_to_update[] = $entity_type->getDataTable();
      $tables_to_update[] = $entity_type->getRevisionDataTable();
    }
  }
  $db_connection = Database::getConnection();
  foreach ($tables_to_update as $table) {
    if ($table && $db_connection->schema()->fieldExists($table, 'moderation_state')) {
      $db_connection->update($table)->fields(['moderation_state' => NULL])->execute();
    }
  }

But I still run into the same issue. If I can figure out what's causing this for me, I'll post an updated patch.

Update: I realized what I'm doing wrong--my additional code was looking in the tables of the moderatable content types, rather than the moderation state entity's tables. I think I've got a working version, testing now then I'll post.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented
FileSize
2846618-82-83-upgrade-path-12.patch629 KB

Here's an updated patch that works for me.

It first collects all of the required state data like before, but instead of clearing it out at the same time, it goes back afterward to update every row that has a value for that field in the tables to make sure there is nothing left before removing the field.

This allowed the update hook to complete successfully for me on a large site with a lot of existing revision data.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented

Found additional issue after the fact: Somehow the entity type for content moderation is being changed in the install hook from "node" to "block_content" and I'm this experiencing many issues after the upgrade.

Trying to investigate how this entity type could be wrong in the config. It seems like it must be related to the last fix in the patch version before I posted mine which changes how the entity type is determined, however I don't know why it's returning something wrong yet.

Status: Needs review » Needs work

The last submitted patch, 12: 2846618-82-83-upgrade-path-12.patch, failed testing.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented

It's because the patch is comparing based on $bundle_config_prefix, which for both node and block_content is "type". Trying to figure out a way around that now.

Are you sure that the first value in the config name isn't the entity type? If not, I don't think the entity type is listed anywhere in the bundle config which would be strange IMO. I'm going to try and verify what the best way to figure out the entity type from a bundle config entity is.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented
FileSize
2846618-82-83-upgrade-path-16.patch628.61 KB

I ended up reverting to using the first part of the config name as the entity type as it was more reliable than using the bundle config prefix. However there may be a better way altogether.

The tests may fail, I haven't touched them.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language Malayalam
Location Manchester
CreditAttribution: rakesh.gectcr as a volunteer and at Valuebound commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 16: 2846618-82-83-upgrade-path-16.patch, failed testing.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Hi bmcclure, thanks for working on this, glad you were able to use the patch. It would be great if you could provide an interdiff with your patches, so everyone can see what changed between them.

It would also be great to extend the test coverage with whatever bugs you encounter.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Are you sure that the first value in the config name isn't the entity type? If not, I don't think the entity type is listed anywhere in the bundle config which would be strange IMO. I'm going to try and verify what the best way to figure out the entity type from a bundle config entity is.

Makes sense, so we need a better way to figure this out.

I ended up reverting to using the first part of the config name as the entity type as it was more reliable than using the bundle config prefix. However there may be a better way altogether.

This didn't work in my case, although it did work for node and block_content as you suggest.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Thinking about this, I think the correct fix would be to get list($provider_of_the_entity_type, $config_prefix,) = explode..., then check the entity type ID against both the config prefix and the provider.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.1 KB
2846618-82-83-upgrade-path-22.patch628.95 KB

Lets see if this goes green and fixes bmcclure's issue at the same time.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented
FileSize
intertiff-2846618-6-16.txt3.3 KB

I think that solves that last issue I had. However one thing that still occurs with my dataset using this latest patch is it doesn't actually clear out every value of the workflow_state field, only most of them. In particular there are some left-over values in the main field data (not revision data) table.

In my last patch, the way I solved this was to split up the storing of needed values for later, and clearing of existing values:

// Before uninstalling the old moderation state field, build a list of
  // content_moderation_state revisions mapped to states.
  $state_map = [];
  $content_moderation_state_revisions = \Drupal::entityQuery('content_moderation_state')
    ->allRevisions()
    ->execute();
  $content_moderation_state_storage = $entity_type_manager->getStorage('content_moderation_state');
  foreach ($content_moderation_state_revisions as $revision_id => $id) {
    $content_moderation_state = $content_moderation_state_storage->loadRevision($revision_id);
    $state_map[$revision_id] = $content_moderation_state->moderation_state->value;
  }

  // Remove existing moderation state field data
  $definition = \Drupal::entityTypeManager()->getDefinition('content_moderation_state');
  $db_connection = Database::getConnection();
  foreach ([$definition->getDataTable(), $definition->getRevisionDataTable()] as $table) {
    $db_connection->update($table)->fields(['moderation_state' => NULL])->execute();
  }

By actually just querying the table to clear those values, it ensured I wouldn't get an error about there being non-purgable data in the field. I've attached an interdiff between my last patch and your previous patch before that to show the differences there more clearly.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Thanks for the interdiff! Much easier to review this way. 

+++ b/core/modules/content_moderation/content_moderation.install
@@ -0,0 +1,160 @@
+  // Before uninstalling the old moderation state field, build a list of
+  // content_moderation_state revisions mapped to states.
+  $state_map = [];
+  $content_moderation_state_revisions = \Drupal::entityQuery('content_moderation_state')
+    ->allRevisions()
+    ->execute();
+  $content_moderation_state_storage = $entity_type_manager->getStorage('content_moderation_state');
+  foreach ($content_moderation_state_revisions as $revision_id => $id) {
+    $content_moderation_state = $content_moderation_state_storage->loadRevision($revision_id);
+    $state_map[$revision_id] = $content_moderation_state->moderation_state->value;
+    // Data must be cleared from the column
+    $content_moderation_state->moderation_state = NULL;
+    ContentModerationState::updateOrCreateFromEntity($content_moderation_state);
+  }

I wonder what about this causes some of the field values to be left over? Maybe the entity query needs accessCheck(false).

+++ b/core/modules/content_moderation/content_moderation.install
@@ -6,15 +6,18 @@
+  $entity_definition_update_manager = \Drupal::entityDefinitionUpdateManager();
+  $old_moderation_state_field = $entity_definition_update_manager->getFieldStorageDefinition('moderation_state', 'content_moderation_state');
+
+  // Check if we're already using the new schema
+  if ($old_moderation_state_field->getType() == 'string') {
     return;
   }

This makes a lot of sense, I like this.

+++ b/core/modules/content_moderation/content_moderation.install
@@ -54,17 +56,7 @@
-      list(,$bundle_config_prefix,$bundle_id) = explode('.', $bundle_config_id);
-      $entity_type_id = FALSE;
-      foreach ($entity_type_manager->getDefinitions() as $entity_definition) {
-        if ($entity_definition->get('config_prefix') === $bundle_config_prefix) {
-          $entity_type_id = $entity_definition->getBundleOf();
-          break;
-        }
-      }
-      if (!$entity_type_id) {
-        throw new \Exception('Something went wrong.');
-      }
+      list($entity_type_id,,$bundle_id) = explode('.', $bundle_config_id);

Sounds like we've got a solution for this that works across all entity types now.

+++ b/core/modules/content_moderation/content_moderation.install
@@ -131,13 +123,16 @@
+  foreach ([$definition->getDataTable(), $definition->getRevisionDataTable()] as $table) {
+    $db_connection->update($table)->fields(['moderation_state' => NULL])->execute();
   }

I wonder if this works for non-translatable entity types. I'd be more interested in figuring out why the entity query can't empty everything.

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented

Thanks for reviewing!

I couldn't figure out why that initial query didn't catch all of the values either. I don't think it would be related to an access check as I'm running the updates via drush, however it might be worth a shot. I can set up a test environment with my old data to verify whether it works or not as well.

I noticed that values were only left over in the base data table and not in the revision data table for moderation states. I don't know if or why that's significant, but just wanted to note it.

I did originally install content_moderation on an existing database, so none of the content had a moderation state until it was saved again after that, and some of the content might never have been saved. But I don't think that would affect this, since if there is no moderation state then it wouldn't contain non-purgable data.

Is it possible there could be data in the base table which is never returned in an entity query because there is revision data overriding it? I'm not entirely clear on how data is stored in the base table vs the revision table when using revisions, TBH.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

The data table should have the default revision in it, which is a revision none the less, so it should have been updated. I wonder if we can safely empty the table, but I don't think I've seen a precent for this anywhere else.

nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented

I will try this patch out and provide feedback on my experience. I'm not using content_moderation heavily (just for one node bundle with a draft and published state).

One concern about using the patch is that if there is no hope of it being adopted into core then I will most certainly have to remove the patch after it does it's work so I don't disrupt the upgrade path for core in future. But, the 8001 update hook will already have been fired in my system so when I remove the patch I will also have to trigger a reset on the system table for the content_moderation module.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

@nlisgo, that's a really good consideration to make. I'm totally in support of removing the patch as soon as the update hook has run, it was designed for that, but as far as the schema version goes, that'll need rolling back.

Maybe core make an allowance for this issue and start at 800X for wherever this gets up to, otherwise we'll need some instructions for dialing it back, but it's indeed another thing to consider/think about.

nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented
FileSize
interdiff-2846618-22-29.txt1 KB
unofficial-2846618-29.patch629.21 KB

I ran into an issue when the update hooks are fired. Sorry, I don't have time to improve the test to catch this bug but may contribute that later. Obviously, I have some modules installed that you may not have.

$ drush updatedb -y
The following updates are pending:

system module :
  8300 -   Add detailed cron logging configuration.
  8301 -   Add install profile to core.extension configuration.

block_content module :
  8300 -   Fix the block_content entity type to specify its revision data table.

content_moderation module :
  8001 -   Upgrade path from 8.2.x.

crop module :
  8002 -   Let Drupal know that there is a new config available.

image module :
  8201 -   Flush caches as we changed field formatter metadata.

node module :
  8300 -   Change {node_access}.fallback from an int to a tinyint as it is a boolean.
  8301 -   Set the 'published' entity key.

serialization module :
  8301 -   @see hal_update_8301()
  8302 -   Add serialization.settings::bc_primitives_as_strings configuration.

block module :
  Disable blocks that are placed into the "disabled" region.

system module :
  Update entity displays to contain the region for each field.
  Force caches using hashes to be cleared (Twig, render cache, etc.).
  Force plugin definitions to be cleared.   @see https:www.drupal.orgnode2802663

user module :
  Enforce order of role permissions.

views module :
  Rebuild caches to ensure schema changes are read in.

Do you wish to run all pending updates? (y/n): y
Performing node_update_8300                                          [ok]
Performing system_update_8300                                        [ok]
Performing serialization_update_8301                                 [ok]
Performing system_update_8301                                        [ok]
The "workflow" entity type does not exist.                           [error]
Performing block_content_update_8300                                 [ok]
Performing content_moderation_update_8001                            [ok]
Performing crop_update_8002                                          [ok]
Performing image_update_8201                                         [ok]
Performing node_update_8301                                          [ok]
Performing serialization_update_8302                                 [ok]
Failed: The "workflow" entity type does not exist.                   [error]
Cache rebuild complete.                                              [ok]
Finished performing updates.                                         [ok]

I tried simply rearranging the order in which the hooks were fired to ensure that content_moderation_update_8001 was run before block_content_update_8300 but it still gave me an error:

Performing system_update_8300                                        [ok]
The "workflow" entity type does not exist.                           [error]
Performing content_moderation_update_8001                            [ok]
Performing node_update_8300                                          [ok]
Performing serialization_update_8301                                 [ok]
Performing system_update_8301                                        [ok]
Performing crop_update_8002                                          [ok]
Performing image_update_8201                                         [ok]
Performing node_update_8301                                          [ok]
Performing serialization_update_8302                                 [ok]
Drupal\Component\Plugin\Exception\PluginNotFoundException: The       [error]
"workflow" entity type does not exist. in
/var/www/journal-cms/web/core/lib/Drupal/Core/Entity/EntityTypeManager.php:133
Stack trace:
#0
/var/www/journal-cms/web/core/lib/Drupal/Core/Entity/EntityManager.php(46):
Drupal\Core\Entity\EntityTypeManager->getDefinition('workflow', true)
#1
/var/www/journal-cms/web/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php(116):
Drupal\Core\Entity\EntityManager->getDefinition('workflow')
#2
/var/www/journal-cms/web/core/lib/Drupal/Core/Field/BaseFieldDefinition.php(634):
Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::schema(Object(Drupal\Core\Field\BaseFieldDefinition))
#3
/var/www/journal-cms/web/core/lib/Drupal/Core/Field/BaseFieldDefinition.php(657):
Drupal\Core\Field\BaseFieldDefinition->getSchema()
#4
/var/www/journal-cms/web/core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php(189):
Drupal\Core\Field\BaseFieldDefinition->getColumns()
#5
/var/www/journal-cms/web/core/modules/views/src/EntityViewsData.php(365):
Drupal\Core\Entity\Sql\DefaultTableMapping->getColumnNames('workflow')
#6
/var/www/journal-cms/web/core/modules/views/src/EntityViewsData.php(266):
Drupal\views\EntityViewsData->mapFieldDefinition('content_moderat...',
'workflow', Object(Drupal\Core\Field\BaseFieldDefinition),
Object(Drupal\Core\Entity\Sql\DefaultTableMapping), Array)
#7 /var/www/journal-cms/web/core/modules/views/views.views.inc(169):
Drupal\views\EntityViewsData->getViewsData()
#8 [internal function]: views_views_data()
#9
/var/www/journal-cms/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(391):
call_user_func_array('views_views_dat...', Array)
#10
/var/www/journal-cms/web/core/modules/views/src/ViewsData.php(245):
Drupal\Core\Extension\ModuleHandler->invoke('views', 'views_data')
#11
/var/www/journal-cms/web/core/modules/views/src/ViewsData.php(162):
Drupal\views\ViewsData->getData()
#12
/var/www/journal-cms/web/core/modules/views/src/Plugin/Derivative/ViewsEntityRow.php(91):
Drupal\views\ViewsData->get('block_content')
#13
/var/www/journal-cms/web/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php(101):
Drupal\views\Plugin\Derivative\ViewsEntityRow->getDerivativeDefinitions(Array)
#14
/var/www/journal-cms/web/core/lib/Drupal/Component/Plugin/Discovery/DerivativeDiscoveryDecorator.php(87):
Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDerivatives(Array)
#15
/var/www/journal-cms/web/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php(283):
Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDefinitions()
#16
/var/www/journal-cms/web/core/lib/Drupal/Core/Plugin/DefaultPluginManager.php(174):
Drupal\Core\Plugin\DefaultPluginManager->findDefinitions()
#17
/var/www/journal-cms/web/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryCachedTrait.php(22):
Drupal\Core\Plugin\DefaultPluginManager->getDefinitions()
#18
/var/www/journal-cms/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php(16):
Drupal\Core\Plugin\DefaultPluginManager->getDefinition('node_rss')
#19
/var/www/journal-cms/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php(84):
Drupal\Core\Plugin\Factory\ContainerFactory->createInstance('node_rss',
Array)
#20
/var/www/journal-cms/web/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php(810):
Drupal\Component\Plugin\PluginManagerBase->createInstance('node_rss')
#21
/var/www/journal-cms/web/core/modules/views/src/Plugin/views/style/StylePluginBase.php(122):
Drupal\views\Plugin\views\display\DisplayPluginBase->getPlugin('row')
#22
/var/www/journal-cms/web/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php(813):
Drupal\views\Plugin\views\style\StylePluginBase->init(Object(Drupal\views\ViewExecutable),
Object(Drupal\views\Plugin\views\display\Feed), Array)
#23
/var/www/journal-cms/web/core/modules/views/src/ViewExecutable.php(871):
Drupal\views\Plugin\views\display\DisplayPluginBase->getPlugin('style')
#24
/var/www/journal-cms/web/core/modules/views/src/ViewExecutable.php(1828):
Drupal\views\ViewExecutable->initStyle()
#25
/var/www/journal-cms/web/core/modules/views/src/Plugin/views/display/PathPluginBase.php(132):
Drupal\views\ViewExecutable->getTitle()
#26
/var/www/journal-cms/web/core/modules/views/src/Plugin/views/display/PathPluginBase.php(220):
Drupal\views\Plugin\views\display\PathPluginBase->getRoute('frontpage',
'feed_1')
#27
/var/www/journal-cms/web/core/modules/views/src/EventSubscriber/RouteSubscriber.php(120):
Drupal\views\Plugin\views\display\PathPluginBase->collectRoutes(Object(Symfony\Component\Routing\RouteCollection))
#28 [internal function]:
Drupal\views\EventSubscriber\RouteSubscriber->routes()
#29
/var/www/journal-cms/web/core/lib/Drupal/Core/Routing/RouteBuilder.php(146):
call_user_func(Array)
#30
/var/www/journal-cms/web/core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php(83):
Drupal\Core\Routing\RouteBuilder->rebuild()
#31 /var/www/journal-cms/web/core/includes/common.inc(1141):
Drupal\Core\ProxyClass\Routing\RouteBuilder->rebuild()
#32
/var/www/journal-cms/vendor/drush/drush/commands/core/drupal/update.inc(120):
drupal_flush_all_caches()
#33
/var/www/journal-cms/vendor/drush/drush/commands/core/drupal/batch.inc(163):
drush_update_cache_rebuild(Object(DrushBatchContext))
#34
/var/www/journal-cms/vendor/drush/drush/commands/core/drupal/batch.inc(111):
_drush_batch_worker()
#35 /var/www/journal-cms/vendor/drush/drush/includes/batch.inc(98):
_drush_batch_command('2')
#36
/var/www/journal-cms/vendor/drush/drush/commands/core/drupal/update.inc(193):
drush_batch_command('2')
#37
/var/www/journal-cms/vendor/drush/drush/commands/core/core.drush.inc(1227):
_update_batch_command('2')
#38
/var/www/journal-cms/vendor/drush/drush/includes/command.inc(422):
drush_core_updatedb_batch_process('2')
#39
/var/www/journal-cms/vendor/drush/drush/includes/command.inc(231):
_drush_invoke_hooks(Array, Array)
#40
/var/www/journal-cms/vendor/drush/drush/includes/command.inc(199):
drush_command('2')
#41
/var/www/journal-cms/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67):
drush_dispatch(Array)
#42
/var/www/journal-cms/vendor/drush/drush/includes/preflight.inc(66):
Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#43 /var/www/journal-cms/vendor/drush/drush/drush.php(12):
drush_main()
#44 {main}
Cache rebuild complete.                                              [ok]
Finished performing updates.                                         [ok]

So, I added in the following:

$entity_type_manager->clearCachedDefinitions();

And it worked!

bmcclure’s picture

bmcclure CreditAttribution: bmcclure as a volunteer and at Top Floor commented

I agree about removing the patch and rolling back after a successful upgrade.

Once you remove the patch, I think you could use the following drush command: drush ev "drupal_set_installed_schema_version('content_moderation', 8000)

This way if any update hooks are ever added to content_moderation, they are sure to run still.

nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented

#30 worked for me to undo the patch and restore the schema version: https://github.com/elifesciences/journal-cms/pull/165/files#diff-346cbc6...

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Rolling back the schema version seems like a great idea. Perhaps worth adding some instructions in the issue summary?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

Glad I found this issue, it's serving as a nice guideline for upgrade paths. While I haven't tested the update hook here (we have more than one workflow that needs to be generated from a huge number of states/transitions), this approach looks solid.

One suggestion would be to document putting this code into a custom module (and renaming the update hooks accordingly), so that in the future when content_moderation is stable, and starts getting its own official update hooks, sites aren't broken thinking they've already run the new update hooks.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Yeah, I mentioned in #32 we should probably put the schema rollback instructions in the issue summary.

I toyed with the idea of dynamically generating workflows based on the states/transitions allowed for specific bundles, but I haven't looked into the complexity of doing this yet.

T-lo’s picture

T-lo
Primary language English
Location Bristol, UK
CreditAttribution: T-lo as a volunteer commented

Thanks for this, worked smoothly for me.

One thing to note is if you'd assigned roles to the previous workflow states, you'll now need to add permissions

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue summary: View changes

Thanks for the feedback! Adding some basic instructions to the issue summary to include #37.

mstef’s picture

mstef CreditAttribution: mstef commented

edit: additional cache clear seems to resolve it. patchworks. thanks!

After update, /admin/config/workflow/workflows throws:

Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException: The "workflow" entity type did not specify a list_builder handler. in Drupal\Core\Entity\EntityTypeManager->getHandler() (line 236 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).

Drupal\Core\Entity\EntityTypeManager->getListBuilder('workflow') (Line: 73)
Drupal\Core\Entity\EntityManager->getListBuilder('workflow') (Line: 22)
Drupal\Core\Entity\Controller\EntityListController->listing('workflow')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 144)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 64)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 75)
Drupal\shield\ShieldMiddleware->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 656)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Not sure if it's related to the patch or something specific on my site.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

The list builder class is set in workflows_entity_type_build. Still getting the error after a cache rebuild?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue summary: View changes
mstef’s picture

mstef CreditAttribution: mstef commented

An additional cache clear after updb did the trick. Everything is working well. Thank you to everyone involved for working on this. Great job.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue summary: View changes
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

For sites migrating from 8.2 that have translations enabled, the fix over in #2881645: ContentModerationState 'workflow' column empty for all non-default translations. will keep things working. Without that, the non-default language moderation states don't get properly migrated.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented 
+++ b/core/modules/content_moderation/content_moderation.install
@@ -0,0 +1,171 @@
+  // Before uninstalling the old moderation state field, build a list of
+  // content_moderation_state revisions mapped to states.
+  $state_map = [];
+  $content_moderation_state_revisions = \Drupal::entityQuery('content_moderation_state')
+    ->allRevisions()
+    ->execute();

I'm working on a site that is migrating some 130k moderation state revisions, so this bit where it loads them all in memory had to be refactored. I don't have the code to post yet, but will try to do so shortly.

In that code I've also split up each discreet operation into a separate update hook, so ones that require utilizing a batch can do so, and those that don't are kept simple.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

Another issue I just ran into, if the old entity reference moderation_state field (attached to entities/bundles) had base_field_overrides defined, those needed to be removed or the system got really confused. I'm not familiar with how/when those base_field_overrides get created though, just something I ran into...

ansitun’s picture

ansitun CreditAttribution: ansitun as a volunteer and commented

Hi,

I have tried to upgrade from drupal 8.2.7 to the latest version, drupal 8.3.3, I have many content entities that have content moderation enabled; I have followed the below steps for the upgrade:

1. composer update [since my codebase is setup with composer]
2. Apply the patch and did composer install
3. drush updb
4. drush entity-updates because there were entity changes
5. Removed the patch and did composer install
6. drush entity-updates and I get this error:

MacBook-Pro-3:web ansuraj$ drush entity-updates
The following updates are pending:

content_moderation_state entity type : 
  The Content moderation state entity type needs to be updated.
  The Workflow field needs to be installed.
node entity type : 
  The node.field_table_text field needs to be updated.
Do you wish to run all pending updates? (y/n): y
Drupal\Core\Entity\EntityStorageException: Exception thrown while performing a schema update. SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was[error]
too long; max key length is 767 bytes: ALTER TABLE {content_moderation_state_field_data} ADD UNIQUE KEY `content_moderation_state__lookup` (`content_entity_type_id`,
`content_entity_id`, `content_entity_revision_id`, `workflow`, `langcode`); Array
(
)
 in Drupal\Core\Entity\Sql\SqlContentEntityStorage->wrapSchemaException() (line 1485 of
/Users/ansuraj/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
Failed: Drupal\Core\Entity\EntityStorageException: !message in Drupal\Core\Entity\Sql\SqlContentEntityStorage->wrapSchemaException() (line 1485 of                  [error]
/Users/ansuraj/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
Cache rebuild complete.

Kindly let me know if you find a patch for this.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

I believe the error was introduced by #2779931: Add storage exception that enforces unique content_entity_type_id and content_entity_id on the content moderation state content entity, and add access control handler to forbid all access. We need to add the change to the max_length to the upgrade path or the index we create is too long.

Re-queuing the tests to see if they catch this.

Status: Needs review » Needs work

The last submitted patch, 29: unofficial-2846618-29.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

This is the code for a site that has about 10 different 'workflows' in 8.2.x. I've edited the file to only show 3 for demonstration purposes. The other major difference from the approach here is that it uses a temporary table for the moderation state migration since there were too many to do in a single pass in memory.

https://gist.github.com/jhedstrom/0a1723a94484e3a0e46e2692d4698fe6

I was going to try to apply some of this to the patch here, but realized that due to the custom workflow logic, it wouldn't be applicable, and each site using more than one workflow would need their own logic for that bit of the migration.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented

I tried adding this bit to handle the column length change:

  // Update max_length of the content_entity_type_id so the unique index can
  // be added without being too long.
  // @see https://www.drupal.org/node/2779931
  $definition = \Drupal::entityDefinitionUpdateManager()->getFieldStorageDefinition('content_entity_type_id', 'content_moderation_state');
  $definition->setSetting('max_length', EntityTypeInterface::ID_MAX_LENGTH);
  \Drupal::entityDefinitionUpdateManager()->updateFieldStorageDefinition($definition);

but that failed to update the actual column length, because from what I can tell, the schema isn't updated if a setting is changed. I've taken the approach of directly updating the sql table definitions:

  // Update max_length of the content_entity_type_id so the unique index can
  // be added without being too long.
  // @see https://www.drupal.org/node/2779931
  $definition = \Drupal::entityDefinitionUpdateManager()->getFieldStorageDefinition('content_entity_type_id', 'content_moderation_state');
  // Calling `setSetting` with a new `max_length` here doesn't appear to work,
  // so the database columns are directly updated.
  $spec = $definition->getColumns();
  $spec['value']['length'] = EntityTypeInterface::ID_MAX_LENGTH;
  db_change_field('content_moderation_state_field_data', 'content_entity_type_id', 'content_entity_type_id', $spec['value']);
  db_change_field('content_moderation_state_field_revision', 'content_entity_type_id', 'content_entity_type_id', $spec['value']);

which seems to work out.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Awesome, I was going to open up an issue for 8.2 to 8.3 upgrade path and request future schema breaks not be backported. #52 is a great start.

Rop’s picture

Rop CreditAttribution: Rop as a volunteer commented

I did an upgrade from 8.27 to 8.3.3 using #29 patch which worked fine.
Only now when upgrading from 8.3.3 to 8.3.4 I get the before mentioned error (#48) :

SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was[error]
too long; max key length is 767 bytes:...

Where is #52 to be placed?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Title: Unofficial content_moderation 8.2 to 8.3 upgrade path » Unofficial content_moderation 8.2.0 to 8.3.0 upgrade path
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Title: Unofficial content_moderation 8.2.0 to 8.3.0 upgrade path » Unofficial content_moderation 8.2.x to 8.3.0 upgrade path

It's clear we're going to have to organise these into patches that upgrade from very specific releases. To make things easier I have:

  1. Renamed this issue to be very specific with version numbers.
  2. Opened #2890187: [META] Unofficial content_moderation upgrade path as a meta.
  3. Opened #2890189: Unofficial content_moderation 8.3.2 to 8.3.3 upgrade path to track 8.3.2 to 8.3.3.

If @jhedstrom wants to cut a patch for that last issue, that would be awesome, otherwise I can upload it.

I think it'll be easiest to assume the schema version is 8000 before applying and running any of the patches, but make sure we're include checks and balances at the top of each update hook to make sure the schema change hasn't already been applied.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Parent issue: » #2890187: [META] Unofficial content_moderation upgrade path
ahebrank’s picture

ahebrank CreditAttribution: ahebrank commented

EDIT: never mind, I need to read these upgrade threads more carefully.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs work » Closed (works as designed)
nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented

For my needs, I am using content moderation in a limited way. I have decided that uninstalling and reinstalling the module at the appropriate time is the best way forward. As long as I do this at a moment that no content is pending review for my project this will have zero impact.

In an update hook, I delete all of the existing content moderation states and then I can uninstall the module. My deployment workflow is to perform a configuration import after a `drush updatedb -y` so the module will be reinstalled immediately after uninstalling it and with the configuration that I have specified.