CRM-18681 respect api permissions flag on batch merge job
authoreileen <emcnaughton@wikimedia.org>
Thu, 26 May 2016 01:43:38 +0000 (13:43 +1200)
committereileen <emcnaughton@wikimedia.org>
Sat, 25 Jun 2016 01:07:02 +0000 (13:07 +1200)
CRM/Core/BAO/PrevNextCache.php
CRM/Core/OptionValue.php
CRM/Dedupe/Finder.php
CRM/Dedupe/Merger.php
api/v3/Job.php
tests/phpunit/api/v3/JobTest.php

index 8cacb0d0fee9ab0cfb32a7f086f27d4e3ebe1047..91bb82780486be792c27e5e788fb7fa5b29d1b94 100644 (file)
@@ -344,11 +344,16 @@ WHERE (pn.cacheKey $op %1 OR pn.cacheKey $op %2)
    * @param array $criteria
    *   Additional criteria to filter by.
    *
+   * @param bool $checkPermissions
+   *   Respect logged in user's permissions.
+   *
    * @return bool
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  public static function refillCache($rgid = NULL, $gid = NULL, $cacheKeyString = NULL, $criteria = array()) {
+  public static function refillCache($rgid, $gid, $cacheKeyString, $criteria, $checkPermissions) {
     if (!$cacheKeyString && $rgid) {
-      $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria);
+      $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions);
     }
 
     if (!$cacheKeyString) {
@@ -373,7 +378,7 @@ WHERE (pn.cacheKey $op %1 OR pn.cacheKey $op %2)
         $contacts = civicrm_api3('Contact', 'get', array_merge(array('options' => array('limit' => 0), 'return' => 'id'), $criteria['contact']));
         $contactIDs = array_keys($contacts['values']);
       }
-      $foundDupes = CRM_Dedupe_Finder::dupes($rgid, $contactIDs);
+      $foundDupes = CRM_Dedupe_Finder::dupes($rgid, $contactIDs, $checkPermissions);
     }
 
     if (!empty($foundDupes)) {
index 21998224648946ca03b9b3369424f3185d540f88..df4a386029b7a3f7305440de7dbf328e24cbb6b7 100644 (file)
@@ -281,8 +281,7 @@ class CRM_Core_OptionValue {
    * @param string $mode
    * @param string $contactType
    *
-   * @return bool
-   *   true if object exists
+   * @return array
    */
   public static function getFields($mode = '', $contactType = 'Individual') {
     $key = "$mode $contactType";
index cb0bc2668b93aafcbeb6ac49c006d7aafbee8d5b..38fdf81e7aff8ffb10254f2a887c0bd5b4598731 100644 (file)
@@ -48,10 +48,14 @@ class CRM_Dedupe_Finder {
    * @param array $cids
    *   Contact ids to limit the search to.
    *
+   * @param bool $checkPermissions
+   *   Respect logged in user permissions.
+   *
    * @return array
-   *   array of (cid1, cid2, weight) dupe triples
+   *   Array of (cid1, cid2, weight) dupe triples
+   * @throws \Exception
    */
-  public static function dupes($rgid, $cids = array()) {
+  public static function dupes($rgid, $cids = array(), $checkPermissions = TRUE) {
     $rgBao = new CRM_Dedupe_BAO_RuleGroup();
     $rgBao->id = $rgid;
     $rgBao->contactIds = $cids;
@@ -61,7 +65,7 @@ class CRM_Dedupe_Finder {
 
     $rgBao->fillTable();
     $dao = new CRM_Core_DAO();
-    $dao->query($rgBao->thresholdQuery());
+    $dao->query($rgBao->thresholdQuery($checkPermissions));
     $dupes = array();
     while ($dao->fetch()) {
       $dupes[] = array($dao->id1, $dao->id2, $dao->weight);
index 0bc09a6f4d19dcfedaf2e5f2c3dce3228f82a871..2a1923495d0797b7c39a6d632b436a97025d34ca 100644 (file)
@@ -593,21 +593,24 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    * @param array $criteria
    *   Criteria to use in the filter.
    *
+   * @param bool $checkPermissions
+   *   Respect logged in user permissions.
+   *
    * @return array|bool
    */
-  public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $autoFlip = TRUE, $batchLimit = 1, $isSelected = 2, $criteria = array()) {
+  public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $autoFlip = TRUE, $batchLimit = 1, $isSelected = 2, $criteria = array(), $checkPermissions = TRUE) {
     $redirectForPerformance = ($batchLimit > 1) ? TRUE : FALSE;
     $reloadCacheIfEmpty = (!$redirectForPerformance && $isSelected == 2);
-    $dupePairs = self::getDuplicatePairs($rgid, $gid, $reloadCacheIfEmpty, $batchLimit, $isSelected, '', ($mode == 'aggressive'), $criteria);
+    $dupePairs = self::getDuplicatePairs($rgid, $gid, $reloadCacheIfEmpty, $batchLimit, $isSelected, '', ($mode == 'aggressive'), $criteria, $checkPermissions);
 
     $cacheParams = array(
-      'cache_key_string' => self::getMergeCacheKeyString($rgid, $gid, $criteria),
+      'cache_key_string' => self::getMergeCacheKeyString($rgid, $gid, $criteria, $checkPermissions),
       // @todo stop passing these parameters in & instead calculate them in the merge function based
       // on the 'real' params like $isRespectExclusions $batchLimit and $isSelected.
       'join' => self::getJoinOnDedupeTable(),
       'where' => self::getWhereString($batchLimit, $isSelected),
     );
-    return CRM_Dedupe_Merger::merge($dupePairs, $cacheParams, $mode, $autoFlip, $redirectForPerformance);
+    return CRM_Dedupe_Merger::merge($dupePairs, $cacheParams, $mode, $autoFlip, $redirectForPerformance, $checkPermissions);
   }
 
   /**
@@ -716,15 +719,18 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    *                             A 'safe' value skips the merge if there are any un-resolved conflicts.
    *                             Does a force merge otherwise (aggressive mode).
    * @param bool $autoFlip to let api decide which contact to retain and which to delete.
-   *   Wether to let api decide which contact to retain and which to delete.
-   *
+   *   Whether to let api decide which contact to retain and which to delete.
    *
    * @param bool $redirectForPerformance
+   *   Redirect to a url for batch processing.
+   *
+   * @param bool $checkPermissions
+   *   Respect logged in user permissions.
    *
    * @return array|bool
    */
   public static function merge($dupePairs = array(), $cacheParams = array(), $mode = 'safe',
-                               $autoFlip = TRUE, $redirectForPerformance = FALSE
+                               $autoFlip = TRUE, $redirectForPerformance = FALSE, $checkPermissions = TRUE
   ) {
     $cacheKeyString = CRM_Utils_Array::value('cache_key_string', $cacheParams);
     $resultStats = array('merged' => array(), 'skipped' => array());
@@ -769,7 +775,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
         // go ahead with merge if there is no conflict
         $conflicts = array();
         if (!CRM_Dedupe_Merger::skipMerge($mainId, $otherId, $migrationInfo, $mode, $conflicts)) {
-          CRM_Dedupe_Merger::moveAllBelongings($mainId, $otherId, $migrationInfo);
+          CRM_Dedupe_Merger::moveAllBelongings($mainId, $otherId, $migrationInfo, $checkPermissions);
           $resultStats['merged'][] = array('main_id' => $mainId, 'other_id' => $otherId);
           $deletedContacts[] = $otherId;
         }
@@ -1044,6 +1050,9 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    *     to move all fields from the 'other' contact to the 'main' contact, as
    *     though the form had been submitted with those options.
    *
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Exception
    */
   public static function getRowsElementsAndInfo($mainId, $otherId) {
     $qfZeroBug = 'e8cddb72-a257-11dc-b9cc-0016d3330ee9';
@@ -1515,9 +1524,12 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    *
    * @param $migrationInfo
    *
+   * @param bool $checkPermissions
+   *   Respect logged in user permissions.
+   *
    * @return bool
    */
-  public static function moveAllBelongings($mainId, $otherId, $migrationInfo) {
+  public static function moveAllBelongings($mainId, $otherId, $migrationInfo, $checkPermissions = TRUE) {
     if (empty($migrationInfo)) {
       return FALSE;
     }
@@ -1838,22 +1850,16 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
       CRM_Core_BAO_CustomValueTable::setValues($viewOnlyCustomFields);
     }
 
-    if (CRM_Core_Permission::check('merge duplicate contacts') &&
-      CRM_Core_Permission::check('delete contacts')
+    if (!$checkPermissions || (CRM_Core_Permission::check('merge duplicate contacts') &&
+      CRM_Core_Permission::check('delete contacts'))
     ) {
       // if ext id is submitted then set it null for contact to be deleted
       if (!empty($submitted['external_identifier'])) {
         $query = "UPDATE civicrm_contact SET external_identifier = null WHERE id = {$otherId}";
         CRM_Core_DAO::executeQuery($query);
       }
-
       civicrm_api3('contact', 'delete', array('id' => $otherId));
-      CRM_Core_BAO_PrevNextCache::deleteItem($otherId);
     }
-    // FIXME: else part
-    // else {
-    //  CRM_Core_Session::setStatus( ts('Do not have sufficient permission to delete duplicate contact.') );
-    // }
 
     // CRM-15681 merge sub_types
     if ($other_sub_types = CRM_Utils_array::value('contact_sub_type', $migrationInfo['other_details'])) {
@@ -2019,24 +2025,27 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    * @param bool $reloadCacheIfEmpty
    * @param int $batchLimit
    * @param bool $isSelected
-   * @param array $orderByClause
+   * @param array|string $orderByClause
    * @param bool $includeConflicts
    * @param array $criteria
    *   Additional criteria to narrow down the merge group.
    *
+   * @param bool $checkPermissions
+   *   Respect logged in user permissions.
+   *
    * @return array
-   *   Array of matches meeting the criteria.
+   *    Array of matches meeting the criteria.
    */
-  public static function getDuplicatePairs($rule_group_id, $group_id, $reloadCacheIfEmpty, $batchLimit, $isSelected, $orderByClause = '', $includeConflicts = TRUE, $criteria = array()) {
+  public static function getDuplicatePairs($rule_group_id, $group_id, $reloadCacheIfEmpty, $batchLimit, $isSelected, $orderByClause = '', $includeConflicts = TRUE, $criteria = array(), $checkPermissions = TRUE) {
     $where = self::getWhereString($batchLimit, $isSelected);
-    $cacheKeyString = self::getMergeCacheKeyString($rule_group_id, $group_id, $criteria);
+    $cacheKeyString = self::getMergeCacheKeyString($rule_group_id, $group_id, $criteria, $checkPermissions);
     $join = self::getJoinOnDedupeTable();
     $dupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where, 0, 0, array(), $orderByClause, $includeConflicts);
     if (empty($dupePairs) && $reloadCacheIfEmpty) {
       // If we haven't found any dupes, probably cache is empty.
       // Try filling cache and give another try. We don't need to specify include conflicts here are there will not be any
       // until we have done some processing.
-      CRM_Core_BAO_PrevNextCache::refillCache($rule_group_id, $group_id, $cacheKeyString, $criteria);
+      CRM_Core_BAO_PrevNextCache::refillCache($rule_group_id, $group_id, $cacheKeyString, $criteria, $checkPermissions);
       $dupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where, 0, 0, array(), $orderByClause, $includeConflicts);
       return $dupePairs;
     }
@@ -2052,14 +2061,28 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
    *   Additional criteria to narrow down the merge group.
    *   Currently we are only supporting the key 'contact' within it.
    *
+   * @param bool $checkPermissions
+   *   Respect the users permissions.
+   *
    * @return string
    */
-  public static function getMergeCacheKeyString($rule_group_id, $group_id, $criteria = array()) {
+  public static function getMergeCacheKeyString($rule_group_id, $group_id, $criteria = array(), $checkPermissions = TRUE) {
     $contactType = CRM_Dedupe_BAO_RuleGroup::getContactTypeForRuleGroup($rule_group_id);
     $cacheKeyString = "merge {$contactType}";
     $cacheKeyString .= $rule_group_id ? "_{$rule_group_id}" : '_0';
     $cacheKeyString .= $group_id ? "_{$group_id}" : '_0';
     $cacheKeyString .= !empty($criteria) ? serialize($criteria) : '_0';
+    if ($checkPermissions) {
+      $contactID = CRM_Core_Session::getLoggedInContactID();
+      if (!$contactID) {
+        // Distinguish between no permission check & no logged in user.
+        $contactID = 'null';
+      }
+      $cacheKeyString .= '_' . $contactID;
+    }
+    else {
+      $cacheKeyString .= '_0';
+    }
     return $cacheKeyString;
   }
 
index da4eb0476300efc5e09595fffe864a5191788116..5ae0363f8d5b836a496512fb429e97724b102a9c 100644 (file)
@@ -490,7 +490,7 @@ function civicrm_api3_job_process_batch_merge($params) {
   $mode = CRM_Utils_Array::value('mode', $params, 'safe');
   $autoFlip = CRM_Utils_Array::value('auto_flip', $params, TRUE);
 
-  $result = CRM_Dedupe_Merger::batchMerge($rule_group_id, $gid, $mode, $autoFlip, 1, 2, CRM_Utils_Array::value('criteria', $params, array()));
+  $result = CRM_Dedupe_Merger::batchMerge($rule_group_id, $gid, $mode, $autoFlip, 1, 2, CRM_Utils_Array::value('criteria', $params, array()), CRM_Utils_Array::value('check_permissions', $params));
 
   return civicrm_api3_create_success($result, $params);
 }
index dfe3648dcfc245c9f7644785c2021c14f5f93978..19c46026fb15d2b11d74d7a8aea5e18112744fdb 100644 (file)
@@ -438,6 +438,42 @@ class api_v3_JobTest extends CiviUnitTestCase {
 
   }
 
+  /**
+   * Test the batch merge function actually works!
+   *
+   * @dataProvider getMergeSets
+   *
+   * @param $dataSet
+   */
+  public function testBatchMergeWorksCheckPermissionsTrue($dataSet) {
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = array('access CiviCRM', 'administer CiviCRM');
+    foreach ($dataSet['contacts'] as $params) {
+      $this->callAPISuccess('Contact', 'create', $params);
+    }
+
+    $result = $this->callAPISuccess('Job', 'process_batch_merge', array('check_permissions' => 1, 'mode' => $dataSet['mode']));
+    $this->assertEquals(0, count($result['values']['merged']), 'User does not have permission to any contacts, so no merging');
+    $this->assertEquals(0, count($result['values']['skipped']), 'User does not have permission to any contacts, so no skip visibility');
+  }
+
+  /**
+   * Test the batch merge function actually works!
+   *
+   * @dataProvider getMergeSets
+   *
+   * @param $dataSet
+   */
+  public function testBatchMergeWorksCheckPermissionsFalse($dataSet) {
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = array('access CiviCRM', 'edit my contact');
+    foreach ($dataSet['contacts'] as $params) {
+      $this->callAPISuccess('Contact', 'create', $params);
+    }
+
+    $result = $this->callAPISuccess('Job', 'process_batch_merge', array('check_permissions' => 0, 'mode' => $dataSet['mode']));
+    $this->assertEquals($dataSet['skipped'], count($result['values']['skipped']), 'Failed to skip the right number:' . $dataSet['skipped']);
+    $this->assertEquals($dataSet['merged'], count($result['values']['merged']));
+  }
+
   /**
    * Get data for batch merge.
    */