Fix malformed query when user with no access to any financial acls accesses civimember search

[eileenmcnaughton]

Overview

1. enable financial acls
2. give civimember access to webuser
3. log in as advisor/advisor & access membership search

Before

After

Technical Details

The contact cannot view any memberships but returning 0 winds up dropped - '= 0' works...

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

@eileenmcnaughton so if I'm understanding this correctly the malformed sql is AND AND - it should be AND 0 AND but there's probably some php !empty() check which short-circuits?
But I'm confused about how AND =0 AND would actually be ok sql.
A lot of other funcitons like this return the string "(0)" instead of just "0".

[eileenmcnaughton]

@colemanw it winds up as 'membership_type_id = 0' - I think maybe it got filtered out of something - I'm not sure exactly now cos my brain didn't hold it - but on r-run it was pretty clear that it failed before & worked after!

[colemanw]

@eileenmcnaughton ok but I still can't make sense of line 228: return 1;. Wouldn't that also create bad sql?

[eileenmcnaughton]

@colemanw hmm - that return 1 was added here #22864 - I'm just trying to test but hit ... yet another bug in the acls

[eileenmcnaughton]

Ok - here was the bug I just hit... #23235

[eileenmcnaughton]

@colemanw I've fixed it to return '=0' when civiMember is disabled too - I couldn't actually hit it this time - but it should be the same for when CiviMember is disabled as when there are no available membership types - MembershipType api still works 'correctly' with civimember disabled - although I think it's kinda tough for code writers that it throws an exception (even with checkPermissions=FALSE I think) in v4 api

[colemanw]

Ok. I see that this function is supposed to return the latter 2/3 of a WHERE clause (the operator and the value) and now it consistently does so.

[colemanw]

@eileenmcnaughton the thinking behind it is that we are moving components to be more extension-like (and even moving some components to be extensions) so APIv4 helps with that transition. In v3 disabled extensions would throw an exception if you tried their api, but disabled components would not. In v4 it's consistent: calling the Volunteer api with CiviVolunteer disabled throws the same exception as calling the Member api with CiviMember disabled.

[eileenmcnaughton]

hmm ok - ....