Fix ACLs to use the correct name of the civicrm_group table #26615
Conversation
|
(Standard links)
|
|
ping @MegaphoneJon this maybe relevant to your things |
| @@ -1477,8 +1477,7 @@ public function testCreateWithLesserPermissions() { | |||
| * this from civicrm_generated.mysql | |||
| */ | |||
| private function setUpACLByCheating() { | |||
| CRM_Core_DAO::executeQuery("INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active) VALUES ('Edit All Contacts', 0, 'civicrm_acl_role', 1, 'Edit', 'civicrm_saved_search', 0, NULL, NULL, 1)"); | |||
| CRM_Core_DAO::executeQuery("INSERT INTO civicrm_acl (name, deny, entity_table, entity_id, operation, object_table, object_id, acl_table, acl_id, is_active) VALUES ('Core ACL',0,'civicrm_acl_role',0,'All','access CiviMail subscribe/unsubscribe pages',NULL,NULL,NULL,1)"); | |||
There was a problem hiding this comment.
@demeritcowboy why did you include this? :) object_id surely should be an id not a string right? :)
There was a problem hiding this comment.
It's null, and I took it straight out of civicrm_generated, but the removed "Core ACL" line is necessary for the test above to work if I remember right. The test may now ALWAYS pass which wasn't the intent.
There was a problem hiding this comment.
I'm open to doing the test a different way - it might be useful to have the test installs mirror more closely a real install for ALL tests.
There was a problem hiding this comment.
@demeritcowboy the line I removed in the test is also being removed from real databases by this PR
There was a problem hiding this comment.
Ok I see. I'll do a followup after since the test isn't completely useless but it's not testing what it was testing and the ACL part of it now is a bit misleading.
seamuslee001
added
merge on pass
merge ready
For unknown historical reasons, group-based ACLs incorrectly used 'civicrm_saved_search' as the value for 'object_table' when referencing a group id. This updates all ACL records, tests, and adds legacy handling for hook subscribers with a noisy deprecation notice.
colemanw
force-pushed
the
aclTableNameFix
branch
from
5cb4597 to
f704d62
Compare
Overview
Fixes a weird quirk in the ACL code.
Technical Details
For unknown historical reasons, group-based ACLs incorrectly used
'civicrm_saved_search'as the value for'object_table'when referencing a Group (the correct name of that table is'civicrm_group').This updates all ACL records, tests, and strings.
Hook Update: Unfortunately that incorrect value was being passed to
hook_civicrm_aclGroup. This PR deals with that by calling the hook twice with both values. If your hook listener responds to the old'civicrm_saved_search'value, it will emit a deprecation notice.