Problem/Motivation

After making an entity type revisionable and visiting the revision overview of that entity, the page crashes with the following error: 'The timestamp must be numeric'. This issue was first reported by @joachim in #2350939-82: Implement a generic revision UI and I just encountered the same issue when working on #2880154: Convert comments to be revisionable.

I can't seem to find code that set an initial value for this field (and the revision_user field for that matter) on any entity types that were already converted to be revisionable (eg. block content, terms, menu links), so I'm assuming there's existing content in the wild with NULL values for those columns.

Steps to reproduce

  1. Make an entity type revisionable
  2. Include the changes of #2350939: Implement a generic revision UI in your project
  3. Visit the revision overview of a pre-existing entity

Proposed resolution

Either we should set an initial value for revision_created during the upgrade (taken from the created column), or we should handle the field being empty throughout the UI.

In #2880154: Convert comments to be revisionable I created a post update hook to set the default values, but another way to do it would be to use Drupal\Core\FieldBaseFieldDefinition::setInitialValueFromField. That doesn't currently work though, but it used to before. Since the removal of SqlContentEntityStorageSchemaConverter we have to use updateFieldableEntityType when making an entity type revisionable. The installation of the revision fields happens in that method. However, setInitialValueFromField doesn't seem to work when installing fields using updateFieldableEntityType. I tried it and the revision_created and revision_log_message columns of existing comments are still NULL after the upgrade.

Comments

DieterHolvoet created an issue. See original summary.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented

Also seeing the same issue where the initial value is ignored when using updateFieldableEntityType when adding an owner and status fields. These columns cannot be null and therefore we get an integrity constraint error.

acbramley’s picture

acbramley commented

I'd kinda expect to have to create post update hooks? See #3038365: [PP-1] Add owner to the BlockContent entity type

Are you wanting a solution that automatically sets these values for existing revisions for you? I'm not sure how that would work...

psf_’s picture

psf_ commented

+1 here :) . I'm using Drupal 10.2.2

acbramley’s picture

acbramley commented

I've created #3459169: Protect against empty revision timestamps in VersionHistoryController::getRevisionDescription as I think we should be defensive for empty timestamps regardless of default values.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.