FIX Use OR conjuctive in filterAny aggregate queries

[emteknetnz]

Issue #10744

[GuySartorelli]

I haven't done a full review of this, as it seems likely that a lot will need to change to account for not making the API breaking changes.

[GuySartorelli]

Two things that don't fit in code comments:

Which branch to target

This is a bug fix so arguably should target 4.13 or 5.0 - I'm guessing you're targetting 5 because of the API change?
I'm on the fence so I'll raise the question but not ask for one or the other.

How did it used to work?

I can't find anything in this PR that points at how the original HAVING clause was built.... it might just be that I missed it, but I'm a little worried that this solution is just side-stepping whatever the original mechanism was, which might mean we're left with dead code.

[emteknetnz]

Which branch to target

One of the ACs is:

Fix is targeted at 5.1. We won't fix this in CMS 4.

How did it used to work? .. I can't find anything in this PR that points at how the original HAVING clause was built

I'm not sure either, though it's also not really relevant? Somewhere else in the ORM a HAVING clause was chosen to be used, which may seem a little weird though it's fine since it still does what it needs to. Changing the HAVING clause to a WHERE clause is out of scope for this bugfix PR. Also probably no point in doing so.

[GuySartorelli]

Good point on the AC, I forgot about that - I'll need to make more of an effort to remember to check that more frequently while reviewing lest I review out of context 😅

I'm not sure either, though it's also not really relevant? Somewhere else in the ORM a HAVING clause was chosen to be used, which may seem a little weird though it's fine since it still does what it needs to. Changing the HAVING clause to a WHERE clause is out of scope for this bugfix PR. Also probably no point in doing so.

I'm not saying we should change from HAVING to WHERE - what I'm saying is that the mechanism that was previously deciding this code should result in a HAVING clause is no longer being used in this scenario - so it may be dead code. We should be making a reasonable effort to avoid having dead code.

[emteknetnz]

what I'm saying is that the mechanism that was previously deciding this code should result in a HAVING clause is no longer being used in this scenario ?

I'm confused - the HAVING clause is still being used? The point of the code update is to simply to change the conjunctive in the HAVING clause from AND to OR

[GuySartorelli]

As per discussion IRL - the old mechanism is still in use for other purposes so no change needed for that.