When using "group results", price fields render correctly if the group column if Number and additionally the Currency Code field is selected.

However, choosing SUM or AVG causes the price field to render as a normal integer field.

CommentFileSizeAuthor
#23 views.view_.taxes_report.yml42.1 KBjohaziel
#20 2975099-sum-avg-price-fix.patch4.32 KBdlevchik
#19 2975099-19.patch5.95 KBjohaziel
#18 2975099-18.patch5.93 KBjohaziel
#13 2975099-13.patch5.15 KBmglaman
#11 2975099-11.patch4.27 KBmglaman
#9 2975099-9.patch6.72 KBmglaman
#9 Revenue_report__Order_report____Drush_Site-Install.png109.31 KBmglaman
#7 2975099-7.patch4.68 KBmglaman
Revenue_report__Order_report____Drush_Site-Install.png139.66 KBmglaman
Revenue_report__Order_report____Drush_Site-Install.png137.55 KBmglaman

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

In commerce_price.views.inc

      $data[$table_name][$field_name . '_number']['field'] = [
        'id' => 'numeric',
        'field_name' => $table_data[$field_name]['field']['field_name'],
        'entity_type' => $table_data[$field_name]['field']['entity_type'],
        'label' => t('number from @field_name', ['@field_name' => $field_name]),
      ];

The number field is marked to use the numeric field. When aggregating I am guessing that Views will default to the "first" field property as being the "real field", which is *_number. So on aggregation it sums to amount and it is rendering it using the numeric field handler.

Looks like we'll need to provide a custom handler.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

It might be more than that.

In \Drupal\views\Plugin\views\query\Sql::getAggregationInfo the aggregate options define the field handler.

      'sum' => [
        'title' => $this->t('Sum'),
        'method' => 'aggregationMethodSimple',
        'handler' => [
          'argument' => 'groupby_numeric',
          'field' => 'numeric',
          'filter' => 'groupby_numeric',
          'sort' => 'groupby_numeric',
        ],
      ],
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Okay, so by picking any form of aggregate function it overrides the price field to be a Numeric handler and not even EntityField.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

There's no way to override the handler...easily.

In \Drupal\views\Plugin\views\display\DisplayPluginBase::getHandlers

          $aggregate = $this->view->query->getAggregationInfo();
          if (!empty($aggregate[$info['group_type']]['handler'][$type])) {
            $override = $aggregate[$info['group_type']]['handler'][$type];
          }

The handler is overridden from that aggregation info. We would have to create a new display plugin which attempted to override this, using its own handler to support SUM on the field while GROUP on the currency code.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Related issues: +#2975149: When using aggregation, additional fields should not be included in the grouping function

This looks to be blocked, partially, by core #2975149: When using aggregation, additional fields should not be included in the grouping function

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs work
StatusFileSize
new 2975099-7.patch4.68 KB

Here is a patch which implements the core patch fix to commerce_order_report views. It unsets aggregate functions to additional fields added in. Now we need to fix the formatting, which should be possible since we are grouping by the currency code, now.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Okay, it looks like we can't format the output in the pre_render Views hook. The \Drupal\views\Plugin\views\field\NumericField::render method calls $value = round($value, $precision); which causes the formatter price to return 0.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new Revenue_report__Order_report____Drush_Site-Install.png109.31 KB
new 2975099-9.patch6.72 KB

This makes it work! Swaps out the field handler later on.

Status: Needs review » Needs work

The last submitted patch, 9: 2975099-9.patch, failed testing. View results

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2975099-11.patch4.27 KB

Whoops, that had other cruft in it.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work 
+++ b/commerce_reports.module
@@ -10,3 +15,62 @@ function commerce_reports_query_commerce_reports_alter(AlterableInterface $query
+  // Swap out the NumericField handler with our special Price one.
+  $view->field['amount'] = \Drupal\commerce_reports\Plugin\views\field\PriceNumericField::createFromNumericField(
+    $view->field['amount']
+  );

Forgot I didn't make this dynamic and just hardcoded it for testing.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2975099-13.patch5.15 KB

Using an array_intersect we can check if there are matching additional fields and determine the price fields.

      $intersect_test = array_intersect([
        $field_plugin->field . '_number',
        $field_plugin->field . '_currency_code'
      ], $field_plugin->additional_fields);

      if (!empty($intersect_test)) {
        // Swap out the NumericField handler with our special Price one.
        $view->field[$key] = PriceNumericField::createFromNumericField($field_plugin);
      }

  • mglaman committed 7147c91 on 8.x-1.x 
    Issue #2975099 by mglaman: Price fields do not work properly when using...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

loze’s picture

loze commented

I know this is very old but it looks like this is not working any longer.

I can t SUM on my price fields. Ive traced it back to commerce_reports_views_pre_build where this check here fails 

  if ($field_plugin instanceof NumericField) {

The amount fields arent passing here so the custom formatter is never being used.

johaziel’s picture

johaziel
Primary language French
Location Bretagne
commented
StatusFileSize
new 2975099-18.patch5.93 KB

Hi,
I think issue need to be reopen,
$field_plugin is now intanceof EntityField (don't know why...)

I created createFromEntityField() based on createFromNumericField() and add test 

if ($field_plugin instanceof EntityField) { ...
...PriceNumericField::createFromEntityField($field_plugin)..
}

The code is in the patch and now SUM work again and I can have my view of Tax reports working !

To be continue...

johaziel’s picture

johaziel
Primary language French
Location Bretagne
commented
StatusFileSize
new 2975099-19.patch5.95 KB

Sorry, some mistakes in the patch..

dlevchik’s picture

dlevchik commented
StatusFileSize
new 2975099-sum-avg-price-fix.patch4.32 KB

johaziel, thanks for the patch. Unfortunately, it was not applying for 1.0.0-rc4 because you forgot ddl test output and commerce_reports.info.yml composer output.

I've updated your patch and now it applies for 1.0.0-rc4. I can confirm it works with AVG and SUM for my report views.

loze’s picture

loze commented

This doesn't quite solve the problem either.

The patch applies and I can add a field and aggregate on it, but when I go to edit the field, I get the following error:

"
An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /admin/structure/views/ajax/handler-group/sales_report/block_1/field/amount_2
StatusText: 500 Service unavailable (with message)
ResponseText: The website encountered an unexpected error. Try again later.TypeError: array_filter(): Argument #1 ($array) must be of type array, null given in array_filter() (line 727 of core/modules/views/src/Plugin/views/field/EntityField.php). Drupal\views\Plugin\views\field\EntityField->submitGroupByForm(Array, Object) (Line: 106)
Drupal\views_ui\Form\Ajax\ConfigHandlerGroup->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 268)
Drupal\views_ui\ViewUI->standardSubmit(Array, Object)
call_user_func_array(Array, Array) (Line: 129)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 67)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597)
Drupal\Core\Form\FormBuilder->processForm('views_ui_config_item_group_form', Array, Object) (Line: 326)
Drupal\Core\Form\FormBuilder->buildForm('Drupal\views_ui\Form\Ajax\ConfigHandlerGroup', Object) (Line: 215)
Drupal\views_ui\Form\Ajax\ViewsFormBase->Drupal\views_ui\Form\Ajax\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 217)
Drupal\views_ui\Form\Ajax\ViewsFormBase->ajaxFormWrapper('Drupal\views_ui\Form\Ajax\ConfigHandlerGroup', Object) (Line: 127)
Drupal\views_ui\Form\Ajax\ViewsFormBase->getForm(Object, 'block_1', 'ajax') (Line: 38)
Drupal\views_ui\Form\Ajax\ConfigHandlerGroup->getForm(Object, 'block_1', 'ajax', 'field', 'amount_2')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
"
loze’s picture

loze commented

the core patch here helps get this working for me #3344910: Ajax error on views with aggregation

johaziel’s picture

johaziel
Primary language French
Location Bretagne
commented
StatusFileSize
new views.view_.taxes_report.yml42.1 KB

@dlevchik Thanks for your correction, I was "speed" when I did it

@loze may be your view is to complicated, did you test with the base view_sales_report ?

My first goal was to create a taxes report view and also respond to #2976637: Create a taxes report

I put here my view, it works for me. Can you test with it.
You have to add in commerce_reports.links.menu.yml

commerce_reports.taxes_report:
  title: 'Taxes'
  route_name: 'view.taxes_report.monthly'
  parent: 'commerce_reports.overview'
tomtech’s picture

tomtech commented

Hi all,

Appreciate the feedback, but this issue was fixed 7 years ago.

If there is a regression or new problem, a new issue should be created, and optionally reference this one, if needed.

Based on the comments and patches, I suspect everyone encountering this issue now is using Drupal Core 10.4.6 or 11.1.6?

There was an issue in those releases that caused a problem with aggregate fields. (See: #2735997: Decimal separator and decimals settings ignored when aggregating decimal fields )

This issue was reverted in the following releases, 10.4.7 and 11.1.7. Upgrading to those version will likely resolve this regression.

If this doesn't resolve it, please open a new issue referencing this one.