Problem/Motivation

Have view with a decimal field, say A.
Choosing comma as decimal separator.
Choosing 2 in Scale selector (number of digits to the right of the separator).
Result without aggregation correct. All values display with 2 decimals and comma as decimal point.

With aggregation the sum of A display with decimal point and 14 digits left of the separator.
Seams that the up-summing totally ignore the option I chose for the field A in views field selector.

Steps to reproduce

  1. Add a Decimal field, multi value, to a content type
  2. Add a node of that content type, with two values in the decimal field
  3. Create a view for that content type
  4. Enable aggregation
  5. Add the Decimal field to the view and set the Decimal marker to 'Comma' .
  6. In the Aggregation settings, set the Aggregation type to 'Sum'

Proposed resolution

Revert accidental commit of field handlers on aggregation field settings within get_aggregation_info() from this commit:
https://git.drupalcode.org/project/drupal/-/commit/684b4a036e736b16d21a1...

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chrotto created an issue. See original summary.

dagmar’s picture

Version: 8.1.1 » 8.1.x-dev
Component: views_ui.module » views.module

Thanks for your report. Moving to the right component.

MarkoT91’s picture

Assigned: Unassigned » MarkoT91
MarkoT91’s picture

Testing on 8.1.2. Tested also on 8.0.x versions.
This is the problem on views since the first stable release of drupal 8.
This problem occurs when aggregation is set on SUM or AVERAGE aggregation function.

MarkoT91’s picture

Assigned: MarkoT91 » Unassigned
FileSize
94.9 KB

Setting this to unassigned for now. Posted the screenshot of the issue.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

chrotto’s picture

I am now using 8.2.0-rc1 and the wrong decimal handling is still there.
Perhaps the decimal point (point or comma) is not of great priority for most of you out there, but the number of decimals in the up summing must be!

dawehner’s picture

Well, noone had time/motivation to work on this issue :)

chrotto’s picture

So the thing to do is to rewrite the result with Twig.
Solved the number of decimals with {{ field_myfield|round(2, 'common') }}. With a little bit more of Twig perhaps I can manage to also make a comma instead of a point.
Think it is a bit strange if it not is anyone that is interested to make the settings in Views to be right when using aggregate option.

dawehner’s picture

Think it is a bit strange if it not is anyone that is interested to make the settings in Views to be right than using aggregate option.

Well, what about you?

Here is a quick outline of thoughts:

  • \Drupal\views\Plugin\views\field\NumericField::buildOptionsForm has support for float fields, using $this->definition['float']
  • In \Drupal\views\Plugin\views\display\DisplayPluginBase::getHandlers we initialize the fields, and provide a special way for aggregated fields
  • \Drupal\views\Plugin\views\query\Sql::getAggregationInfo defines these special fields
  • We could somehow let those special fields define a definition for those fields

This should be totally doable in a BC compatible way.

chrotto’s picture

Would if I could.
Unfortunately I do not have the skills for it. At most my use of Drupal is depending on UI.

_Archy_’s picture

Assigned: Unassigned » _Archy_
_Archy_’s picture

Assigned: _Archy_ » Unassigned

Unassigning myself for now, but I'll be back.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Mschudders’s picture

It seems like this piece of code is responsible for the bad output.

function template_preprocess_views_view_field(&$variables) {
  $variables['output'] = $variables['field']->advancedRender($variables['row']);
}

It seems to me "Precision should be set."

$this->options['set_precision']

or else you'll get:

$remainder = abs($value) - intval(abs($value));
      $value = $value > 0 ? floor($value) : ceil($value);
      $value = number_format($value, 0, '', $this->options['separator']);
      if ($remainder) {
        // The substr may not be locale safe.
        $value .= $this->options['decimal'] . substr($remainder, 2);
      }

A temporary solution could be:

/**
 * Prepares variables for views field templates.
 *
 * Default template: views-view-field.html.twig.
 *
 * @param array $variables
 *   An associative array containing:
 *   - field: The field handler object for the current field.
 *   - row: Object representing the raw result of the SQL query for the current
 *     field.
 *   - view: Instance of the ViewExecutable object for the parent view.
 */
function THEME_preprocess_views_view_field(&$variables) {
  $variables['field']->options['set_precision'] = TRUE;
  $variables['field']->options['precision'] = 2;
  $variables['output'] = $variables['field']->advancedRender($variables['row']);
}

The solution if we can fix this

ISSUE
In "NumericField.php"

  public function buildOptionsForm(&$form, FormStateInterface $form_state) {
    if (!empty($this->definition['float'])) {

This is actually not set if your field is a decimal or a float. :/ (I think it should be ?)
and this allows you to set the precision and other configuration via the views interface and thus resolving the issue. I could put the if switch in comment, but I don't know how it should exactly work.

And I don't know where "definition" is set actually :/

Ben Greenberg’s picture

Solved the number of decimals with {{ field_myfield|round(2, 'common') }}. With a little bit more of Twig perhaps I can manage to also make a comma instead of a point.

Try using Twig's "number_format" filter instead of "round":
{{ field_myfield|number_format(2, '.', ',') }}

Arguments:

  • decimal: The number of decimal points to display
  • decimal_point: The character(s) to use for the decimal point
  • thousand_sep: The character(s) to use for the thousands separator

From: http://twig.sensiolabs.org/doc/2.x/filters/number_format.html

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Pancho’s picture

Issue tags: +views aggregation
chrotto’s picture

Looks like Mschudders have some good thinking about this. Is there not anyone who can help him with a solution to this.

nikita_tt’s picture

FileSize
341.12 KB

If aggregation is enabled then views field definition will be overriden here \Drupal\views\Plugin\views\query\Sql::getAggregationInfo() (See screenshot for more details).

If you want to have an ability to set "Precision", "Thousands marker", "Decimal point" then you need to enable additional options for the float numbers. You can do this by adding "float" option to your field definition. This can be done at least on hook_views_data_alter().

Here is an example:

function MYMODULE_views_data_alter(array &$data) {
  $data['node_field_data']['field_amount']['field']['float'] = TRUE;
}
super_romeo’s picture

Dear @nikita_tt,
for me works this:

$data['node__field_test']['field_test']['field']['float'] = TRUE;

Anyway, I think it is just a workaround.

Lendude’s picture

Title: Decimal handling in Views » Decimal separator and decimals settings ignored when aggregating decimal fields
Version: 8.6.x-dev » 8.9.x-dev

Updated the title a bit to make it clearer what this is about

playful’s picture

I tried controlling the decimal precision by using twig in a custom text field, such as the following:

{{ field_value|round }}
{{ field_value|round(1, 'common') }}

But none of these worked. They all returned a 0.

On a side note, no other twig math functions worked either, even for basic arithmetic. Is that a separate bug or are twig functions in the Views UI limited to conditional logic?

It really seems there should be options in the UI to control decimal precision when using aggregation. Any updated ideas on how to achieve this?

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

chankongching’s picture

#16 is good enough

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sarguna raj M’s picture

Hi,

Tried #16 but its not working, the value returned as 0. Since the views uses the order total value (Sum and Average). Any update on this?

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ramil g’s picture

Hey all, I created a patch for this.

Can everyone test it and let me know if you find any side-effects. With this patch, you don't need to put anything in your template file or implement any hooks. Aggregated fields will have the same settings that non-aggregated decimal fields have, just like how it works in Drupal 7 right now.

It also fixes the bug of the 'Group column' and 'Group column (additional)' sections missing from a field's Aggregation settings

ramil g’s picture

Status: Active » Needs review
ramil g’s picture

I overlooked something with patch #33. The 'thousand marker' settings was being duplicated. Fixed with this patch.

ranjith_kumar_k_u’s picture

ramil g’s picture

Thanks for fixing the whitespace issue @ranjith_kumar_k_u but there's actually a problem with the patch, which I only found out after I saved my view. I'll see if I can find a different way to fix this.

Edit:
Actually I think the patch works. I've been testing on two different sites, one was a vanilla install of drupal 9(using 9.5.x-dev) and the other one, a real site, with some custom modules. I found out that it was one of the custom modules that had caused the issue. It seems to be working fine with the vanilla drupal 9 site. I would love to get others' feedback though.

ramil g’s picture

Ignore my previous patches. This problem seems to have originated from this commit: lets use annotated handlers in even more places

See attached image. In get_aggregation_info(), somebody pasted the field handler where it should've fallen through to the original handler. It seems like a copy/paste error.

This patch corrects that error, removing the field handler from sum, avg, min, max, and stddev_pop

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +PHP 8.1

#2735997-38: Decimal separator and decimals settings ignored when aggregating decimal fields While testing with @ramil g we thought through the possible regressions and this may need a CR for those that have implemented #2735997-23: Decimal separator and decimals settings ignored when aggregating decimal fields, but the twig template workaround should continue to work.

This also fixes a bug that we found that when you choose aggregation, the "Aggregation settings" lose the EntityField additional fields and yield a PHP 8.1 Warning (though notice in PHP 7) because it expects the "Group column" and " Group columns (additional)" there but they hide after any other option than "Group results together" is chosen.

quietone’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update, +Needs screenshots

Lovely to see older bugs getting fixed!

This issue summary is out of date and does not document the proposed change here. Adding tag. I've added some steps to reproduce because I had to poke around and figure out how to create the problem.

In #39 @joelpittet explains the testing that was done but this is a UI issue so before and after screen shots should be added to the issue summary.

Since no tests are breaking with the change should one be added to prevent future problems?

joelpittet’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update +Needs tests

@quietone, thanks for the review, I've added the proposed change and yes probably could use a regression test.

asad_ahmed’s picture

Assigned: Unassigned » asad_ahmed
asad_ahmed’s picture

Assigned: asad_ahmed » Unassigned
asad_ahmed’s picture

FileSize
55.7 KB
55.45 KB

I can apply the patch successfully and add before and after screenshots. Please review the screenshots.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joelpittet’s picture

@asad_ahmed Can you let us know why there is a comma separator in your screenshot?

Akram Khan’s picture

Created patch for updated version 10.1.x

Akram Khan’s picture

Resolve #47

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ressa’s picture

Thanks for sharing a pragmatic Twig solution in #16 @Ben Greenberg. Drupal 10 is now on Twig 3, so the URL is https://twig.symfony.com/doc/3.x/filters/number_format.html, but the syntax is the same.