Check RevisionLogInterface for summary and provide

@lhangea the revision comment field only exists on node revisions. All other entities don't have a similar comment. As a result, the revision tab could only display the revision id and date. Also the revision author is optional data.

You can see an example revision tab in collect module: #2446011: Show container revisions

To show more than just the date, the image outputs the content with a cutoff at 255 chars. This is still not satisfying. The general revisions tab needs to provide extensibility for a nice revision summary.

We want to decouple the general revisions tab from a pluggable revision handler. As berdir pointed out, the summary is just one thing.
This would be a healthy pattern to adopt by core once in future.

Unpostponing. The generic controller has landed.
Revision summary still easily sucks.

IMHO if the revision summary is not provided by the user, the system could add a revision change / summary statement. Not just if the revision comment field is technically not existing.

Soo... I just checked the issue queue.
Most issues are cleanups and small bugs. After the most recent bugfix.

This is the only remaining major feature we are eager to see.
It's not a release blocker though. We are also fine to add it on a 1.1+ release as it's also not introducing any incompatibility.

In any case, a diff release is near. :-)

This whole discussion about the view is more related to #2452523: [Meta] Offer a revisions tab for all entities
In code there is a reference to the core issue that wants to convert the revision tab to a view... You might want to create a separate issue about this subject?

So after the progress, discussions and the direction about diff pluggability, i think it's clear:
This handler will provide revision (summary) information and not really anything diff specific.

Yeah a valid first step.

Checkout Node.php. It has a list_builder in the annotation or 'translation' for a TranslationHandler.

What we want is to introduce a new key here for the revision handler.
We could alter entity definitions and bring the key in for all entities.
Or we could fall back to our code when no specific one is declared. I guess that's more minimalistic.

1. +++ b/src/EntityRevision.php
   @@ -0,0 +1,76 @@
   +      $revision_summary = Xss::filter($revision->getRevisionLogMessage());
   ...
   +      // @todo Implement a method to get a revision summary when is none.
   

   Also do the fallback if the RevisionLogMessage is empty.

2. +++ b/src/EntityRevisionInterface.php
   @@ -0,0 +1,22 @@
   + * Defines an interface to build entity listings.
   

   Almost. ;-)

3. +++ b/src/EntityRevisionInterface.php
   @@ -0,0 +1,22 @@
   +interface EntityRevisionInterface {
   ...
   +  public function summary(EntityInterface $revision);
   

   I think our name doesn't differentiate yet and can be easily confused with RevisionableInterface. Interestingly this skips "Entity" completely.
   Are we only handling the summary or are we going to add other revision related methods too?

We had some general discussions and change directions.

It turns out that 8.2 core introduced an interface to deal with revisions: RevisionLogInterface
Node implements it and sure returns empty strings if no log message is stored.
Still the Revision tab can not be altered easily but we override it anyway.

We only need pluggability if we stop overriding and the revision tab is provided by Entity.
To stay simple, let's stay within our straight scope:

Add the fallback logic into our override tab controller and check for RevisionLogInterface and if inexisting or not available, fall back to some generated summary. I think that helps most for fast iteration and doesn't add new assumptions or APIs to maintain.
We can then bring in our extensibility requirements to Entity module or Core when we are well progressed.

Entity has RevisionOverviewController with getRevisionDescription for a revision.
For the specific case of Collect, Diff won't be able to offer a reasonable default.
We will create a follow-up to switch away from the own Controller for revisionOverview to the generic Entity revision tab and then implement the interface to offer a generated summary.

A revision summary could be to loop over all fields and compare each of the values and mention which field changed.
That however would require the summary of each revision to load the previous revision too.
But it seems that will always be needed for a generated change summary.

+++ b/src/Controller/PluginRevisionController.php
@@ -277,4 +285,37 @@ class PluginRevisionController extends ControllerBase {
+    $revisions = $this->getRevisionIds($storage, $revision->id());

getRevisionIds is node specific. Also some things are this line seem wild. ;-)

Berdir proposes to use an entity query.

This method should get the current and the previews revision, or at least call another method that gets both params.

Nice. I think we should make it more compact and implode by "," so it fits better the text format of a revision log.

As an idea to better indicate change:
Git displays on the terminal repeatingly + or - varying the amount of changes. Or sometimes % change. And a summary on a merge.

Example:

core/lib/Drupal/Core/Session/SessionManager.php           |  3 ++-
 .../src/Controller/TestPageTestController.php             |  3 ++-
 core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php | 15 +++++++++++++++
 core/tests/Drupal/Tests/BrowserTestBase.php               |  8 ++++++--
 4 files changed, 25 insertions(+), 4 deletions(-)

So the challenge would be that we need to trigger the diff engine to know how many characters / words / lines have been removed and added.
Title +3/-15, Body +1k/-3.1k

Or even more simplified, we could output the change in length?

Title -34%, Body -15%
Title -12, Body -2.1k
Title 28>16, Body 14k>11.9k

But i guess those are lots of ideas for follow-ups... But not yet sure what we need to boost UX.

Ha, funny, the first field i get listed as changed is "Revision ID", so we should definitively only list fields that are subject for diff like we filter the fields in our field settings UI.

As much as at a first glance this looks nice:
"Revision ID, Changed, Revision timestamp, Revision log message, Body"
Only the "Body" part of it is real information. The other ones are all garbage.

BTW how are we determining field sequence?

I'm not 100% sure if we can only list the fields or if we need to prepend the message with "Changed: " or "Changes on ".

Great stuff.

Hey tests, where are you hiding? :-)

1. +++ b/src/DiffEntityComparison.php
   @@ -292,16 +292,19 @@
   +        $summary .= $summary != '' ? ', ' : 'Changes on: ';
   

   I don't like these crypto lines.
   i'd just build an array of items, after the loop check for count() and implode conditionally with prepending the text with else "No changes".

2. +++ b/src/Form/RevisionOverviewForm.php
   @@ -205,7 +205,7 @@
   -          $key - 1 > 0 ? $old_key = $key - 1 : $old_key = 0;
   +          isset($vids[$key + 1]) ? $old_key = $key + 1 : $old_key = $key;
   

   Any what is this crapto line all about?

Ha! Funny how simple this code can be!
Plus really awesome to have this in now!

Committed and updated summary a bit from comments.

Created Follow-up.