Reviewed & tested by the community
Project:
Views Aggregator Plus
Version:
2.1.x-dev
Component:
Code
Priority:
Minor
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
18 Nov 2022 at 11:06 UTC
Updated:
28 Oct 2024 at 04:52 UTC
Jump to comment: Most recent
Comments
Comment #3
tr commentedI triggered a re-test now that #3322628: Core Drupal broke this module's PHP 8 tests has been fixed.
Comment #4
mortona2k commentedI encountered this issue, looks like I have an aggregated table with a sum on a column that has no values.
This patch fixes the warnings. It's just checking isset($result_row->_relationship_entities[$relationship]) before using the value.
Comment #5
tr commentedComment #13
tr commentedOK I screwed up the rebasing to a new branch, but it's fixed now. The new MR has the same changes as @plopesc posted in #1, but it's now on the current 2.1.x branch.
Adding an isset() to make an error go away is something I really don't like to do. Because for example in this case $source never gets a value, which may cause problems further down the line, and even if it doesn't cause a problem having to check a value with isset() tells me that there's something wrong in the code before we get to that point. Shouldn't we know at this point whether the relationship exists? Why are we trying to do something with it?
I am also trying to get new test cases written, so that we can catch things like this before they happen, or at least ensure that once this is fixed it won't show up again in the future because someone inadvertently broke it. A simple view export that causes this error to happen would be great if you can provide it, that way I can put it in a test.
Comment #14
tr commentedAlso, see #3241057: Trouble calculating column sum on view with active views group_by (aggregation) which makes a very similar change for a different reason.