From 5214f03bbc28ffb57c683660637ebfea5a0ea7e4 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 1 Jul 2019 13:01:19 +1200
Subject: [PATCH] [REF] move formatting of conflict into markConflict function

The goal here is to make this information more usable to an api caller. Currently the 'conflict' key holds text formatted
for a specific form. I was going to change this to hold an array of data that an api could use. However, the
Contact.get_merge_conflicts already has a specific api-friendly format that is keyed by 'mode'
at the top level so my revised plan is to return api-friendly data as well (without messign with the
existing form's data).

This change simply moves the decisions about what data to store & how down to the function whose responsibility
it is (markConflict) and adds a test to ensure that no change results

[REF] move formatting of conflict into markConflict function

The goal here is to make this information more usable to an api caller. Currently the 'conflict' key holds text formatted
for a specific form. I was going to change this to hold an array of data that an api could use. However, the
Contact.get_merge_conflicts already has a specific api-friendly format that is keyed by 'mode'
at the top level so my revised plan is to return api-friendly data as well (without messign with the
existing form's data).

This change simply moves the decisions about what data to store & how down to the function whose responsibility
it is (markConflict) and adds a test to ensure that no change results
---
 CRM/Core/BAO/PrevNextCache.php   | 7 ++++++-
 CRM/Dedupe/Merger.php            | 8 +++++++-
 Civi/Test/Api3TestTrait.php      | 5 +++--
 tests/phpunit/api/v3/JobTest.php | 6 ++----
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php
index 116f87c19e..a886e6d184 100644
--- a/CRM/Core/BAO/PrevNextCache.php
+++ b/CRM/Core/BAO/PrevNextCache.php
@@ -192,11 +192,16 @@ WHERE  cachekey     = %3 AND
     ];
     $pncFind = CRM_Core_DAO::executeQuery($sql, $params);
 
+    $conflictTexts = [];
+    foreach ($conflicts as $conflict) {
+      $conflictTexts[] = "{$conflict['title']}: '{$conflict[$id1]}' vs. '{$conflict[$id2]}'";
+    }
+    $conflictString = implode(', ', $conflictTexts);
     while ($pncFind->fetch()) {
       $data = $pncFind->data;
       if (!empty($data)) {
         $data = CRM_Core_DAO::unSerializeField($data, CRM_Core_DAO::SERIALIZE_PHP);
-        $data['conflicts'] = implode(",", array_values($conflicts));
+        $data['conflicts'] = $conflictString;
 
         $pncUp = new CRM_Core_DAO_PrevNextCache();
         $pncUp->id = $pncFind->id;
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
index 20b5711a8a..91d5a6c882 100644
--- a/CRM/Dedupe/Merger.php
+++ b/CRM/Dedupe/Merger.php
@@ -2129,7 +2129,13 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
     // store any conflicts
     if (!empty($conflicts)) {
       foreach ($conflicts as $key => $dnc) {
-        $conflicts[$key] = "{$migrationInfo['rows'][$key]['title']}: '{$migrationInfo['rows'][$key]['main']}' vs. '{$migrationInfo['rows'][$key]['other']}'";
+        unset($conflicts[$key]);
+        $conflicts[substr($key, 5)] = [
+          'title' => $migrationInfo['rows'][$key]['title'],
+          $mainId => $migrationInfo['rows'][$key]['main'],
+          $otherId => $migrationInfo['rows'][$key]['other'],
+          'key' => $key,
+        ];
       }
       CRM_Core_BAO_PrevNextCache::markConflict($mainId, $otherId, $cacheKeyString, $conflicts);
     }
diff --git a/Civi/Test/Api3TestTrait.php b/Civi/Test/Api3TestTrait.php
index 3b50e4862f..1c99c0490b 100644
--- a/Civi/Test/Api3TestTrait.php
+++ b/Civi/Test/Api3TestTrait.php
@@ -249,6 +249,7 @@ trait Api3TestTrait {
    *   - object
    *
    * @return array|int
+   * @throws \CRM_Core_Exception
    */
   public function callAPISuccessGetValue($entity, $params, $type = NULL) {
     $params += [
@@ -257,10 +258,10 @@ trait Api3TestTrait {
     ];
     $result = $this->civicrm_api($entity, 'getvalue', $params);
     if (is_array($result) && (!empty($result['is_error']) || isset($result['values']))) {
-      throw new \Exception('Invalid getvalue result' . print_r($result, TRUE));
+      throw new \CRM_Core_Exception('Invalid getvalue result' . print_r($result, TRUE));
     }
     if ($type) {
-      if ($type == 'integer') {
+      if ($type === 'integer') {
         // api seems to return integers as strings
         $this->assertTrue(is_numeric($result), "expected a numeric value but got " . print_r($result, 1));
       }
diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
index b20d3f260d..802dd15b72 100644
--- a/tests/phpunit/api/v3/JobTest.php
+++ b/tests/phpunit/api/v3/JobTest.php
@@ -889,10 +889,8 @@ class api_v3_JobTest extends CiviUnitTestCase {
     // Each row specifies: contact 1 on_hold, contact 2 on_hold, merge? (0 or 1),
     $sets = [
       [0, 0, 1, NULL],
-      [0, 1, 0, "Email 2 (Work): 'batman@gotham.met' vs. 'batman@gotham.met
-(On Hold)'"],
-      [1, 0, 0, "Email 2 (Work): 'batman@gotham.met
-(On Hold)' vs. 'batman@gotham.met'"],
+      [0, 1, 0, "Email 2 (Work): 'batman@gotham.met' vs. 'batman@gotham.met\n(On Hold)'"],
+      [1, 0, 0, "Email 2 (Work): 'batman@gotham.met\n(On Hold)' vs. 'batman@gotham.met'"],
       [1, 1, 1, NULL],
     ];
     return $sets;
-- 
2.25.1