From 2fd72602a6491197f3fa2846357cb8884a963bdc Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 24 Oct 2019 15:37:19 +1300 Subject: [PATCH 1/3] [REF] minor refactor towards removing complexity. This change actually makes the foreach & if following it redundant. I left them out for legibility but basically all the wrangling is on the 'other' contact. We do just one thing with the 'main' contact so the whole foreach & if is silly. Will follow up with removal of those --- CRM/Dedupe/Merger.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 5020ac7863f0..e513265d807d 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -1091,10 +1091,12 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio // CRM-15681 don't display sub-types in UI continue; } - foreach (['main' => $main, 'other' => $other] as $moniker => $contact) { - list($label, $value) = self::getFieldValueAndLabel($field, $contact); - $rows["move_$field"][$moniker] = $label; + $rows["move_$field"]['main'] = self::getFieldValueAndLabel($field, $main)['label']; + $rows["move_$field"]['other'] = self::getFieldValueAndLabel($field, $other)['label']; + + foreach (['other' => $other] as $moniker => $contact) { if ($moniker == 'other') { + $value = self::getFieldValueAndLabel($field, $other)['value']; //CRM-14334 if ($value === NULL || $value == '') { $value = 'null'; @@ -2524,7 +2526,7 @@ private static function getFieldValueAndLabel($field, $contact): array { elseif ($field == 'current_employer_id' && !empty($value)) { $label = "$value (" . CRM_Contact_BAO_Contact::displayName($value) . ")"; } - return [$label, $value]; + return ['label' => $label, 'value' => $value]; } /** From ccd2609636a6fb19b6421566171b33f42287f441 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 24 Oct 2019 15:40:06 +1300 Subject: [PATCH 2/3] Remove IF as always true --- CRM/Dedupe/Merger.php | 48 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index e513265d807d..5cdab873aff0 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -1095,34 +1095,32 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio $rows["move_$field"]['other'] = self::getFieldValueAndLabel($field, $other)['label']; foreach (['other' => $other] as $moniker => $contact) { - if ($moniker == 'other') { - $value = self::getFieldValueAndLabel($field, $other)['value']; - //CRM-14334 - if ($value === NULL || $value == '') { - $value = 'null'; - } - if ($value === 0 or $value === '0') { - $value = $qfZeroBug; - } - if (is_array($value) && empty($value[1])) { - $value[1] = NULL; - } + $value = self::getFieldValueAndLabel($field, $other)['value']; + //CRM-14334 + if ($value === NULL || $value == '') { + $value = 'null'; + } + if ($value === 0 or $value === '0') { + $value = $qfZeroBug; + } + if (is_array($value) && empty($value[1])) { + $value[1] = NULL; + } - // Display a checkbox to migrate, only if the values are different - if ($value != $main[$field]) { - $elements[] = [ - 'advcheckbox', - "move_$field", - NULL, - NULL, - NULL, - $value, - ]; - } + // Display a checkbox to migrate, only if the values are different + if ($value != $main[$field]) { + $elements[] = [ + 'advcheckbox', + "move_$field", + NULL, + NULL, + NULL, + $value, + ]; + } - $migrationInfo["move_$field"] = $value; + $migrationInfo["move_$field"] = $value; } - } $rows["move_$field"]['title'] = $fields[$field]['title']; } From bdc6e6ee560ceafd5132d2e0151d4d86a39e04c1 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 24 Oct 2019 15:42:37 +1300 Subject: [PATCH 3/3] Remove for-each as silly --- CRM/Dedupe/Merger.php | 48 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 5cdab873aff0..339338bf2a6c 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -1094,33 +1094,31 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio $rows["move_$field"]['main'] = self::getFieldValueAndLabel($field, $main)['label']; $rows["move_$field"]['other'] = self::getFieldValueAndLabel($field, $other)['label']; - foreach (['other' => $other] as $moniker => $contact) { - $value = self::getFieldValueAndLabel($field, $other)['value']; - //CRM-14334 - if ($value === NULL || $value == '') { - $value = 'null'; - } - if ($value === 0 or $value === '0') { - $value = $qfZeroBug; - } - if (is_array($value) && empty($value[1])) { - $value[1] = NULL; - } + $value = self::getFieldValueAndLabel($field, $other)['value']; + //CRM-14334 + if ($value === NULL || $value == '') { + $value = 'null'; + } + if ($value === 0 or $value === '0') { + $value = $qfZeroBug; + } + if (is_array($value) && empty($value[1])) { + $value[1] = NULL; + } - // Display a checkbox to migrate, only if the values are different - if ($value != $main[$field]) { - $elements[] = [ - 'advcheckbox', - "move_$field", - NULL, - NULL, - NULL, - $value, - ]; - } + // Display a checkbox to migrate, only if the values are different + if ($value != $main[$field]) { + $elements[] = [ + 'advcheckbox', + "move_$field", + NULL, + NULL, + NULL, + $value, + ]; + } - $migrationInfo["move_$field"] = $value; - } + $migrationInfo["move_$field"] = $value; $rows["move_$field"]['title'] = $fields[$field]['title']; }