Eileen has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311908

Change subject: Fix to prevent duplicate Home addresses resulting in no primary 
on merge.
......................................................................

Fix to prevent duplicate Home addresses resulting in no primary on merge.

Bug: T145873
Change-Id: Icee9aab5179e6e42cbedd2fa615072053834962b
---
M sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
M sites/all/modules/wmf_civicrm/wmf_civicrm.module
2 files changed, 125 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm 
refs/changes/08/311908/1

diff --git a/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php 
b/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
index 349c8d9..1f71ccd 100644
--- a/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
+++ b/sites/all/modules/wmf_civicrm/tests/phpunit/MergeTest.php
@@ -426,7 +426,7 @@
    */
   public function getMergeLocations($locationParams1, $locationParams2, 
$locationParams3, $entity, $additionalExpected = array()) {
     $data = array(
-      1 => array(
+      'matching_primary' => array(
         'matching_primary' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -461,7 +461,7 @@
           )),
         ),
       ),
-      2 => array(
+      'matching_primary_reverse' => array(
         'matching_primary_reverse' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -496,7 +496,7 @@
           )),
         ),
       ),
-      3 => array(
+      'only_one_has_address' => array(
         'only_one_has_address' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -528,7 +528,7 @@
           )),
         ),
       ),
-      4 => array(
+      'only_one_has_address_reverse' => array(
         'only_one_has_address_reverse' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -558,7 +558,7 @@
           )),
         ),
       ),
-      5 => array(
+      'different_primaries_with_different_location_type' => array(
         'different_primaries_with_different_location_type' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -589,7 +589,7 @@
           )),
         ),
       ),
-      6 => array(
+      'different_primaries_with_different_location_type_reverse' => array(
         'different_primaries_with_different_location_type_reverse' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -619,7 +619,7 @@
           )),
         ),
       ),
-      7 => array(
+      'different_primaries_location_match_only_one_address' => array(
         'different_primaries_location_match_only_one_address' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -654,7 +654,7 @@
           )),
         ),
       ),
-      8 => array(
+      'different_primaries_location_match_only_one_address_reverse' => array(
         'different_primaries_location_match_only_one_address_reverse' => array(
           'merged' => 1,
           'skipped' => 0,
@@ -688,7 +688,7 @@
           )),
         ),
       ),
-      9 => array(
+      'same_primaries_different_location' => array(
         'same_primaries_different_location' => array(
           'fix_required_for_reverse' => 1,
           'comment' => 'core is not identifying this as an address conflict in 
reverse order'
@@ -719,7 +719,7 @@
           )),
         ),
       ),
-      10 => array(
+      'same_primaries_different_location_reverse' => array(
         'same_primaries_different_location_reverse' => array(
           'fix_required_for_reverse' => 1,
           'comment' => 'core is not identifying this as an address conflict in 
reverse order'
@@ -749,7 +749,7 @@
           )),
         ),
       ),
-      11 => array(
+      'conflicting_home_address_major_gifts' => array(
         'conflicting_home_address_major_gifts' => array(
           'merged' => 0,
           'skipped' => 1,
@@ -775,7 +775,7 @@
           )),
         ),
       ),
-      12 => array(
+      'conflicting_home_address_not_major_gifts' => array(
         'conflicting_home_address_not_major_gifts' => array(
           'fix_required_for_reverse' => 1,
           'comment' => 'our code needs an update as both are being kept'
@@ -805,7 +805,7 @@
           )),
         ),
       ),
-      13 => array(
+      'conflicting_home_address_one_more_major_gifts' => array(
         'conflicting_home_address_one_more_major_gifts' => array(
           'merged' => 0,
           'skipped' => 1,
@@ -843,7 +843,7 @@
           )),
         ),
       ),
-      14 => array(
+      'conflicting_home_address__one_more_not_major_gifts' => array(
         'conflicting_home_address__one_more_not_major_gifts' => array(
           'fix_required_for_reverse' => 1,
           'comment' => 'our code needs an update as an extra 1 is being kept'
@@ -885,6 +885,75 @@
           )),
         ),
       ),
+      'duplicate_home_address_on_one_contact' => array(
+        'duplicate_home_address_on_one_contact' => array(
+          'fix_required_for_reverse' => 1,
+          'comment' => 'our code needs an update as an extra 1 is being kept'
+            . ' this is not an issue at the moment as it only happens in 
reverse from the'
+            . 'form merge - where we do not intervene',
+          'merged' => 1,
+          'skipped' => 0,
+          'is_major_gifts' => 0,
+          'entity' => $entity,
+          'contact_1' => array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 0,
+            ), $locationParams1),
+          ),
+          'contact_2' => array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 1,
+            ), $locationParams1),
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 0,
+            ), $locationParams1),
+          ),
+          'expected_hook' => array_merge($additionalExpected, array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 1,
+            ), $locationParams1),
+          )),
+        ),
+      ),
+      'duplicate_home_address_on_one_contact_second_primary' => array(
+        'duplicate_home_address_on_one_contact_second_primary' => array(
+          'fix_required_for_reverse' => 1,
+          'comment' => 'our code needs an update as an extra 1 is being kept'
+            . ' this is not an issue at the moment as it only happens in 
reverse from the'
+            . 'form merge - where we do not intervene',
+          'merged' => 1,
+          'skipped' => 0,
+          'is_major_gifts' => 0,
+          'entity' => $entity,
+          'contact_1' => array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 0,
+            ), $locationParams1),
+          ),
+          'contact_2' => array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 0,
+            ), $locationParams1),
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 1,
+            ), $locationParams1),
+          ),
+          'expected_hook' => array_merge($additionalExpected, array(
+            array_merge(array(
+              'location_type_id' => 'Home',
+              'is_primary' => 1,
+            ), $locationParams1),
+          )),
+        ),
+      ),
+
     );
     return $data;
   }
diff --git a/sites/all/modules/wmf_civicrm/wmf_civicrm.module 
b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
index 21d09d9..376d3e8 100644
--- a/sites/all/modules/wmf_civicrm/wmf_civicrm.module
+++ b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
@@ -2281,8 +2281,9 @@
   // In the test this should always be true - but keep the check in case
   // something changes that we need to detect.
   if ($lastDonorID != $mainId) {
-    $higherPriorityLocationBlocks = 
$migrationInfo['other_details']['location_blocks'];
+    $higherPriorityLocationBlocks = 
_wmf_civicrm_clean_duplicate_location_from_merge_blocks($migrationInfo['other_details']['location_blocks']);
     $lowerPriorityLocationBlocks = 
$migrationInfo['main_details']['location_blocks'];
+
     foreach ($higherPriorityLocationBlocks as $blockType => $blocks) {
       foreach ($blocks as $block) {
         if (empty($lowerPriorityLocationBlocks[$blockType])) {
@@ -2317,6 +2318,46 @@
 }
 
 /**
+ * Clean up any instances of 2 identical home addresses before processing.
+ *
+ * It's not valid to have 2 addresses of the same location type in CiviCRM.
+ * However, they seem to still be coming in so for now ensure they don't cause
+ * issues when deduping, with the is_primary flag being lost.
+ *
+ * T145873
+ *
+ * @param array $blocks
+ *   An array of locations associated with a contact. These are keyed by entity
+ *   (email, address, phone) and under that an array of db entries.
+ *
+ * @return array
+ *   Blocks, possibly with some removed.
+ */
+function _wmf_civicrm_clean_duplicate_location_from_merge_blocks($blocks) {
+  foreach ($blocks as $entity => $locationBlocks) {
+    $locations = array();
+    $primaryID = NULL;
+    foreach ($locationBlocks as $index => $locationBlock) {
+      $locations[$locationBlock['location_type_id']][$index] = 
$locationBlock['id'];
+      if ($locationBlock['is_primary']) {
+        $primaryID = $locationBlock['id'];
+      }
+    }
+    foreach ($locations as $locationsOfType) {
+      if (count($locationsOfType) > 1) {
+        // We have a duplicate.
+        foreach ($locationsOfType as $index => $locationID) {
+          if ($locationID <> $primaryID) {
+            unset($blocks[$entity][$index]);
+          }
+        }
+      }
+    }
+  }
+  return $blocks;
+}
+
+/**
  * Add the calculated fields to the migration info when merging.
  *
  * The contributions have already been merged so we can query the DB for this 
info. We

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icee9aab5179e6e42cbedd2fa615072053834962b
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Eileen <emcnaugh...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to