When working on aggregated Views for Commerce Reports, I noticed some weird behaviors with our Price field. Output would be fine when the normal "group", but once I changed to SUM or AVG it would render as a numeric/integer field and lose all formatting. I did some digging and found out the Display plugin overrides the handler based on information in \Drupal\views\Plugin\views\query\Sql::getAggregationInfo.
The problem is that \Drupal\views\Plugin\views\field\FieldPluginBase::addAdditionalFields applies that grouping to each additional field. In the case of an entity field that means it tries to SUM/AVG the bundle, entity ID, or any other field columns. In our case, it kills the currency code.
Upon reviewing the code I see:
$group_params = [];
if ($this->options['group_type'] != 'group') {
$group_params = [
'function' => $this->options['group_type'],
];
}
if (!empty($fields) && is_array($fields)) {
....
$params += $group_params;
$this->aliases[$identifier] = $this->query->addField($table_alias, $info['field'], NULL, $params);
As you can see in this screenshot, the langcode, bundle, and currency_code become "0" since it performs SUM() on a string.
Views should not apply the group/aggregate function on all additional fields.
I don't know if this is considered BC or not. None of the additional fields are rendered or ever managed (or ever had valid data.)
Create patch
Discuss if BC
Hopefully fix?
Needs work
11.0 🔥
Last updated
Used to track the progress of issues reviewed by the Drupal Needs Review Queue Initiative.
The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.
Think next step would be adding a test case to show this issue.
Patch in #3 still gets applied on 10.1, 10.2 and 11.x.