Totals per page and column precision always set to 0 in views_aggregator_update_10200

Created on 6 November 2024, about 2 months ago

Problem/Motivation

Originally reported in https://www.drupal.org/project/views_aggregator/issues/3482129#comment-1... πŸ› Crashes with Drupal\views_aggregator\Plugin\views\style\Table::setAggregatedGroupValues(): Argument #3 ($group_aggregation_results) must be of type int, string given Active by @joachim :

the update function correctly changes the type of the group_aggregation_results property:

-            group_aggregation_results: '0'
+            group_aggregation_results: 0

However, the update function also made these changes which I'm confused about:

-            totals_per_page: 1
-            precision: 2
+            totals_per_page: 0
+            precision: 0

Proposed resolution

It looks like there is a missing . in views_aggregator_update_10200 between the key and property for when the old values of precision and totals_per_page are fetched which is then not getting the actual old values and always setting them as 0.

E.g. the old value comes from $key . 'column_aggregation.precision' and is set to $key . '.column_aggregation.precision'

πŸ› Bug report
Status

Active

Version

2.1

Component

Code

Created by

πŸ‡³πŸ‡ΏNew Zealand ericgsmith

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ericgsmith
  • Pipeline finished with Success
    about 2 months ago
    Total: 162s
    #331393
  • First commit to issue fork.
  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    Thanks for noticing that and figuring out the problem @ericgsmith!

    Unfortunately my original tests only checked the data types of the configuration variables before and after the update hook, but did not check the actual values. If the tests checked the values, then that problem would have been caught. So what I did is, in addition to your fix, I updated the tests so that they would check both the data types and values. Notably, as you can see in the test-only output (i.e. the new test run WITHOUT your fix) the test now catches the problem. The test also now proves your fix works.

    Fixing hook_update_10200() like this will prevent other people from having this same problem, so I'm going to commit this change and push out a new release.

    Unfortunately, anyone who has already run the old version of hook_update_10200() will have already had the values of totals_per_page and precision both set to 0 - that can't be fixed automatically, and will require anyone experiencing that problem to edit and re-save their Views in order to set those variables how they want.

  • Pipeline finished with Skipped
    about 2 months ago
    #333531
  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024