- 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 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!
- Merge request !27Issue #3484017 by tr, joachim: Table::getCellRaw(): Return value must be of... β (Open) created by tr
- πΊπΈ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 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.
- π¦πΉ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 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.