Eileen has submitted this change and it was merged.
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(-)
Approvals:
Ejegg: Looks good to me, approved
jenkins-bot: Verified
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 b3614b5..654f568 100644
--- a/sites/all/modules/wmf_civicrm/wmf_civicrm.module
+++ b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
@@ -2310,8 +2310,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])) {
@@ -2346,6 +2347,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: merged
Gerrit-Change-Id: Icee9aab5179e6e42cbedd2fa615072053834962b
Gerrit-PatchSet: 2
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Eileen <emcnaugh...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Eileen <emcnaugh...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
