Needs review
Project:
Activity Tracker
Version:
1.0.x-dev
Component:
Code
Priority:
Major
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
13 Mar 2013 at 14:18 UTC
Updated:
4 Jan 2026 at 18:02 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
damiankloip commentedComment #2
xjmComment #3
xjmActually, let's be bolder. There's no reason to maintain the legacy listing since tracker is a feature module.
Comment #4
ekes commentedChanging the filter to use the tracker_user table alters
explainresults to:Where it was PRIMARY and DEPENDENT SUBQUERY before.
Looking at making a patch.
Comment #5
ekes commentedAttached patch which still needs work. Three known issues. I don't know if anyone has any thoughts:-
user/%/trackpage 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 thetaxonomy/term/%case); or #2029509: Add a generic entity argument validation plugin. with the ability to have both view content permission, and access user entity.tracker/%current_user. There istracker/%usergenerally (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 pagetracker/myas 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.Comment #6
xjmNice, thanks @ekes!
Comment #8
xjmComment #9
oadaeh commentedThis is just an FYI that I am working on this issue to upgrade the patch to the current state of D8.
Comment #10
oadaeh commentedHere'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.
Comment #12
oadaeh commentedHere'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.
Comment #14
killua99 commentedFirst a re-roll cause the patch doesn't apply to the last 8.x
So my interdiff is quite hard to reproduce.
Comment #15
oadaeh commentedLet's at least see what the test bot has to say.
Comment #17
killua99 commented@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)
Comment #18
dawehner.
Comment #19
amitgoyal commentedReroll of #14.
Comment #21
oadaeh commentedThe patches in #14 and #19 are incomplete.
Comment #22
oadaeh commentedComment #23
oadaeh commentedComment #24
dawehnerNote: converting the tracker page to a view would fix IMHO a security issue of not checking the access to those base fields.
Comment #25
larowlan+1 to the security hardening here - bumping it to major for that reason
Comment #26
oadaeh commentedAs 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.
Comment #27
pcambraI gave the reroll a try and updated the view. Let's get the ball rolling again :)
Comment #29
pcambraSome 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.
Comment #30
pcambraOoops, interdiff.
Comment #32
pcambraSome 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
Comment #34
fgmLatest patch no longer even applied. Rerolled to check how it fares now.
Comment #35
dawehnerThis is the only access checking we do ... we need so a test to ensure that it works.
Maybe I'm stupid but we seem to not care about the level of access control ...
Comment #38
fgmAssigning to self to avoid someone else doing duplicate work.
Comment #39
fgmThis seems to work locally, do tests pass ?
@dawehner : all displays have access control based on
perm: access content.Comment #41
fgmSome (all ?) fails were due to views' format changes. This should fail less.
Comment #42
andypostLooks the issue depends on related #2086125: Last read comment field/filter/argument uses still the node.changed instead of node_field_data.changed column
Comment #44
fgmMore fixes.
Comment #46
fgmMuch better, and it confirms andypost's remark about the comment statistics issue.
Comment #47
fgmThis should fix all the fails on the NodeAccessTest...
Comment #49
fgmThis restores the tabs, so should fixe some more errors.
Comment #51
fgmThis removes most exceptions and some of the errors locally. What does it give here ?
Comment #53
fgmNo time for more right now, but this should remove one fail.
Comment #55
fgmJust 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.
Comment #57
fgmOne more of the test functions should now pass.
Comment #59
fgmOne more complete test should now be passing: the regex for new nodes had been wrong all the time.
Comment #61
fgmExpecting one less fail.
Comment #63
fgmOne 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.
Comment #66
andypostSuppose that should go into 8.1
Comment #67
gaydabura commentedComment #69
xjmThis 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.
Comment #70
gaydabura commentedComment #73
manuel garcia commentedHad 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.
Comment #74
manuel garcia commentedComment #75
pk188 commentedComment #77
pk188 commentedComment #84
vsujeetkumar commentedPatch re-rolled, Please review.
Comment #86
hardik_patel_12 commentedRe-rolling the patch against 9.1.x-dev.
Comment #92
smustgrave commentedrerolled for 9.5
Comment #93
smustgrave commentedphpcs fix
Comment #94
smustgrave commentedAnother phpcs
Comment #96
smustgrave commentedSo 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?
Comment #97
smustgrave commentedComment #98
smustgrave commentedUpdated 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.
Comment #100
quietone commentedThis 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.
Comment #102
andypostnow in contrib
Comment #104
batigolixComment #105
batigolixComment #106
batigolixThe patch was written when the module was still in core.
I rewrote it with the help of an AI agent, so that the changes can be applied.
This can be reviewed:
- 3 views are available for pages: /activity /activity/{userid} and /user/{userid}/activity
- a permission is added for accessing these pages
- UI should stay similar
Comment #107
batigolix