Create a view for content revisions

Looks great. Next step would be to add hook_view_data_alter in $module.views.inc file with disable revert data on node you can have a look at $data['node_field_revision']['link_to_revision'] in NodeViewData.

Working on this.

Here we go.

Added sorting, page title and set page as a menu tab.
Injected dependencies and updated plugin.

Before enabling view.

After enabling view.

Do we want to get this in now or need tests first?

Looks good

Some functional tests would be great. Let's wait for @dawehner review maybe UX review if possible.

The patch looks pretty reasonable. Also sorting by changed on the revisions is exactly what you need.
So yeah maybe some form of test coverage would be nice. https://github.com/Jaesin/content_entity_base/blob/master/tests/src/Kern... tests something really similar already.

It is passing now so add as much asserts as you like.

• Fixed schema.
• Fixed permissions.
• Fixed tests.

Scope of test should be public.

Turns out we don' t have to call $node->setNewRevision(TRUE);. moderation_state_node_presave() is handling that.

+++ b/src/Tests/ModerationStateTestTrait.php
@@ -0,0 +1,38 @@
+    $role->grantPermission(sprintf('create %s content', $content_type_id));
+    $role->grantPermission(sprintf('edit any %s content', $content_type_id));

IMHO sprintf here is not worth it, just concat the strings as needed. Sprinf is IMHO for more generic functionality, where you have arbitrary strings and arguments.

 +    $this->drupalLogin($this->adminUser);
+    $node = $this->drupalCreateNode([
+      'type' => 'moderated_content',
+      'moderation_state' => 'draft'
+    ]);
+    $node->moderation_state = 'needs_review';
+    $node->save();
+    $node->moderation_state = 'published';
+    $node->save();
+    $this->drupalGet('node/' . $node->id() . '/revisions');
+    $this->assertResponse(200);
 

You should setup individual timestamps if you care about order.

honestly I would just hardcode a number and not use time()
Should I give that a try and debug that?

+      sorts:
+        changed:
+          id: changed
+          table: node_field_revision
+          field: changed

This is not really the field you are looking for. This is the revisioned changed field, not the time when the revision got created (according to the api).
{node_revision}.revision_timestamp is what you ideally look forward to.

Well, sounds easy, let's just use that in views, done.

In reality its a bit different. It turns out the join from {node_field_revision} to {node_revision} is defined the wrong way round,
so that field never appears in the UI because views cannot resolve the dependency path.

looking

This passes for me, pushed to branch

Uses the UI instead of the API, and in doing so increases our test coverage - found a bug with permission in the widget and a bug with _original_delta in the widget.

Fixed both in process.

I'm sorry but we should sort by the right value, not what works because of implementation details. The revision_timestamp field is supposed to be used IMHO here

Sure this base work is great, maybe add a TODO to switch over once #2630886: Correct the join from revision data table to revision base table is in.
IMHO using UI only tests is the wrong thing given we rely on the wrong assumptions.

@larowlan
Are you sure the interdiff is the right one?

Thanks @larowlan for looking into it. Nice feedback @dawehner.

The revision_timestamp field is supposed to be used IMHO here

I agree so I applied #2630886-3: Correct the join from revision data table to revision base table and updated the view.

IMHO using UI only tests is the wrong thing given we rely on the wrong assumptions.

Reverted that part from the test.

Uses the UI instead of the API, and in doing so increases our test coverage - found a bug with permission in the widget and a bug with _original_delta in the widget.

@larowlan can you please create a new issue for this?

This is done imo. We are postpone on #2630886: Correct the join from revision data table to revision base table.

Nice work @kim.pepper can you reroll core patch in #1863906: [PP-1] Replace content revision table with a view as well?

Looks good to go for me!

Needs reroll.

Have we investigated how this view plays with Diff? I just tested 8.x-1.x with Diff module, and it works swimmingly. If we're going to take over the Revisions tab, we need to make sure Diff continues to work.

This is indeed a tough question.

Diff module, by construction, needs a form in order to be able to do its comparison feature (at least for now).
Currently diff just takes over the existing route from node module (but we are working on something else: https://www.drupal.org/node/2634212 ).
Once we have a generic revisions overview route (https://github.com/fago/entity/pull/18 tries to do that), diff would probably still override the entire controller.

My general thought was that with views being quite flexible someone could make a diff <-> views integration so you can configure a view which looks like a diff (through the same mechanism as exposed filter), but NOT a automatic working solution. N^2 integrations are hard.

There we go #2636406: Add a views form integration for diff exists now. I would argue given that we could proceed from here on.

For other entity types someone could write something like a views generator (wizard *hint *hint*) which makes that easier.

To ensure no loss of functionality, I'm going to mark this as postponed on that one. But yes, I like the approach of just making Diff cleaner.

Feels like process for the sake of process.
This patch would be still identical, people would just configure it different when they have diff enabled on their side.
If they want the diff controller instead, they would just disable the view provided by moderation_state and be done.

1. +++ b/config/optional/views.view.content_revisions.yml
   @@ -0,0 +1,715 @@
   +  module:
   +    - moderation_state
   

   we need to adapt that

2. +++ b/src/Plugin/views/field/RevisionLinkSetCurrent.php
   @@ -0,0 +1,96 @@
   + */
   +class RevisionLinkSetCurrent extends RevisionLinkRevert {
   +
   

   Ideally we would write that class independent from the entity type

3. +++ b/src/Tests/ModerationStateTestTrait.php
   @@ -0,0 +1,38 @@
   + * @file
   + * Contains Drupal\moderation_state\Tests\ModerationStateTestTrait
   + */
   +
   +namespace Drupal\moderation_state\Tests;
   

   namespace stuff.

#2630886: Correct the join from revision data table to revision base table is in.

@catch just recommitted it.

Does anyone have a status of this issue?

It's on hold until Diff module figures out what it's doing, so that we don't bump into Diff. I don't want to add a View here that ends up being incompatible with Diff.

Also, this patch is probably needed, too: #2674520: Add current revision filter to views.

I see three ways to tackle this:

1. We could either wait for this core issue that turns the listing into a view: #1863906: [PP-1] Replace content revision table with a view.
2. @dawehner sent at patch that does the same thing at the Diff issue queue: #2636406: Add a views form integration for diff.
3. @EclipseGC suggested once that we could use a contrib module that solely takes care of this, so then modules such as Diff or WM can depend on it and extend it.

How about if we create a contrib module such as views_revisions that contains the patch at #2636406: Add a views form integration for diff? We could use this module for 8.0.x and 8.1.x, while for 8.2.x we hopefully will have it in core thanks to #1863906: [PP-1] Replace content revision table with a view.

@juampynr, @crell, I think creating the contrib module gives us the most flexibility.

If someone writes such a contrib module, I'm OK with integrating with it. But someone would have to show me how to cleanly insert new columns into someone else's View. :-) (I'd also not make it a dependency, but an optional integration.)

You simply don't. Drupal is a system which allows you to configure how the site behaves, so let's just people use views and be done. If you can configure diff and WB, why would you have not learned views in the first place.

Let's just replace core content revision page here and after #2636406: Add a views form integration for diff we can update the existing view in follow up.

Sorry for the spam, but for someone looking for this, it is now possible without Workflow Moderation with this module proposed to perhaps become part of Diff: https://www.drupal.org/project/diff_moderate