Rename field to be priority and add in upgrade guards as per Coleman

civicrm · Jun 22, 2023 · 66ccb70 · 66ccb70
1 parent efddd39
commit 66ccb70
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 48 deletions.
diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php
@@ -529,37 +529,48 @@ private static function loadDenyIDs(int $contactID, string $tableName, int $type
     $acls = CRM_ACL_BAO_Cache::build($contactID);
     $aclKeys = array_keys($acls);
     $aclKeys = implode(',', $aclKeys);
+    $select = "a.operation, a.object_id";
+    $hasPriorty = FALSE;
+    if (array_key_exists('priority', CRM_ACL_BAO_ACL::getSupportedFields())) {
+      $select .= ",a.priority";
+      $hasPriority = TRUE;
+    }
     $query = "
-SELECT   a.operation, a.object_id, a.weight
+SELECT   {$select}
   FROM   civicrm_acl_cache c, civicrm_acl a
  WHERE   c.acl_id       =  a.id
    AND   a.is_active    =  1
    AND   a.object_table = %1
    AND   a.id        IN ( $aclKeys )
    AND   a.deny         = 1
-GROUP BY a.operation,a.object_id, a.weight
-ORDER BY a.object_id
 ";
     $params = [1 => [$tableName, 'String']];
     $dao = CRM_Core_DAO::executeQuery($query, $params);
     while ($dao->fetch()) {
       if ($dao->object_id) {
         if (self::matchType($type, $dao->operation)) {
-          $permittedRules = CRM_Core_DAO::singleValueQuery("SELECT count(a.id)
-            FROM civicrm_acl a
-            INNER JOIN civicrm_acl_cache c ON c.acl_id = a.id
-            WHERE a.id IN ( $aclKeys )
-            AND a.is_active = 1
-            AND a.object_table = %1
-            AND a.deny = 0
-            AND a.weight > %2
-            AND a.object_id = %3", [
-              1 => [$tableName, 'String'],
-              2 => [(int) $dao->weight, 'Integer'],
-              3 => [$dao->object_id, 'Integer'],
-            ]);
-          if (empty($permittedRules)) {
-            $ids[] = $dao->object_id;
+          if ($hasPriority) {
+            $permittedRules = CRM_Core_DAO::singleValueQuery("SELECT count(a.id)
+              FROM civicrm_acl a
+              INNER JOIN civicrm_acl_cache c ON c.acl_id = a.id
+              WHERE a.id IN ( $aclKeys )
+              AND a.is_active = 1
+              AND a.object_table = %1
+              AND a.deny = 0
+              AND a.priority > %2
+              AND a.object_id = %3", [
+                1 => [$tableName, 'String'],
+                2 => [(int) $dao->priority, 'Integer'],
+                3 => [$dao->object_id, 'Integer'],
+              ]);
+            if (empty($permittedRules) && !in_array($dao->object_id, $ids, TRUE)) {
+              $ids[] = $dao->object_id;
+            }
+          }
+          else {
+            if (!in_array($dao->object_id, $ids, TRUE)) {
+              $ids[] = $dao->object_id;
+            }
           }
         }
       }

diff --git a/CRM/ACL/DAO/ACL.php b/CRM/ACL/DAO/ACL.php
@@ -6,7 +6,7 @@
  *
  * Generated from xml/schema/CRM/ACL/ACL.xml
  * DO NOT EDIT.  Generated by CRM_Core_CodeGen
- * (GenCodeChecksum:02dc2c6a0ab80f3d396aed305eaad680)
+ * (GenCodeChecksum:662599614a2cd32151c5b492534415a6)
  */
 
 /**
@@ -134,7 +134,7 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO {
    *   (SQL type: int)
    *   Note that values will be retrieved from the database as a string.
    */
-  public $weight;
+  public $priority;
 
   /**
    * Class constructor.
@@ -410,18 +410,18 @@ public static function &fields() {
           ],
           'add' => '1.6',
         ],
-        'weight' => [
-          'name' => 'weight',
+        'priority' => [
+          'name' => 'priority',
           'type' => CRM_Utils_Type::T_INT,
-          'title' => ts('Weight'),
+          'title' => ts('Priority'),
           'required' => TRUE,
           'usage' => [
             'import' => FALSE,
             'export' => FALSE,
             'duplicate_matching' => FALSE,
             'token' => FALSE,
           ],
-          'where' => 'civicrm_acl.weight',
+          'where' => 'civicrm_acl.priority',
           'default' => '0',
           'table_name' => 'civicrm_acl',
           'entity' => 'ACL',
@@ -510,13 +510,13 @@ public static function indices($localize = TRUE) {
         'localizable' => FALSE,
         'sig' => 'civicrm_acl::0::acl_id',
       ],
-      'index_weight' => [
-        'name' => 'index_weight',
+      'index_priority' => [
+        'name' => 'index_priority',
         'field' => [
-          0 => 'weight',
+          0 => 'priority',
         ],
         'localizable' => FALSE,
-        'sig' => 'civicrm_acl::0::weight',
+        'sig' => 'civicrm_acl::0::priority',
       ],
     ];
     return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices;

diff --git a/CRM/ACL/Form/ACL.php b/CRM/ACL/Form/ACL.php
@@ -29,7 +29,7 @@ public function setDefaultValues() {
 
     if ($this->_action & CRM_Core_Action::ADD) {
       $defaults['object_type'] = 1;
-      $defaults['weight'] = 0;
+      $defaults['priority'] = 0;
     }
 
     $showHide = new CRM_Core_ShowHideBlocks();
@@ -165,7 +165,7 @@ public function buildQuickForm() {
       0 => ts('Allow'),
       1 => ts('Deny'),
     ]);
-    $this->add('text', 'weight', ts('Weight'));
+    $this->add('text', 'priority', ts('Priority'));
 
     $this->addFormRule(['CRM_ACL_Form_ACL', 'formRule']);
   }

diff --git a/CRM/ACL/Page/ACL.php b/CRM/ACL/Page/ACL.php
@@ -137,7 +137,7 @@ public function browse() {
       $acl[$dao->id]['object_id'] = $dao->object_id;
       $acl[$dao->id]['is_active'] = $dao->is_active;
       $acl[$dao->id]['deny'] = $dao->deny;
-      $acl[$dao->id]['weight'] = $dao->weight;
+      $acl[$dao->id]['priority'] = $dao->priority;
 
       if ($acl[$dao->id]['entity_id']) {
         $acl[$dao->id]['entity'] = $roles[$acl[$dao->id]['entity_id']] ?? NULL;

diff --git a/CRM/Upgrade/Incremental/php/FiveSixtyFour.php b/CRM/Upgrade/Incremental/php/FiveSixtyFour.php
@@ -28,7 +28,7 @@ class CRM_Upgrade_Incremental_php_FiveSixtyFour extends CRM_Upgrade_Incremental_
    *   The version number matching this function name
    */
   public function upgrade_5_64_alpha1($rev): void {
-    $this->addTask('Add weight column onto ACL table', 'addColumn', 'weight', 'int NOT NULL DEFAULT 0');
+    $this->addTask('Add weight column onto ACL table', 'addColumn', 'civicrm_acl', 'priority', 'int NOT NULL DEFAULT 0');
     $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
     $this->addTask('Update post_URL/cancel_URL in logging tables', 'updateLogging');
   }

diff --git a/templates/CRM/ACL/Form/ACL.tpl b/templates/CRM/ACL/Form/ACL.tpl
@@ -49,10 +49,10 @@
        <td class="label">{$form.deny.label}</td>
        <td>{$form.deny.html}</td>
      </tr>
-     <tr class="crm-acl-form-block-weight">
-       <td class="label">{$form.weight.label}</td>
-       <td>{$form.weight.label}<br />
-         <span class="description">{ts}Higher weighted acl rules will override lower weighted rules{/ts}</span>
+     <tr class="crm-acl-form-block-priority">
+       <td class="label">{$form.priority.label}</td>
+       <td>{$form.priority.label}<br />
+         <span class="description">{ts}Higher priority ACL rules will override lower priority rules{/ts}</span>
        </td>
      </tr>
   </table>

diff --git a/templates/CRM/ACL/Page/ACL.tpl b/templates/CRM/ACL/Page/ACL.tpl
@@ -45,7 +45,7 @@
                 <td class="crm-acl-name crm-editable" data-field="name">{$row.name}</td>
                 <td class="crm-acl-is_active" id="row_{$aclID}_status">{if $row.is_active eq 1} {ts}Yes{/ts} {else} {ts}No{/ts} {/if}</td>
                 <td class="crm-acl-deny" id="row_{$aclID}_deny">{if $row.deny}{ts}Deny{/ts}{else}{ts}Allow{/ts}{/if}</td>
-                <td class="crm-acl-weight" id="row_{$aclID}_deny">{$row.weight}</td>
+                <td class="crm-acl-priority" id="row_{$aclID}_priority">{$row.priority}</td>
                 <td>{$row.action|replace:'xx':$aclID}</td>
               </tr>
             {/foreach}

diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -1319,24 +1319,23 @@ public function testNegativeCustomGroupACL(): void {
   /**
    * @throws \CRM_Core_Exception
    */
-  public function testWeightedCustomGroupACL(): void {
+  public function testPriorityCustomGroupACL(): void {
     // Create 2 multi-record custom entities and 2 regular custom fields
     $customGroups = [];
     foreach ([1, 2] as $i) {
       $customGroups[$i] = CustomGroup::create(FALSE)
-        ->addValue('title', "weight_extra_group_$i")
+        ->addValue('title', "priority_extra_group_$i")
         ->addValue('extends', 'Contact')
         ->addValue('is_multiple', FALSE)
         ->addChain('field', CustomField::create()
-          ->addValue('label', "weight_extra_field_$i")
+          ->addValue('label', "priority_extra_field_$i")
           ->addValue('custom_group_id', '$id')
           ->addValue('html_type', 'Text')
           ->addValue('data_type', 'String')
         )
         ->execute()->single()['id'];
       $this->callAPISuccess('Acl', 'create', [
         'name' => 'Deny everyone to access custom group ' . $customGroups[$i],
-        'deny' => 0,
         'entity_table' => 'civicrm_acl_role',
         'entity_id' => 0,
         'operation' => 'Edit',
@@ -1348,7 +1347,7 @@ public function testWeightedCustomGroupACL(): void {
 
     $this->callAPISuccess('OptionValue', 'create', [
       'option_group_id' => 'acl_role',
-      'label' => 'Test weighted ACL Role',
+      'label' => 'Test Priority ACL Role',
       'value' => 5,
       'is_active' => 1,
     ]);
@@ -1360,8 +1359,8 @@ public function testWeightedCustomGroupACL(): void {
       'is_active' => 1,
     ])->execute();
     $this->callAPISuccess('Acl', 'create', [
-      'name' => 'Test Postive Weight ACL',
-      'weight' => 1,
+      'name' => 'Test Postive Priority ACL',
+      'priority' => 1,
       'entity_table' => 'civicrm_acl_role',
       'entity_id' => 5,
       'operation' => 'Edit',
@@ -1381,7 +1380,7 @@ public function testWeightedCustomGroupACL(): void {
     Civi::cache('metadata')->clear();
     Civi::$statics['CRM_ACL_BAO_ACL'] = [];
     $getFields = Contact::getFields()
-      ->addWhere('name', 'LIKE', 'weight_extra_group_%.weight_extra_field_%')
+      ->addWhere('name', 'LIKE', 'priority_extra_group_%.priority_extra_field_%')
       ->execute();
     $this->assertCount(1, $getFields);
 

diff --git a/xml/schema/ACL/ACL.xml b/xml/schema/ACL/ACL.xml
@@ -127,7 +127,7 @@
     </html>
   </field>
   <field>
-    <name>weight</name>
+    <name>priority</name>
     <type>int</type>
     <default>0</default>
     <required>true</required>
@@ -137,7 +137,7 @@
     </html>
   </field>
   <index>
-    <name>index_weight</name>
-    <fieldName>weight</fieldName>
+    <name>index_priority</name>
+    <fieldName>priority</fieldName>
   </index>
 </table>