Protect against empty revision timestamps in VersionHistoryController::getRevisionDescription

somehow related?
https://www.drupal.org/project/drupal/issues/3439347 🐛 InvalidArgumentException: The timestamp must be numeric. in Drupal\Component\Datetime\DateTimePlus::createFromTimestamp() Postponed: needs info

No, that is in a different part of the codebase.

Pasting partially from 🐛 Core now allows moderation of taxonomy terms, but an error is thrown when trying to use scheduled transitions on this Closed: works as designed

I dont think we should be adding any defensiveness until something is decided for \Drupal\Core\Entity\RevisionLogInterface::getRevisionCreationTime.

There's no point in adding a NULL check if its illegal to return NULL. Violates static analysis, etc.

Somehow it is allowed to be non-int, by design: this doesnt make sense to me, as revisions should always have a create date.
Data is malformed, for example during a migration or bad update: a migration should be created to fix the data.
The interface maybe needs to be updated to return NULL.

I made an update to #3461602: Core now allows moderation of taxonomy terms, but an error is thrown when trying to use scheduled transitions on this

Everything works with new terms, but my existing terms have the error mentioned there.

I encountered the same problem in drupal 10.3.2. When I accessed the Revision list of a certain taxonomy/taxonomy/term/xxx/revisions, the page reported an error InvalidArgumentException: The timestamp must be numeric.
I created a patch and when the revision created time is empty, I display the changed time of the revision as link text.

I encountered the same problem in Drupal 10.3.6. When clicking the Revision Tab link on a taxonomy term, the page error reported:
"InvalidArgumentException: The timestamp must be numeric"

This was an upgrade from 10.2.5.

Patch in #7 fixed it.

https://www.drupal.org/files/issues/2024-08-21/3459169.patch →

The patch in #3459169-7: Protect against empty revision timestamps in VersionHistoryController::getRevisionDescription → is a great start, but it still errors when you go to revert to a revision with a missing timestamp, since it tries to put the revision timestamp in the page title.

In the spirit of #3459169-5: Protect against empty revision timestamps in VersionHistoryController::getRevisionDescription → , I've attached a patch with a proposed solution that will fix the issue at the source and prevent the value ever being null:

1. In RevisionLogEntityTrait:getRevisionCreationTime(), we check if revision_created is null.
2. If null, we fallback to changed (credit to @connbi for that idea).
3. If changed is somehow null (I'm not even sure if this would ever happen?), we fall back to 0 which will get rendered as January 1, 1970 (a pretty universally recognizable "missing" timestamp)

If this feels like a good solution, I can start an MR and add tests.

Thanks for working on this.

Fixes should be in an MR to start with so we can see if it breaks anything first. For this kind of issue maybe it would be best to start with a test case to fully show the issue? Then that could help drive the solution.

Personally what you had makes sense.