dev/core#1046

diff --git a/CRM/Case/XMLProcessor/Process.php b/CRM/Case/XMLProcessor/Process.php
index 4cfb5c8d7e1abcf6142a5e97ab2acc580d1663c3..d2c2cfbe2eee1c05bd6f5dfb13c58ca84bfae537 100644 (file)
--- 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 53dcd83690acdd2d87a4addeb77e4f0daa3a0cd0..e7c04f6dc4476ac1daabf2c21c7242606b29010c 100644 (file)
--- 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'],
+    ];
+  }
+
 }