From 90b5313bcd2fa3f559b880e94bd7331b0473ff77 Mon Sep 17 00:00:00 2001
From: DemeritCowboy <demeritcowboy@hotmail.com>
Date: Sun, 1 Sep 2019 12:57:53 -0400
Subject: [PATCH] dev/core#1046

---
 CRM/Case/XMLProcessor/Process.php             | 31 ++++++++++++++++++-
 .../CRM/Case/XMLProcessor/ProcessTest.php     | 28 +++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/CRM/Case/XMLProcessor/Process.php b/CRM/Case/XMLProcessor/Process.php
index 4cfb5c8d7e..d2c2cfbe2e 100644
--- a/CRM/Case/XMLProcessor/Process.php
+++ b/CRM/Case/XMLProcessor/Process.php
@@ -105,7 +105,7 @@ class CRM_Case_XMLProcessor_Process extends CRM_Case_XMLProcessor {
       foreach ($xml->CaseRoles as $caseRoleXML) {
         foreach ($caseRoleXML->RelationshipType as $relationshipTypeXML) {
           if ((int ) $relationshipTypeXML->creator == 1) {
-            if (!$this->createRelationships((string ) $relationshipTypeXML->name,
+            if (!$this->createRelationships($this->locateNameOrLabel($relationshipTypeXML),
               $params
             )
             ) {
@@ -846,4 +846,33 @@ AND        a.is_deleted = 0
     return $default;
   }
 
+  /**
+   * At some point name and label got mixed up for case roles.
+   * Check for higher priority tag <machineName> first which represents name, then fall back to the <name> tag which somehow became label.
+   * We do this to avoid requiring people to update their xml files which can be stored in external files.
+   *
+   * Note this is different than doing something like comparing the <name> tag against name in the database and then falling back to comparing label in the database, which is subject to an edge case where you would get the wrong one (where the label of one relationship type is the same as the name of another). Here there are two tags with explicit single meanings.
+   *
+   * @param SimpleXMLElement $xml
+   *
+   * @return string
+   */
+  public function locateNameOrLabel($xml) {
+    /* While it's unlikely, it's possible somebody is using '0' as their machineName, so we should let them.
+     * Specifically if machineName is:
+     * missing - use name
+     * null - use name
+     * blank - use name
+     * the string '0' - use machineName
+     * the number 0 - use machineName (but can't really have number 0 in simplexml unless cast to number)
+     * the word 'null' - use machineName and best not to think about it
+     */
+    if (empty($xml->machineName)) {
+      if (!isset($xml->machineName) || ((string) $xml->machineName) !== '0') {
+        return (string) $xml->name;
+      }
+    }
+    return (string) $xml->machineName;
+  }
+
 }
diff --git a/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php b/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php
index 53dcd83690..e7c04f6dc4 100644
--- a/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php
+++ b/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php
@@ -239,4 +239,32 @@ class CRM_Case_XMLProcessor_ProcessTest extends CiviCaseTestCase {
     $this->assertEquals($expectedContact, $activity['assignee_contact_id'], 'Activity is not assigned to expected contact');
   }
 
+  /**
+   * Test that locateNameOrLabel does the right things.
+   *
+   * @dataProvider xmlDataProvider
+   */
+  public function testLocateNameOrLabel($xmlString, $expected) {
+    $xmlObj = new SimpleXMLElement($xmlString);
+    $this->assertEquals($expected, $this->process->locateNameOrLabel($xmlObj));
+  }
+
+  /**
+   * Data provider for testLocateNameOrLabel
+   * @return array
+   */
+  public function xmlDataProvider() {
+    return [
+      ['<RelationshipType><name>Senior Services Coordinator</name><creator>1</creator><manager>1</manager></RelationshipType>', 'Senior Services Coordinator'],
+      ['<RelationshipType><name>Senior Services Coordinator</name></RelationshipType>', 'Senior Services Coordinator'],
+      ['<RelationshipType><name>Lion Tamer&#39;s Obituary Writer</name></RelationshipType>', "Lion Tamer's Obituary Writer"],
+      ['<RelationshipType><machineName>BP1234</machineName><name>Banana Peeler</name></RelationshipType>', 'BP1234'],
+      ['<RelationshipType><machineName>BP1234</machineName><name>Banana Peeler</name><creator>1</creator><manager>1</manager></RelationshipType>', 'BP1234'],
+      ['<RelationshipType><machineName>0</machineName><name>Assistant Level 0</name></RelationshipType>', '0'],
+      ['<RelationshipType><machineName></machineName><name>Banana Peeler</name></RelationshipType>', 'Banana Peeler'],
+      // hopefully nobody would do this
+      ['<RelationshipType><machineName>null</machineName><name>Annulled Relationship</name></RelationshipType>', 'null'],
+    ];
+  }
+
 }
-- 
2.25.1