Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] simplify references to civicrm_acl #16671

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 2, 2020

Overview

Minor code cleanup

Before

A variable used in mysql to stand in for civicrm_acl

After

civicrm_acl is used - no change in functionality - ACLS still work (& we have test cover on that)

Technical Details

We can see looking at ACL.xml that civicm_acl can't be localised so we gain nothing
by using a varible.

Also note that we want to switch to CRM_Core_Execute::query in these functions so
that table translation is handled by that function - this is old code done the old way.

However, the thing I'm trying to unravel is if & where the values for the fields
entity_table & object_table are localised. We have been switching AWAY from
saving localised table names & TBH I think we maybe don't & it's a lot of cruft
for some ideas that never eventuated but I'm still digging

Comments

We can see looking at ACL.xml that civicm_acl can't be localised so we gain nothing
by using a varible.

Also note that we want to switch to CRM_Core_Execute::query in these functions so
that table translation is handled by that function - this is old code done the old way.

However, the thing I'm trying to unravel is if & where the values for the fields
entity_table & object_table are localised. We have been switching AWAY from
saving localised table names & TBH I think we maybe don't & it's a lot of cruft
for some ideas that never eventuated but I'm still digging
@civibot
Copy link

civibot bot commented Mar 2, 2020

(Standard links)

@civibot civibot bot added the master label Mar 2, 2020
@@ -160,11 +160,10 @@ protected static function getACLs(int $contact_id) {

$rule = new CRM_ACL_BAO_ACL();

$acl = self::getTableName();
$contact = CRM_Contact_BAO_Contact::getTableName();

$query = " SELECT acl.*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 I've been digging on this - I can't find any evidence that anywhere in core ever saves to civicrm_acl with the value entity_table = 'contact'

It seems the OG bridge has some hard coding to acl_role

@seamuslee001
Copy link
Contributor

Changes i think are ok merging

@seamuslee001 seamuslee001 merged commit 501d220 into civicrm:master Mar 16, 2020
@seamuslee001 seamuslee001 deleted the acl branch March 16, 2020 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants