Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
See #928110: Expose tracker2 tables and columns to views for background.
dawehner said he doesn't want any more context than that... blame him. ;)
Cheers,
-Derek
Comment | File | Size | Author |
---|---|---|---|
#32 | drupal-1829734-32.patch | 16.8 KB | dawehner |
#32 | interdiff.txt | 1.01 KB | dawehner |
#30 | interdiff.txt | 1.05 KB | dawehner |
#30 | drupal-1829734-30.patch | 16.8 KB | dawehner |
#28 | drupal-1829734-28.patch | 16.83 KB | dawehner |
Comments
Comment #1
dawehnerLet's just do that.
Comment #2
dwwI just tried testing this on a local site. Fundamentally, it's working (at least the uid_touch replacement, which is the main thing I care about). A few things I found in the process:
A)
B) UI-wise, I was expecting the uid_touch handler to live in the 'Tracker - User' group, not directly in 'Tracker' (since it's user-centric tracker data).
Otherwise, it all seems to be working great in my mild testing (I also added all the 'Tracker - User' fields and they're working, too).
Comment #3
dwwLike so. Tested somewhat extensively. ;) This is working for me locally to resolve #1828756: D7 views don't honor tracker and issue following.
Comment #4
dwwDoh! The patch would help. ;)
Comment #5
dawehnerMh it seems to be that parts of my php configuration is a bit borked.
Your changes look perfect, so I committed and pushed it, to help the drupal.org update.
Let's push this to d8.
Comment #6
dwwSweet, thanks! I don't have bandwidth to re-roll this for core, but it should be fairly straight-forward.
Yay,
-Derek
Comment #7
dawehnerSo here we go.
Comment #8
xjmAh-ha, it was hiding out in the Views queue. :)
Just a quick note since this has come up in a couple patches in a row. :) The doxygen standard say these summaries should start with a third-person verb, as in "Defines a UID filter for nodes that the user posted or commented on" and "Implements hook_views_data()."
Also moving to the correct queue, tagging for test coverage, and adding to the meta at #1853522: [META] (Re)introduce Views data integration for core modules.
Comment #9
xjmRelated issue: #1853572: Remove the old default tracker view
Comment #10
dawehnerOh right, this happens if you steal some code from contrib :)
Comment #11
Cameron Tod CreditAttribution: Cameron Tod commentedReupping to retest.
Comment #13
Cameron Tod CreditAttribution: Cameron Tod commentedLet's try this.
Comment #14
dawehner#13: 1829734-VDC-tracker_data-13.patch queued for re-testing.
Comment #15
damiankloip CreditAttribution: damiankloip commentedThis needs a 'module' key too.
same here.
and here.
Implements ...
We can remove these now.
Also, the BooleanOperator class has a plugin id of 'boolean_operator' but this is not being used in the views data. However, 'tracker_boolean' is. I guess that's what the plugin id is meant to be?
Comment #16
damiankloip CreditAttribution: damiankloip commentedI guess that also confirms we need the extra tests for this :)
Comment #17
damiankloip CreditAttribution: damiankloip commentedThinking about it, do we even need this handler? I think the BooleanOperator can handle most of this? If not, could we add what we need to that, as it won't really be much..
Comment #18
dawehnerYou are so right, ... this file is not needed anymore.
Comment #19
damiankloip CreditAttribution: damiankloip commented#18: drupal-1829734-18.patch queued for re-testing.
Comment #20
damiankloip CreditAttribution: damiankloip commentedLet's see what happens, but this is looking good. I created #1941830: Convert tracker listings to a view to create a new default tracker view.
I might look at some test coverage.
Comment #22
damiankloip CreditAttribution: damiankloip commentedLet's try this out. I have:
And, sorry, forgot an interdiff!
Comment #23
damiankloip CreditAttribution: damiankloip commentedMaybe just use removeItem in the test instead.
Comment #24
damiankloip CreditAttribution: damiankloip commented.
Comment #25
dawehnerWell, the reason to name have comment in the name was that this tracker not only takes care of the UID (which often just refers to the author)
but yeah I don't care.
Thanks for continuing to work on that.
Why not use entity_create or similar instead of executing a post request, this seems to be too much.
Just a nitpick, but we seem to destroy after executing and before something else. This seems to make it a bit easier to read.
Shouldn't we also check the actual result?
Comment #26
damiankloip CreditAttribution: damiankloip commentedThanks! All good points, The test coverage was thrown together pretty quickly, so yeah some of those improvements are welcome!
Comment #27
tim.plunkettThis 'as' is redundant now
Do you actually use $this->user elsewhere? If so, it should be a protected property above the method. If not, just have
$this->drupalLogin($this->drupalCreateUser($permissions));
Implements
Comment #28
dawehnerLet's fix the classname and some other stuff as well.
Comment #30
dawehnerUps, failed to convert the annotation properly.
Comment #32
dawehnerThat's not complicated to fix.
Comment #33
tim.plunkettLooking good! No complaints
Comment #34
dwwYes, looks great to me, too. Happy to see my code from tracker2 finally getting into core, now with tests and lots of other goodness. :)
Thanks!
-Derek
Comment #35
xjmThese are for catch. :)
Comment #36
catchCommitted/pushed to 8.x, thanks!