Convert tracker listings to a view

Actually, let's be bolder. There's no reason to maintain the legacy listing since tracker is a feature module.

Changing the filter to use the tracker_user table alters explain results to:

+----+-------------+-------------------------+--------+------------------------------+---------+---------+
----------------------------+------+---------------------------------+
| id | select_type | table                   | type   | possible_keys                | key     | key_len | ref                        | rows | Extra                           |
+----+-------------+-------------------------+--------+------------------------------+---------+---------+
----------------------------+------+---------------------------------+
|  1 | SIMPLE      | node                    | ALL    | PRIMARY                      | NULL    | NULL    | NULL                       |    2 | Using temporary; Using filesort |
|  1 | SIMPLE      | node_field_data         | ref    | PRIMARY,node_status_type,nid | PRIMARY | 4       | d8_dev.node.nid            |    1 | Using where                     |
|  1 | SIMPLE      | users_node_field_data   | eq_ref | PRIMARY                      | PRIMARY | 4       | d8_dev.node_field_data.uid |    1 |                                 |
|  1 | SIMPLE      | tracker_user            | eq_ref | PRIMARY,tracker              | PRIMARY | 8       | d8_dev.node.nid,const      |    1 | Using index                     |
|  1 | SIMPLE      | node_comment_statistics | ALL    | PRIMARY                      | NULL    | NULL    | NULL                       |    2 | Using where; Using join buffer  |
|  1 | SIMPLE      | history                 | eq_ref | PRIMARY,nid                  | PRIMARY | 8       | const,d8_dev.node.nid      |    1 |                                 |
+----+-------------+-------------------------+--------+------------------------------+---------+---------+
----------------------------+------+---------------------------------+

Where it was PRIMARY and DEPENDENT SUBQUERY before.

Looking at making a patch.

Attached patch which still needs work. Three known issues. I don't know if anyone has any thoughts:-

• The user/%/track page needs an additional check on view the entity. in the present tracker the check is both 'access content' and view user. So that's an additional 'permission to view user' on the 'Tracker - User: ...' contextual filter (which seems less obvious than the taxonomy/term/% case); or #2029509: Add a generic entity argument validation plugin. with the ability to have both view content permission, and access user entity.
• The patch fails testing by not implementing a tab, tracker/%current_user. There is tracker/%user generally (it's the same as user/%/track - so it could actually be removed - permission note above also valid here). It would be easy to make another page tracker/my as a tab where the argument is specified as current user. If following what is already in core is most important this should be done, one more page display - nothing really. It really is duplicated user/%own_user/track though - even in core now.
• The patch fails testing on: line 279 Drupal\tracker\Tests\TrackerTest->testTrackerAdminUnpublish(). For some reason in simpletest there is no table at all displayed if there are no results. If I repeat the test manually (create a node, delete a node, view the tracker table) the view table is displayed with the 'No results' text area shown as the row. I'm not sure what the difference is. Any experienced anything similar?

Nice, thanks @ekes!

This is just an FYI that I am working on this issue to upgrade the patch to the current state of D8.

Here's my attempt so far.

Here are the issues I know about:
* When showing the count of new comments, I get a WSOD and an error similar to this in the Apache logs: "PHP Fatal error: Call to undefined method stdClass::id() in /var/www/vcs/git/drupal/drupal/8.x/core/modules/comment/lib/Drupal/comment/Plugin/views/field/NodeNewComments.php on line 130, referer: http://localhost/drupalsandboxes/drupal-8.x/"
* When removing the tracker.routing.yml file, I got Symfony errors trying to access anything in user other than user/$uid/track, and so I've removed the controllers, since they aren't being used and removing them doesn't appear to cause problems, but I've left the routing and local_task YAML files.
* Local tests failed on a couple of things that were clearly working. I'm hoping those tests won't fail here.

Here's a more improved view that more closely resembles the tables in the D7 version. The tests still fail, but I'm not sure how to correct them. I've tweaked the view and the tests so they better match each other, but Simpletest still doesn't see the results correctly.

First a re-roll cause the patch doesn't apply to the last 8.x

So my interdiff is quite hard to reproduce.

Let's at least see what the test bot has to say.

@oadaeh, that patch is a big fail. I did a wrong stuff with git and did commit the new file and did a mess.

If I have change today I'll fix it, and I did some research and I guess I did find the problem. (for the test stuff)

.

Reroll of #14.

The patches in #14 and #19 are incomplete.

Note: converting the tracker page to a view would fix IMHO a security issue of not checking the access to those base fields.

+1 to the security hardening here - bumping it to major for that reason

As an FYI, this will have to be restarted from scratch, as the most recent patch is so far out of date, it doesn't even pretend to apply. I started it on Sprint day, but I had too many other distractions and didn't make very much headway.

I gave the reroll a try and updated the view. Let's get the ball rolling again :)

Some fixes, I guess that if we're not providing a legacy mode, the routing.yml file should be removed.
There are a bunch of tests accessing "activity" still, I guess they should go to tracker instead.

Ooops, interdiff.

Some progress but still far fetched.

I've removed the routes and the links, I think that should probably happen on the view.

In hook_help there's this but tracker.page is not a route, what's the preferred option for arbitrary internal urls? (does views generate a route?) the Change record doesn't seem to mention it https://www.drupal.org/node/2346779

      $output .= '<dd>' . t('The <a href="!recent">Recent content</a> page shows new and updated content in reverse chronological order, listing the content type, title, author\'s name, number of comments, and time of last update. Content is considered updated when changes occur in the text, or when new comments are added. The <em>My recent content</em> tab limits the list to the currently logged-in user.', array('!recent' => \Drupal::url('tracker.page'))) . '</dd>';

Latest patch no longer even applied. Rerolled to check how it fares now.

1. +++ b/core/modules/tracker/config/install/views.view.tracker.yml
   @@ -0,0 +1,806 @@
   +      access:
   +        type: perm
   +        options:
   +          perm: 'access content'
   
   +++ /dev/null
   @@ -1,26 +0,0 @@
   -  requirements:
   -    _permission: 'access content'
   -    _access_tracker_own_information: 'TRUE'
   

   This is the only access checking we do ... we need so a test to ensure that it works.

2. +++ b/core/modules/tracker/config/install/views.view.tracker.yml
   @@ -0,0 +1,806 @@
   +    position: 2
   +    display_options:
   +      path: user/%/activity
   ...
   +        access: true
   
   +++ /dev/null
   @@ -1,26 +0,0 @@
   -  requirements:
   -    _permission: 'access content'
   -    _entity_access: 'user.view'
   

   Maybe I'm stupid but we seem to not care about the level of access control ...

Assigning to self to avoid someone else doing duplicate work.

This seems to work locally, do tests pass ?

@dawehner : all displays have access control based on perm: access content.

Some (all ?) fails were due to views' format changes. This should fail less.

+++ b/core/modules/tracker/config/install/views.view.tracker.yml
@@ -0,0 +1,835 @@
+          field: last_comment_timestamp

Looks the issue depends on related #2086125: Last read comment field/filter/argument uses still the node.changed instead of node_field_data.changed column

More fixes.

Much better, and it confirms andypost's remark about the comment statistics issue.

This should fix all the fails on the NodeAccessTest...

This restores the tabs, so should fixe some more errors.

This removes most exceptions and some of the errors locally. What does it give here ?

No time for more right now, but this should remove one fail.

Just to be sure, part of one test removed : the case when comment module is there (it's a dependency for tracker) but comments are disabled.

This should pass, and is to make sure the problem is isolated.

One more of the test functions should now pass.

One more complete test should now be passing: the regex for new nodes had been wrong all the time.

Expecting one less fail.

One less fail indeed, but as Drupalcon is over, I will not be able to continue on this in the next few days, so unassigning myself for now.

Suppose that should go into 8.1

This issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.

Had a go at re-rolling this but it may be worth just starting from a new patch at this point. Adding the tag for visibility.

Patch re-rolled, Please review.

Re-rolling the patch against 9.1.x-dev.

rerolled for 9.5

phpcs fix

Another phpcs

So I believe all patches after #75 should be ignored. Seems after 75 the view itself was removed and if that wasn't noticed then these patches probably need more work.

Know the help text in tracker.module isn't functional because it's pointing to a route that is now being deleted.
The view itself probably needs to be recreated
Will need an install hook to import this new view.

Am I forgetting anything?

Updated the issue summary

Deleted TrackerController as that doesn't appear to be needed with tracker.routing being deleted.
Scanned the core repo looking for tracker routes and believe they've all been updated.
Updated the view to be more simple. Removed page_3 as it appeared ot be a duplicate of page_2

Just need to write an update hook

Attaching a work in progress patch for any early review.

This extension is deprecated and scheduled for removal in Drupal 11.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

It will be moved to the contributed extension once the Drupal 11 branch is open.