jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/394239 )

Change subject: Allow Major gift contacts with conflicting addresses to merge.
......................................................................


Allow Major gift contacts with conflicting addresses to merge.

By removing the conflict we allow all the prioritisation rules developed for 
other
contacts to apply

Unit tests updated to reflect the fact that merge now takes place and the most 
recent
donor's details are now used. Comfortingly all the 'resolvable' conflict tests 
are
unchanged, still selecting the preferable addresses. The 'unresolvable' conflict
tests have been updated to reflect use of most recent donor detail. (a couple
of test names had to be updated to be accurate about that distinction

Bug: T181088
Change-Id: Ie78e3a517b796967976936b4fc93d2352a7aa14e
---
M sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
M sites/all/modules/wmf_civicrm/wmf_civicrm.module
2 files changed, 88 insertions(+), 77 deletions(-)

Approvals:
  jenkins-bot: Verified
  Ejegg: Looks good to me, approved



diff --git a/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php 
b/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
index 44fde89..e5eb6cb 100644
--- a/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
+++ b/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
@@ -221,10 +221,10 @@
     if ($dataSet['is_major_gifts']) {
       $this->contributionCreate(array('contact_id' => $this->contactID2, 
'receive_date' => '2012-01-01', 'total_amount' => 300));
     }
-    foreach ($dataSet['contact_1'] as $address) {
+    foreach ($dataSet['earliest_donor'] as $address) {
       $this->callAPISuccess($dataSet['entity'], 'create', 
array_merge(array('contact_id' => $this->contactID), $address));
     }
-    foreach ($dataSet['contact_2'] as $address) {
+    foreach ($dataSet['most_recent_donor'] as $address) {
       $this->callAPISuccess($dataSet['entity'], 'create', 
array_merge(array('contact_id' => $this->contactID2), $address));
     }
 
@@ -265,10 +265,10 @@
     if ($dataSet['is_major_gifts']) {
       $this->contributionCreate(array('contact_id' => $this->contactID, 
'receive_date' => '2012-01-01', 'total_amount' => 300));
     }
-    foreach ($dataSet['contact_1'] as $address) {
+    foreach ($dataSet['earliest_donor'] as $address) {
       $this->callAPISuccess($dataSet['entity'], 'create', 
array_merge(array('contact_id' => $this->contactID2), $address));
     }
-    foreach ($dataSet['contact_2'] as $address) {
+    foreach ($dataSet['most_recent_donor'] as $address) {
       $this->callAPISuccess($dataSet['entity'], 'create', 
array_merge(array('contact_id' => $this->contactID), $address));
     }
 
@@ -704,9 +704,12 @@
   /**
    * Test that we don't see a city named after a country as the same as a 
country.
    *
+   * UPDATE - this is now merged, keeping most recent donor - ie. 1 since
+   * that is the only one with a donation.
+   *
    * Bug T176699
    */
-  public function 
testBatchMergeResolvableConflictCityLooksCountryishWithCounty() {
+  public function 
testBatchMergeUnResolvableConflictCityLooksCountryishWithCounty() {
     $this->callAPISuccess('Address', 'create', array(
       'country_id' => 'US',
       'contact_id' => $this->contactID2,
@@ -722,17 +725,26 @@
     $this->contributionCreate(array('contact_id' => $this->contactID, 
'receive_date' => '2010-01-01', 'total_amount' => 500));
 
     $result = $this->callAPISuccess('Job', 'process_batch_merge', array('mode' 
=> 'safe'));
-    $this->assertEquals(1, count($result['values']['skipped']));
-    $this->assertEquals(0, count($result['values']['merged']));
+    $this->assertEquals(0, count($result['values']['skipped']));
+    $this->assertEquals(1, count($result['values']['merged']));
+
+    $address = $this->callAPISuccessGetSingle('Address', array('contact_id' => 
$this->contactID));
+    $this->assertEquals('First on the left after you cross the border', 
$address['street_address']);
+    $this->assertEquals('MX', 
CRM_Core_PseudoConstant::countryIsoCode($address['country_id']));
+    $this->assertTrue(!isset($address['city']));
+
   }
 
   /**
    * Test that we don't see a city named after a country as the same as a 
country
    * when it has no country.
    *
+   * UPDATE - this is now merged, keeping most recent donor - ie. 1 since
+   * that is the only one with a donation.
+   *
    * Bug T176699
    */
-  public function 
testBatchMergeResolvableConflictCityLooksCountryishNoCountry() {
+  public function 
testBatchMergeUnResolvableConflictCityLooksCountryishNoCountry() {
     $this->callAPISuccess('Address', 'create', array(
       'contact_id' => $this->contactID2,
       'city' => 'Mexico',
@@ -747,8 +759,13 @@
     $this->contributionCreate(array('contact_id' => $this->contactID, 
'receive_date' => '2010-01-01', 'total_amount' => 500));
 
     $result = $this->callAPISuccess('Job', 'process_batch_merge', array('mode' 
=> 'safe'));
-    $this->assertEquals(1, count($result['values']['skipped']));
-    $this->assertEquals(0, count($result['values']['merged']));
+    $this->assertEquals(0, count($result['values']['skipped']));
+    $this->assertEquals(1, count($result['values']['merged']));
+
+    $address = $this->callAPISuccessGetSingle('Address', array('contact_id' => 
$this->contactID));
+    $this->assertEquals('First on the left after you cross the border', 
$address['street_address']);
+    $this->assertEquals('MX', 
CRM_Core_PseudoConstant::countryIsoCode($address['country_id']));
+    $this->assertTrue(!isset($address['city']));
   }
 
   /**
@@ -778,9 +795,12 @@
    * Test that we don't see a city named after a country as the same as a 
country
    * when it has no country.
    *
+   * UPDATE - this is now merged, keeping most recent donor - ie. 1 since
+   * that is the only one with a donation.
+   *
    * Bug T176699
    */
-  public function testBatchMergeResolvableConflictRealConflict() {
+  public function testBatchMergeUnResolvableConflictRealConflict() {
     $this->callAPISuccess('Address', 'create', array(
       'contact_id' => $this->contactID2,
       'city' => 'Poland',
@@ -796,8 +816,12 @@
     $this->contributionCreate(array('contact_id' => $this->contactID, 
'receive_date' => '2010-01-01', 'total_amount' => 500));
 
     $result = $this->callAPISuccess('Job', 'process_batch_merge', array('mode' 
=> 'safe'));
-    $this->assertEquals(1, count($result['values']['skipped']));
-    $this->assertEquals(0, count($result['values']['merged']));
+    $this->assertEquals(0, count($result['values']['skipped']));
+    $this->assertEquals(1, count($result['values']['merged']));
+
+    $address = $this->callAPISuccessGetSingle('Address', array('contact_id' => 
$this->contactID));
+    $this->assertEquals('PL', 
CRM_Core_PseudoConstant::countryIsoCode($address['country_id']));
+    $this->assertTrue(!isset($address['city']));
   }
 
   /**
@@ -1041,7 +1065,7 @@
           'is_major_gifts' => 1,
           'description' => 'Same behaviour with & without the hook, matching 
primary AND other address maintained',
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1051,7 +1075,7 @@
               'is_primary' => 0,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1076,13 +1100,13 @@
           'is_major_gifts' => 1,
           'description' => 'Same behaviour with & without the hook, matching 
primary AND other address maintained',
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1111,7 +1135,7 @@
           'is_major_gifts' => 1,
           'description' => 'Same behaviour with & without the hook, address is 
maintained',
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1121,7 +1145,7 @@
               'is_primary' => 0,
             ), $locationParams2),
           ),
-          'contact_2' => array(),
+          'most_recent_donor' => array(),
           'expected_hook' => array_merge($additionalExpected, array(
             array_merge(array(
               'location_type_id' => 'Home',
@@ -1143,8 +1167,8 @@
           'is_major_gifts' => 1,
           'description' => 'Same behaviour with & without the hook, address is 
maintained',
           'entity' => $entity,
-          'contact_1' => array(),
-          'contact_2' => array(
+          'earliest_donor' => array(),
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1173,13 +1197,13 @@
           'is_major_gifts' => 1,
           'description' => 'Primaries are different with different location. 
Keep both addresses. Set primary to be that of more recent donor',
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
@@ -1203,13 +1227,13 @@
           'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1233,7 +1257,7 @@
           'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1243,7 +1267,7 @@
               'is_primary' => 0,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
@@ -1268,13 +1292,13 @@
           'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1306,13 +1330,13 @@
           'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
@@ -1337,13 +1361,13 @@
           'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1359,17 +1383,17 @@
       ),
       'conflicting_home_address_major_gifts' => array(
         'conflicting_home_address_major_gifts' => array(
-          'merged' => 0,
-          'skipped' => 1,
+          'merged' => 1,
+          'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1379,7 +1403,7 @@
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
-            ), $locationParams1),
+            ), $locationParams2),
           )),
         ),
       ),
@@ -1393,13 +1417,13 @@
           'skipped' => 0,
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1415,11 +1439,11 @@
       ),
       'conflicting_home_address_one_more_major_gifts' => array(
         'conflicting_home_address_one_more_major_gifts' => array(
-          'merged' => 0,
-          'skipped' => 1,
+          'merged' => 1,
+          'skipped' => 0,
           'is_major_gifts' => 1,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
@@ -1429,7 +1453,7 @@
               'is_primary' => 1,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
@@ -1443,7 +1467,7 @@
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
-            ), $locationParams1),
+            ), $locationParams3),
             array_merge(array(
               'location_type_id' => 'Mailing',
               'is_primary' => 1,
@@ -1461,7 +1485,7 @@
           'skipped' => 0,
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
@@ -1471,7 +1495,7 @@
               'is_primary' => 1,
             ), $locationParams2),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
@@ -1503,13 +1527,13 @@
           'skipped' => 0,
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 1,
@@ -1537,13 +1561,13 @@
           'skipped' => 0,
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
@@ -1570,13 +1594,13 @@
           (no high priority improvement)',
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Main',
               'is_primary' => 1,
@@ -1605,13 +1629,13 @@
           'skipped' => 0,
           'is_major_gifts' => 0,
           'entity' => $entity,
-          'contact_1' => array(
+          'earliest_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
             ), $locationParams1),
           ),
-          'contact_2' => array(
+          'most_recent_donor' => array(
             array_merge(array(
               'location_type_id' => 'Home',
               'is_primary' => 0,
diff --git a/sites/all/modules/wmf_civicrm/wmf_civicrm.module 
b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
index 0f1961e..6e07755 100644
--- a/sites/all/modules/wmf_civicrm/wmf_civicrm.module
+++ b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
@@ -2487,29 +2487,16 @@
       $refs['is_major_gift'] = 
_wmf_civicrm_is_merged_contact_major_donor($mainId, $otherId);
     }
     $fieldParts = explode('_', $moveField);
-    if (!$refs['is_major_gift']) {
-      if ($fieldParts[2] == 'email') {
-        $mainIsOnHold = 
$refs['migration_info']['main_details']['location_blocks']['email'][$fieldParts[3]]['on_hold'];
-        $otherIsOnHold = 
$refs['migration_info']['other_details']['location_blocks']['email'][$fieldParts[3]]['on_hold'];
-        if ($mainIsOnHold + $otherIsOnHold === 1) {
-          // One is one hold & the other isn't - conflict.
-          return;
-        }
-      }
-      unset($refs['fields_in_conflict'][$moveField]);
-    }
-    else {
-      if (_wmf_civicrm_merge_is_address_conflict_resolvable(
-        $refs['migration_info']['rows'][$moveField]['other'],
-        $refs['migration_info']['rows'][$moveField]['main'],
-        
$refs['migration_info']['other_details']['location_blocks'][$fieldParts[2]][$fieldParts[3]],
-        
$refs['migration_info']['main_details']['location_blocks'][$fieldParts[2]][$fieldParts[3]]
-    )) {
-          // Actual resolution is in alterLocationMergeData hook.
-          unset($refs['fields_in_conflict'][$moveField]);
-      }
-    }
 
+    if ($fieldParts[2] == 'email') {
+      $mainIsOnHold = 
$refs['migration_info']['main_details']['location_blocks']['email'][$fieldParts[3]]['on_hold'];
+      $otherIsOnHold = 
$refs['migration_info']['other_details']['location_blocks']['email'][$fieldParts[3]]['on_hold'];
+      if ($mainIsOnHold + $otherIsOnHold === 1) {
+        // One is one hold & the other isn't - conflict.
+        return;
+      }
+    }
+    unset($refs['fields_in_conflict'][$moveField]);
   }
 }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/394239
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie78e3a517b796967976936b4fc93d2352a7aa14e
Gerrit-PatchSet: 3
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Eileen <[email protected]>
Gerrit-Reviewer: Eileen <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Mepps <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to