Table::getCellRaw(): Return value must be of type string, false returned

Created on 28 October 2024, about 2 months ago

Issue summary copied from a post by @poker.ca, found in #3482129-6: Crashes with Drupal\views_aggregator\Plugin\views\style\Table::setAggregatedGroupValues(): Argument #3 ($group_aggregation_results) must be of type int, string given β†’

After updating to 2.1.0, I ran update.php and cleared the caches, but still get this error when loading any page that uses this module. Other pages (tables) are working OK...
TypeError: Drupal\views_aggregator\Plugin\views\style\Table::getCellRaw(): Return value must be of type string, false returned in Drupal\views_aggregator\Plugin\views\style\Table->getCellRaw() (line 754 of /var/www/drupal/web/modules/contrib/views_aggregator/src/Plugin/views/style/Table.php).

πŸ› Bug report
Status

Active

Version

2.1

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

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

Merge Requests

Comments & Activities

  • Issue created by @tr
  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    @poker.ca: Can you provide some details about your view? Are you aggregating Webform or Commerce data, or something else? What is the data type of the field? It would help a lot if you could export your view and post it here so we can see exactly what is being done.

    I added stricter typing to this module in 2.1.0, which can reveal existing problems, especially in parts of the code that have no test coverage. There is some special-case code for both Webform and Commerce that is completely untested, and I have mentioned in other issues how this is not maintainable or supportable in the long run. We really need test cases for both of these "exceptions", and I'm not the best person to write them because I don't use either of those modules.

    It would be helpful if I had a *simple* views export that I could use to reproduce this error - that is, maybe one of the standard Drupal views that has been modified to use Views Aggregator and reproduce the problem, so that it doesn't involve a lot of elaborate site setup with complicated content types and many large modules.

    The fix should be quite simple, but I have to understand where the wrongly typed data is coming from, that's why I need to see how you've configured your view.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    Seeing this problem too.

        elseif (isset($field->value)) {
          $value = $field->value;
        }
    

    This gets an empty array.

        if ($compressed && is_array($value)) {
          $value = reset($value);
          if (is_array($value)) {
            $value = reset($value);
          }
        }
    

    reset([]) is FALSE, hence the bad return value.

    My view is a list of timeslots which have a duration field, and I am aggregating the duration.

    If a timeslot is current, it has no end time and no duration, hence the empty field value.

  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    Thank you, that's very helpful.

  • πŸ‡¬πŸ‡§United Kingdom joachim

    It's a duration field from https://www.drupal.org/project/duration_field β†’

    I think the comment here is wrong -- lots of field types have a 'value' property:

        // Get the commerce value.
        elseif (isset($field->value)) {
          $value = $field->value;
        }
    
  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    Yes, trying to figure out the original intent is a bit of a problem in this section of the code since it isn't explained in an issue or in a code comment, and the comments that are there seems to be inconsistent.

    If special casing is going to be used for Commerce and Webform (which is something I'm really not happy with) then it should be explicit: Test for Commerce/Webform, then call a special method with the special code. Else treat it as a normal field. But that's not what was done here - it seems Commerce and Webform have exceptions in many different places, so it's not clear what code is "special" and what code applies to normal fields.

    Regardless, the code has always declared "@return string", and all I did was to reinforce that by adding :string as the return type hint. So before, it was returning an array for you, and it is still returning an array for you, but now it's complaining because the array is not a string.

    Opening a MR with a patch. Please tell me if this fixes the problem for you!

  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    Tests pass, but that means nothing because we don't have any test cases which exercise this section of code.

  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia
  • πŸ‡¬πŸ‡§United Kingdom joachim

    I can reproduce this with a numeric field.

    1. Create a node type
    2. Add a numeric field to it
    3. Create three nodes, one of which has an empty value for that field
    4. Create a view with fields title, numeric field.
    5. Set aggregation to:
    - group and compress title field
    - SUM on numeric field

  • πŸ‡¬πŸ‡§United Kingdom joachim

    The MR fixes the crash with the test case from my last comment.

    I'm not sure where a test would go -- struggling to figure out the structure of the existing tests.

  • πŸ‡ΊπŸ‡ΈUnited States codesmith San Francisco

    This just happened to me and the MR fixes the issue.

  • πŸ‡ͺπŸ‡ΈSpain ferelx

    In my case the MR also fixes de issue.

  • Also got this problem, MR 27 fixed it.

    Thanks,

  • πŸ‡¦πŸ‡ΉAustria stemiwe

    I still get this, even with the latest commit from 2.1.x-dev

    The website encountered an unexpected error. Try again later.

    TypeError: Drupal\views_aggregator\Plugin\views\style\Table::getCellRaw(): Return value must be of type string, bool returned in Drupal\views_aggregator\Plugin\views\style\Table->getCellRaw() (line 758 of modules/contrib/views_aggregator/src/Plugin/views/style/Table.php).

  • stemiwe,

    I think you need to patch using MR 27 here. The lastest dev still doesn't have this MR commit.

  • πŸ‡¬πŸ‡§United Kingdom Jon Pollard

    I am getting a similar problem, this is the error:

    TypeError: Drupal\views_aggregator\Plugin\views\style\Table::getCellRaw(): Return value must be of type string, bool returned in Drupal\views_aggregator\Plugin\views\style\Table->getCellRaw() (line 758 of /public_html/modules/contrib/views_aggregator/src/Plugin/views/style/Table.php).

    Using the forked code doesn't help - if I revert to the old (2.03) version of the Table.php file I don't get the error and everything seems to work!

    I note that this error is for type 'bool', which might be the difference.

  • πŸ‡ΊπŸ‡ΈUnited States tawellman

    We also had this problem, MR 27 fixed it.

    Thank you!!

  • πŸ‡ΊπŸ‡ΈUnited States scampbell1 New York

    I encountered the same error and just tried adding a type-casting if statement, which fixed it for me. However, the solution I found may not work for everyone because my use case is because the node ID column in my view is being shown as a string and not an int.

  • πŸ‡ΊπŸ‡ΈUnited States scampbell1 New York
  • πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

    I don't see what #19 or #20 has to do with this issue. $group_aggregation_results is always an integer, unless you updated your version of the module without running the update hooks. Regardless, it's a different problem and you should open a different issue if you can reproduce the error on a clean site.

Production build 0.71.5 2024