Offer a revisions tab for all entities

The first step in making diff compatible with other entity types.

- look for the version-history link template in the entity and alter the route. ( maybe we need to introduce an option to alter the link templates of entities to be compatible with diff)
- Node centric revisionOverview() method updated to support other entities, maybe we need to introduce new forms I'm not sure at this point.
- RevisionOverviewForm refactored and entity query added to get the revision ids of other entities.

Dynamic routing introduced and node centric route removed. A method introduced to generate URL from these dynamic routes. now stuck with the core issue #2458543: Entity query age(EntityStorageInterface::FIELD_LOAD_REVISION) only gets current revision ID.
Work in progress patch is uploaded here.
(Started from the beginning, no interdiff)

1. +++ b/diff.module
   @@ -76,7 +77,7 @@ function diff_help($route_name, RouteMatchInterface $route_match) {
    
   -    case 'diff.revisions_diff':
   +    case 'diff.revisions_diff.node':
          return '<p>' . t('Comparing two revisions:') . '</p>';
    
   

   This should match any route name that starts with diff.revision_diff.

2. +++ b/diff.module
   @@ -86,3 +87,21 @@ function diff_help($route_name, RouteMatchInterface $route_match) {
   +/**
   + * Returns URL for each entity type revisions diff overview.
   + * @param string $entity_type_id
   + * @param int $entity_id
   + * @param int $left_vid
   + * @param int $right_vid
   + * @param string $filter
   + * @return Url
   + */
   +function diff_generate_url($entity_type_id, $entity_id, $left_vid, $right_vid, $filter = NULL) {
   +  return Url::fromRoute('diff.revisions_diff.' . $entity_type_id, array(
   +    'node' => $entity_id,
   +    'left_vid' => $left_vid,
   +    'right_vid' => $right_vid,
   +    'filter' => $filter,
   +  ));
   

   vid is node specific, the generic term is revision_id, so we should rename it everywhere.

   Also documentation and stuff ;)

   This method could live on the new service that you are creating in the paragraph issue..

3. +++ b/src/DiffBreadcrumbBuilder.php
   @@ -19,7 +19,7 @@ class DiffBreadcrumbBuilder extends PathBasedBreadcrumbBuilder {
      public function applies(RouteMatchInterface $route_match) {
   -    if ($route_match->getRouteName() == 'diff.revisions_diff') {
   +    if ($route_match->getRouteName() == 'diff.revisions_diff.node') {
   

   Same here, needs to apply on all routes starting like that.

4. +++ b/src/Form/RevisionOverviewForm.php
   @@ -103,16 +106,17 @@ class RevisionOverviewForm extends FormBase {
   -    $type = $node->getType();
   +    $type = $entity->bundle();
   

   $type should be renamed too.

5. +++ b/src/Form/RevisionOverviewForm.php
   @@ -123,29 +127,42 @@ class RevisionOverviewForm extends FormBase {
   +    if ($entity->getEntityType()->id() == 'node') {
   +      $vids = array_reverse($entity_storage->revisionIds($entity));
   +    }
   +    else {
   +      $vids = \Drupal::service('entity.query')->get($entity->getEntityTypeId())
   

   As discussed, we *should* remove the node specific part, but the entity query doesn't actually work yet ;)

6. +++ b/src/Form/RevisionOverviewForm.php
   @@ -196,7 +219,7 @@ class RevisionOverviewForm extends FormBase {
              $route_params = array(
   -            'node' => $node->id(),
   +            'node' => $entity->id(),
                'node_revision' => $vid,
              );
   

   sounds like this needs to be more dynamic too.

7. +++ b/src/Routing/RouteSubscriber.php
   @@ -19,14 +20,35 @@ class RouteSubscriber extends RouteSubscriberBase {
   +        $route = $collection->get('entity.' . $entity_type->id() . '.version_history');
   +        $route->setOption('_diff_entity_type', $entity_type->id());
   +        $route->setOption('parameters', array($entity_type->id() => array('type' => 'entity:' . $entity_type->id())));
   +        $route->addDefaults(
   +          array(
   +            '_controller' => '\Drupal\diff\Controller\NodeRevisionController::revisionOverview'
   +          )
   

   As discussed, this should check if that route exists and if not, create it.

   The only problem is access, just default to _entity_access: view for now.

Ok, all content entities can have revisions since they all implement RevisionableInterface but ATM as far as I know only nodes can be saved as revisions (from the user interface). For example the modifications done to users can't be saved as revisions by default. I suppose that's sort of OK with us because we only provide the interface for revision comparison and how those revisions are actually created is handled by core/other contrib modules. Or, should we handle that part too ?

Created a related meta issue: #2465901: [META] Make entity revision translation work.

I worked on this issue and it's almost solved. Basically now we need to work on the #2458543: Entity query age(EntityStorageInterface::FIELD_LOAD_REVISION) only gets current revision ID core issu. After that issue is solved the diff should work fine for all revisionable entities.
Also re-rolled the patch.

Tried to address all the suggestions of Miro and Berdir and mostly solved them.
However there's an important one left:

The only problem is access, just default to _entity_access: view for now.

If I set the requirements to that I get some error:

Notice: Undefined offset: 1 in Drupal\Core\Entity\EntityAccessCheck->access() (line 47 of core/lib/Drupal/Core/Entity/EntityAccessCheck.php).

Maybe tomorrow I will fix that too. For now I left the requirements to:

$requirements = array(
  '_permission' => 'access content'
);

just so that the diff works but that will be changed.

Other than that, it works for nodes as it worked before but now everything is generalized to entities rather than nodes as before.

The syntax for _entity_access is _entity_access: <entity_type>.view (where entity type actually maps to the route argument, not the entity type, but that's the same almost all the time), otherwise the access check doesn't know on which object ->access() needs to be called.

#2462731: Check RevisionLogInterface for summary and provide is a bit confusing, we might want to introduce such a handler here. Everything that might possibly be different based on the entity type should be wrapped in a method on that handler.. one for generating the routes, one for checking access, one to get the revision author, date, summary, .. Then we can have simple assumptions in a default handler and a customized implementation for nodes. On the other side, trying to do that all in a single issue might be too complicated.

Actually there should be more in that interdiff. The part with route subscriber and dynamic routes didn't get in the interdiff :).

And for _entity_access the syntax was incorrect, you're right.
As for the handler I would say after fixing this one to continue in the created issue as this patch is very big already.

The core issue got in :)

The if/else to get the revision ID's can be removed as well now, the entity query should now work nodes as well.

Yep, I was following that issue. Very nice.

Worked a little bit on this. And made it work for other content entities like block_content :)
But...
If you try right now it won't work because:
We assume that every entity defines these 3 routes:

• [entity_type_id].revision_show
• [entity_type_id].revision_revert_confirm
• [entity_type_id].revision_delete_confirm

But they don't.
Only node module does this ATM. But, if they did it'd work just fine.

There are some other inconsistencies. Not very big like the above but still. E.g. I saw that block_content entity doesn't have fields like: revision_timestamp and revision_uid so the interface doesn't look as good as for nodes but maybe we can solve all this with the handlers we are talking about.

Basically the missing routes are the big ones and because of those it doesn't work ATM for other entities unless you comment those sections from the code. OR I could check for route existence and if it doesn't exist just don't display the links (Although I think it would be better to have them because what is the point of having revisions if we cannot look at them or revert them ? ).

In the patch the EntityRevisionController is treated as a new file even though it is just NodeRevisionController renamed. I have the git settings done correctly but the differences are higher than the git threshold for treating a file as renamed :)

There's not much in those controller that isn't generic. So we could consider to provide generic implementations of those, just like we completely replace the revision overview anyway.

If we do that, then we need to think about route names for them though. Note that it's not entity_type_id.revision_show, it's module.revision_show, module and entity_type_id just happen to be the same string in many cases. So if we provide generic versions, we need to think about namespacing them correctly. Unfortunately, a lot of route names weren't standardized on entity.entity_type_id.link_template, but we could either use that, or something like diff.entity_type_id.revision_show, to avoid clashes with future core versions. The handler would then a) decide which routes need to be defined if any, and b) return the route name/object for each link. Yes, those handlers are going to have lots and lots of methods, but that really because core didn't manage to introduce any kind of consistency/standardization for revision UI's.

On rename: Try to experiment with -MX, where X is the amount of a file that still needs to be the same for git to treat it a move. -M40 will consider it a move when at least 40% of the file is still the same.

So, then it seems that in order to advance with this one we would need to work on handlers (at least on the blocking ones here which are the ones generating/providing the routes for viewing/reverting/deleting a revision).
If we (diff module) write those handlers are we going to write 1 handler / revisionable content entity ?
Are we going to allow some kind of pluggability ?

Here's a relevant core issue: #2350939: Implement a generic revision UI

I'm not exactly sure how the handler structure would look like. But you would start with a base class and an interface, and as needed, add entity type specific implementations. Hopefully with as few overrides as possible.

IMHO this is yet another issue which tries to solve too many things at the same time.

You could totally split up this issue into at least 3 subissues and iterate quicker:

• Provide a generic diff controller (see this patch, which leverages some of the entity godness we already have)
• Provide a route provider to provide the required routes automatically
• Provide a revision history listing with diff support
• Get rid of the custom code for node

This time with a patch

As far as i understood we wanted to postpone this since the entity contrib module is about to add similar parts and diff should then build on top of it?

Exactly, this is the idea of the patch so far:

a) Just provide a diff controller, no actual revision listing controller, as this will be provided by entity api at some point and maybe extended by diff module?
b) Leverage some of the functionality like the conversion of revision IDs to entity revision objects on routes for this diff controller.

Added a subissue to just resolve the diff controller, not the listing itself.

.

Now that #2634212: Offer a diff controller for all entity types leveraging entity API module has landed, supplying both a genericEntityController and a DiffRouteProvider service, we can move forwards with other aspects of this.

For example:
- remove NodeRevisionController.php
- modify the routing.yml to use GenericentityController not NodeRevisionController
- identify which other parts of the #16 patch are still relevant
- identify what core and Entity issues we are still blocked on, if any

I suggest either this issue or a new issue become a meta issue to keep track of these subissues. At the least the present IS needs updating.

Created #2759627: Remove redundant NodeRevisionController as a sub issue.

hello guys I'm not sure to understand what we need for a new custom entiy to get revision tab.
Is there any patch include in dev diff module version or i should apply one of this patch to RC-1 ?

Hello everyone.
I've done some work for adding revision tab for all entities. This patch works for version rc2, but have several problems such as:

• Controller for revision tab works directly with route match, because there is no route enhancer (like for entity view or entity form) to convert entity_type_id parameter to something universal and applicable to universal revision tab controller. But implementing this route enhancer here is not relevant actually...
• Revision overview form mostly based on node revision overview, which means can contain some improper solutions (I mean they need for node, but don't need for another entities)
• Requires entity routes with predefined suffixes, such as entity.entity_type.version_history, entity.entity_type.revision, entity.entity_type.revision_revert, entity.entity_type.revision_delete. All custom entities what I've seen implement these routes, but I didn't see any universal tools in Drupal core to declare them, which means if developer don't copy-paste revisions logic from node, he is free to define his own routes. This can be a problem
• Requires manual adding of the revisions-diff link template
• Requires manual adding of the html_diff route provider
• Does not contain any tests

How to use:

1. Add route provider to your entity: html_diff => Drupal\diff\Routing\DiffRouteProvider
2. Add template link to your entity: revisions-diff => /my-entity/{my_entity}/revisions/view/{left_revision}/{right_revision}/{filter}

I hope this patch will be helpful for someone

It's also worth noting that the Entity API module has a similar issue: #2625122: [Meta] Implement a generic revision UI

Testing #30 on a custom entity, it does works (thanks!), but sometimes I get a timeout error. Entity has lot of fields. I did increased resources while testing locally, and it does loads, but it takes a long time. Any suggestions?

• The application did not respond in time.
• Maximum execution time of 120 seconds exceeded (vendor/caxy/php-htmldiff/lib/Caxy/HtmlDiff/HtmlDiff.php:853)
• PHP Memory Limit: 256M (pantheon professional plan)
• max_execution_time: 120
• Drupal version: 8.6.2

If #2350939: Implement a generic revision UI is committed this won't be necessary, might be worth postponing this issue and then either repurposing or closing this issue once that issue makes it in.

Hi @AaronMcHale,

Currently, the issue you related is postponed too, so what will happen with this improvement if we postpone it, is there someone working on this?

There is another issue from contrib Entity API module #2625122: [Meta] Implement a generic revision UI.

@jidrone #2350939: Implement a generic revision UI is postponed on #3043321: Use generic access API for node and media revision UI and possibly also on #2927077: $entity->toUrl('revision-*') should fill revision parameter on all applicable routes..

So once those two issues are in then work can continue on #2350939: Implement a generic revision UI, but there isn't much progress at the moment. If I had more time myself I'd try and push forward on those, so any help to get either of those two blocking issues moved forward would be great.

I'm not involved in the Entity API module issue #2625122: [Meta] Implement a generic revision UI so can't provide any context regarding the progress of that. Ideally it would be great if any available resources that people have could be focused on the Core issues, that way everyone benefits in the long run.

#2350939: Implement a generic revision UI is in a very good state now, only blocked on 1 other issue #3043321: Use generic access API for node and media revision UI

We are using this generic UI for blocks and other entity types with modules like https://www.drupal.org/project/block_content_revision_ui

This issue should move towards extending the new generic revision UI instead.

I came across this issue while trying out the media_revisions_ui module, where I also wanted to add diff support for within Drupal 9.2.6. The proposed patches almost work, so I created a new one that adds the following changes in order to make it work again:

• Use getRawParameter() instead of getParameter() in EntityRevisionController::revisionOverview() because the parameter is upcasted now and would result in a fatal error, when the entity object is passed to entity storage's load() method
• Using the correct route name for entity.{$entity_type}.revision_revert_translation_confirm in EntityRevisionOverviewForm::buildForm()
• Using the correct route name for entity.{$entity_type}. revision_revert_confirm in EntityRevisionOverviewForm::buildForm()
• Using the correct route name for entity.{$entity_type}.revision_delete_confirm in EntityRevisionOverviewForm::buildForm()

So this only makes the patch functional again, but does not add any proposed changes (e.g. those from comment #33) and no additional test coverage yet, but in my opinion it would be great to use this ticket to prepare the diff module to be generic enough to let all entity types can have support for it on their revision pages. Those changes should be added first, before reworking the current code that sometimes only targets nodes exclusively - which should be much easier then, when all the generic code is already available.

#2350939: Implement a generic revision UI has landed in core! 🎉

Change record: https://www.drupal.org/node/3160443

Rerolled #40 and moved to an MR
Hiding patches

Updating title to reflect this isn't a meta, there's code.

Will continue with tests tomorrow

Updated issue summary
This now includes tests and is ready for review
Testing on a client project its working nice 🙌

I just tested out the issue fork and it works like a charm - only found a problem with custom block content entities. The revision overview seems to work as expected (with the radio buttons to compare selected revisions), but when clicking Compare selected revisions, I get this error:

Drupal\Core\Entity\Exception\UndefinedLinkTemplateException: No link template 'revision' found for the 'block_content' entity type in Drupal\Core\Entity\EntityBase->toUrl() (line 211 of core/lib/Drupal/Core/Entity/EntityBase.php).

This looks great, thanks to everyone who worked on it.

@larowlan do you think we should add a test case for media revisions as well, so all core entity types with the generic revision UI are supported?

@hctom can you provide more info about your Drupal version and any other steps to reproduce it? The test coverage suggests that block content is working so perhaps you have a patch or custom/contrib module that is interfering?

Can do for media
I did have that issue with block content before one of the later pushes, so perhaps @hctom was testing an earlier version.
I fixed it whilst adding the tests.

I used the latest patch on a Drupal 10.3.6 install with nothing fancy in there. It's just a block content entity bundle with a custom bundle class.
I also updated the patch with the latest changes now, but the errors persists - and as far as I see, the tests do not show any problems, because they only test the revision overview currently and not the actual revision comparison, where this error occurres.

I was able to reproduce @hctom's results
Expanded the tests and fixed the issue

Back to needs review

Just checked the new changes by larowlan and I can confirm: comparing revisions of block content entities is now working as expected. Thank you!

Rebase is failing tests

Back to green, would be good to get another review on this

Exported MR into patch file.

Responding to acbramley's comment, I took a first pass at de-duplicating the entity form and node form. The node form is now a subclass of the entity form, with a few selective overrides as needed to preserve the former behavior. Also resolved the differences in the table layouts, so that the entity form now looks and acts like the node form did before these changes.

Re-rolled patch based on MR!112, to apply against latest release (2.0.0-beta4)

There are tests failing now.

There seem to be few couple legit test fails.

1. Some strangeness coming from translations; the entity revision overview form does a count on revisions and then later on seems to filter some out of the table, resulting in an incorrect placement of the "Compare revisions" button
2. I don't really understand the change to the route provider regarding the _entity_access -- I didn't see any conversations about changing that expectation in this issue.

PHPUnit is green; just some minor linting to do before it goes all green.

I'll self-assign that for tomorrow AM before sliding over to NR.

...and we're back to green-ish (ignoring the next major stuff).

I want to draw some attention to...

1. A bit of refactoring around the operations links / permissions needed to better align with upstream changes
2. A couple of new phpstan things seemed to leak in that were unrelated to this changeset -- they were benign so I just made the linter happy
3. PHPStan pointed out some points of ambiguity -- I plumbed in the string "Unknown" and a NULL user if a content entity that was passed in somehow doesn't implement the Drupal\Core\Entity\RevisionLogInterface interface. This might be okay, not sure.

Happy to slide this over into Needs Review for more detailed feedback.

Cheers :-)

Patch file comment 70
https://www.drupal.org/files/issues/2026-03-23/diff2x_block_content_revi...

This patch is identical to the MR as it was March 17th 2026

Please merge MR112, it is VERY good and desperately needed.

It's been 11 years and the quality is excellent. Let's merge MR 112 please and thank you and please also tag a new release. I recommend 2.0.0-rc1

Needs reroll due conflicting changes that were merged into 2.x with #3568877: Remove $previous_revision handling from RevisionOverviewForm
Significant merge conflicts.

Fixed the conflicts and moved the code around but was getting test fails locally, I've run out of time to look into it today.

MR is green but there are still 2 open threads that need actioning.

Hey Adam,

I'm still more than willing to knock those threads out, but I had a question on one of them. You mentioned that changing the form build array key would be a B/C break that you don't want to commit.

Are you okay with this changing to "{$entity_type_id}_revisions_table" and updating any tests that fail on account of the change?

Are you okay with this changing to "{$entity_type_id}_revisions_table" and updating any tests that fail on account of the change?

Sorry yes I should have replied to the thread, I think that's fine.

Self-assigned for a team co-working session this afternoon.

Sending back to NR; one thread should be resolvable now and the other needs a response.

Thanks for all you do to keep diff chugging along, Adam.

This is looking good now, it would be good to have someone (especially anyone with a large site with translations) to test this on multiple entity types.

The last thing to figure out is how best to roll this out without being disruptive.

We currently don't have a stable release for 2.x so the options are:

1. Merge this and release in 2.0.0
2. Tag 2.0.0 now, then merge this and release as 2.1.0 (new minor)
3. Tag 2.0.0 now, then merge this and release as 3.0.0 (new major)

Given the scope of changes here I am leaning towards a new major.

I can give the changes a test run given I operate diff_plus (happy to make any changes necessary over there!). We also have a pile of custom entity types. The one thing I can't test is translations though.

I'm leaning towards keeping with 2.x because I don't see anything that's public API changing. Plugins, forms, and controllers aren't strictly covered by B/C guarantees last I was aware...and the benefits here outweigh the risk 👍.

I'm also +1 on using link templates exclusively in the generic form and specializing the node form for the revision revert and delete links. That seems like it's setting up forward compatibility.

NW for https://git.drupalcode.org/project/diff/-/merge_requests/112#note_732050

Self-assign for #83.

https://git.drupalcode.org/project/diff/-/merge_requests/112#note_732050 should be taken care of now, but manual testing revealed another bug that I missed thus far.

There also appears to be new phpstan fails in HEAD. I'm leaving as NW because IMO there should be new test coverage set up to ensure operations links appear appropriately. I'm out of time for tonight, but would be more than happy to contribute a new test tomorrow if no one picks it up by then.

Fixed revert link text handling and added more test coverage for that.

Looks great from my house! I haven't tested against an application with translations though.

See additional test fails added in https://www.drupal.org/project/diff/issues/2452523#mr112-note737858 (pipeline https://git.drupalcode.org/issue/diff-2452523/-/jobs/9249862).

Self-assigning for a deeper dive into resolving the new tests and not messing anything else up. I'll provide the inter-diff once things go green on D.O. again...but I have a feeling that the ::buildForm method needs tweaked a bit more, which I'll try to get in place today.

Alright, sliding back over to NR status. The changes made since last week include https://git.drupalcode.org/project/diff/-/merge_requests/112/diffs?diff_...

1. Two new test cases
2. Some safety added around ancient revisions w/o a creation timestamp (prior to 10.2)
3. Some light refactoring in the EntityRevisionOverviewForm::buildForm method to make the UX a little less strange in some rare scenarios (but still not perfect)

There is still one glaring issue, but I do not know any way around it. Taking the steps to reproduce from the last review...

1. Create a block
2. Re-save the block without changing any data
3. There are now two revisions, but only one with the revision_translation_affected flag set (only one record returned by the entity query).

in this situation we're left with:

which I believe is a core bug to fix. Overall though, I feel the tweaks to the form make things more simple.

Cheers.

For completeness, here's a screenshot of the core block revision ui in the strange case above. Note there isn't really a UX difference between the diff implementation in which revisions are displayed and what their operations are.