Updated: Comment #N
Problem/Motivation
You get errors if you try to use any aggregation options with some handlers. E.g. it's computed, so we can't use an aggregation method in the query for a field that doesn't exist. Such as Drupal\views\Plugin\views\field\Counter
Proposed resolution
Override the usesGroupBy() method from HandlerBase on all handlers where using aggregation doesn't make sense
Remaining tasks
Find/discuss all handlers this should apply to
User interface changes
Removal of 'Aggregation settings' for some handlers
API changes
None
Related Issues
#2030453: Aggregate field data is not available as tokens for output rewriting
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 2080131.patch | 6.94 KB | damiankloip |
Comments
Comment #1
damiankloip commentedLet's start with these. For easy, here is a list of the handlers I have changed so far (Yes I'm lazy and this is just from the git diff):
- core/modules/comment/lib/Drupal/comment/Plugin/views/field/Comment.php
- core/modules/comment/lib/Drupal/comment/Plugin/views/field/NodeNewComments.php
- core/modules/contextual/lib/Drupal/contextual/Plugin/views/field/ContextualLinks.php
- core/modules/history/lib/Drupal/history/Plugin/views/field/HistoryUserTimestamp.php
- core/modules/history/lib/Drupal/history/Plugin/views/filter/HistoryUserTimestamp.php
- core/modules/node/lib/Drupal/node/Plugin/views/field/Link.php
- core/modules/user/lib/Drupal/user/Plugin/views/field/Link.php
- core/modules/views/lib/Drupal/views/Plugin/views/field/Counter.php
- core/modules/views/lib/Drupal/views/Plugin/views/field/Custom.php
- core/modules/views/lib/Drupal/views/Plugin/views/field/Links.php
- core/modules/views/lib/Drupal/views/Plugin/views/sort/Random.php
Comment #2
dawehner+1 so far.
Comment #3
dawehner+1 should actually be RTBC.
Comment #4
dawehner1: 2080131.patch queued for re-testing.
Comment #5
webchickBased on the sheer number of things opting out, should we instead make this an "opt-in" property? (maybe part of the annotation? not sure)
Comment #6
damiankloip commentedHm, not sure about annotations as we currently use the pluginID annotation for these. Also this patch changes 11 or so handlers. We have many many more than that, so in the scheme of things its not that many. We could maybe look at adding this to views info like we do with a few other properties.
Also, not sure if that is a bit out of scope for this issue. Were just turning off aggregation options in the ui for some handlers here. We could change the mecvhanism , or at least discuss it in afollow up?
Comment #7
xano1: 2080131.patch queued for re-testing.
Comment #8
alexpottCommitted 7d42bc2 and pushed to 8.x. Thanks!