[REF] Use Standard function cacheClause to re-use contact acl cache t… #17707
Conversation
|
(Standard links)
|
| @@ -176,7 +176,7 @@ public function testPrevNextCache() { | |||
| 'expected_query' => [ | |||
| 0 => 'default', | |||
| 1 => 'default', | |||
| 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND ( 1 )", | |||
| 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND contact_a.is_deleted = 0", | |||
There was a problem hiding this comment.
This is_deleted suggests the contact in the test does not have permission to view deleted contacts?
There was a problem hiding this comment.
Looks like the cacheClause function was not respecting the view deleted contacts permission
|
Jenkins re test this please |
| $wheres = []; | ||
| foreach ($contactAlias as $alias) { | ||
| if (!CRM_Core_Permission::check('access deleted contacts')) { | ||
| if (is_array($contactAlias)) { |
There was a problem hiding this comment.
I'm not a big fan of code that does if(is_array($foo)) {.. do complex stuff multiple times} else {.. do complex stuff once} if the "complex stuff" is the same in both cases, then it's better to just cast $foo to an array.
| if (is_array($contactAlias)) { | ||
| $wheres = []; | ||
| foreach ($contactAlias as $alias) { | ||
| // CRM-6181 | ||
| $wheres[] = "$alias.is_deleted = 0"; | ||
| } | ||
| return [NULL, '(' . implode(' AND ', $wheres) . ')']; | ||
| } |
There was a problem hiding this comment.
| if (is_array($contactAlias)) { | |
| $wheres = []; | |
| foreach ($contactAlias as $alias) { | |
| // CRM-6181 | |
| $wheres[] = "$alias.is_deleted = 0"; | |
| } | |
| return [NULL, '(' . implode(' AND ', $wheres) . ')']; | |
| } | |
| $wheres = []; | |
| foreach ((array) $contactAlias as $alias) { | |
| // CRM-6181 | |
| $wheres[] = "$alias.is_deleted = 0"; | |
| } | |
| return [NULL, '(' . implode(' AND ', $wheres) . ')']; |
|
I added has-test as the previous fails make it clear this is tested! @seamuslee001 it's stale |
seamuslee001
force-pushed
the
ref_permission_clause_search
branch
from
87423cf to
d3181ef
Compare
eileenmcnaughton
changed the title
seamuslee001
force-pushed
the
ref_permission_clause_search
branch
from
d3181ef to
e9ad71c
Compare
|
Rebased now @eileenmcnaughton |
|
@seamuslee001 This makes sense to me. I'll add the PR to a couple of sites and see if I notice any oddities. |
colemanw
added
the
merge ready
Tidy up where clause generation as per Coleman
…o prove that when someone doesn't have view all contacts but does have access deleted contacts and we are searching in the trash an appropriate where clause is generated
seamuslee001
force-pushed
the
ref_permission_clause_search
branch
from
e9ad71c to
21b8bcb
Compare
|
Jenkins re test this please |
|
@kcristiano @colemanw this has been merge-ready for a while. I would normally merge something after merge-ready but this seems to be pending more testing rather than giving others a chance to input |
|
@eileenmcnaughton I agree I have it applied on our 5.28 RC sites my concern is we won't hot any issues until I can get it on production. I do think it's good to go in master as that will get it further testing before release |
|
Thanks @kcristiano |
…able
Overview
This refactors the CRM_Contact_BAO_Query functions to utilise the cacheClause function which joins on the acl_contact_cache table rather than re-running the ACL where clause again
Before
Doesn't use acl cache contact table
After
Uses ACL cache contact table
ping @kcristiano @eileenmcnaughton @colemanw