Switch to non deprecated buildPermissionClause() for contact summary report

[mattwire]

Overview

Swap deprecated buildACLClause() for non-deprecated buildPermissionClause(). On a site using ACLs restricted users were seeing multiple duplicates when refreshing the report - this was caused by the INNER JOIN on the civicrm_acl_contact_cache that was being added and is not required.

Before

Duplicates for users using ACLs when refreshing the report.

After

No duplicates.

Technical Details

Described above.

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).

[demeritcowboy]

It looks like this gets called already from buildQuery() right below? If you just remove the line does that fix the problem?

[mattwire]

You are right. It works just removing the buildACLClause line

[eileenmcnaughton]

This looks correct to me - I'll leave to you to merge when you are happy with it @demeritcowboy

[eileenmcnaughton]

I took a quick look & there are several reports still with this - the Contact/Detail one looks almost identical to this

[demeritcowboy]

• General standards
  • [r-explain] PASS
    • I can sort of reproduce the problem, although I only get one duplicate record - for the contact of the logged-in user - but maybe the exact results depend on some other factors and what ACLs are set up.
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • Duplicate gone. ACL restriction still seems to work.
• Developer standards
  • [r-tech] Soft PASS
    • My only hesitation is I don't use ACLs much so am not completely sure what to look out for, but (a) it was calling buildPermissionClause() before via buildQuery() so it's not doing anything new that it didn't before, and (b) it's limited to this report.
  • [r-code] PASS
  • [r-maint] ?
  • [r-test] PASS

[demeritcowboy]

And you're right the constituent detail seems to have the same duplicate problem.

There is also a separate problem with the constituent summary if you use force=1 but ... sigh.