From 076f81b69d1a39b01f7ce42049b51001e6157c3c Mon Sep 17 00:00:00 2001
From: "totten@civicrm.org" <Tim Otten>
Date: Fri, 27 Jun 2014 02:36:14 -0700
Subject: [PATCH] CRM-14798 - CRM_Case_XMLRepository - Don't double-munge the
 case-type name.

Prior to this commit, the naming for a case-type flowed more or less
like:

1. User makes up a display-name (title/label)
2. That title is munged via CRM_Utils_String::titleToVar to produce a machine-name
3. That machine-name is munged via CRM_Case_XMLProcessor::mungeCaseType to produce a file-name

As we begin allowing more varied dataflows for managing CaseType XML files
and CaseType DB records, it becomes more important for the naming behavior
to intuitive.
---
 CRM/Case/BAO/CaseType.php             | 13 +++++++++++++
 CRM/Case/XMLProcessor.php             | 18 ++++++++++++++++--
 CRM/Case/XMLRepository.php            |  5 ++++-
 tests/phpunit/api/v3/CaseTypeTest.php | 16 ++++++++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/CRM/Case/BAO/CaseType.php b/CRM/Case/BAO/CaseType.php
index ae5daeeddc..9e4ae0a80b 100644
--- a/CRM/Case/BAO/CaseType.php
+++ b/CRM/Case/BAO/CaseType.php
@@ -70,6 +70,9 @@ class CRM_Case_BAO_CaseType extends CRM_Case_DAO_CaseType {
     if (!$nameParam && empty($params['id'])) {
       $params['name'] = CRM_Utils_String::titleToVar($params['title']);
     }
+    if (!empty($params['name']) && !CRM_Case_BAO_CaseType::isValidName($params['name'])) {
+      throw new CRM_Core_Exception("Cannot create caseType with malformed name [{$params['name']}]");
+    }
 
     // function to format definition column
     if (isset($params['definition']) && is_array($params['definition'])) {
@@ -309,4 +312,14 @@ class CRM_Case_BAO_CaseType extends CRM_Case_DAO_CaseType {
     $caseType->id = $caseTypeId;
     return $caseType->delete();
   }
+
+  /**
+   * Determine if a case-type name is well-formed
+   *
+   * @param string $caseType
+   * @return bool
+   */
+  static function isValidName($caseType) {
+    return preg_match('/^[a-zA-Z0-9_]+$/',  $caseType);
+  }
 }
diff --git a/CRM/Case/XMLProcessor.php b/CRM/Case/XMLProcessor.php
index c83ed41864..49a6338735 100644
--- a/CRM/Case/XMLProcessor.php
+++ b/CRM/Case/XMLProcessor.php
@@ -54,9 +54,23 @@ class CRM_Case_XMLProcessor {
   }
 
   /**
-   * @param $caseType
+   * This function was previously used to convert a case-type's
+   * machine-name to a file-name. However, it's mind-boggling
+   * that the file-name might be a munged version of the
+   * machine-name (which is itself a munged version of the
+   * display-name), and naming is now a more visible issue (since
+   * the overhaul of CaseType admin UI).
+   *
+   * Usage note: This is called externally by civix stubs as a
+   * sort of side-ways validation of the case-type's name
+   * (validation which was needed because of the unintuitive
+   * double-munge). We should update civix templates and then
+   * remove this function in Civi 4.6 or 5.0.
    *
-   * @return mixed|string
+   * @param string $caseType
+   * @return string
+   * @deprecated
+   * @see CRM_Case_BAO_CaseType::isValidName
    */
   public static function mungeCaseType($caseType) {
     // trim all spaces from $caseType
diff --git a/CRM/Case/XMLRepository.php b/CRM/Case/XMLRepository.php
index 61a504b062..e37ea6d0f5 100644
--- a/CRM/Case/XMLRepository.php
+++ b/CRM/Case/XMLRepository.php
@@ -84,7 +84,10 @@ class CRM_Case_XMLRepository {
       return simplexml_load_string($definition);
     }
 
-    $caseType = CRM_Case_XMLProcessor::mungeCaseType($caseType);
+    if (!CRM_Case_BAO_CaseType::isValidName($caseType)) {
+      // perhaps caller provider a the label instead of the name?
+      throw new CRM_Core_Exception("Cannot load caseType with malformed name [$caseType]");
+    }
 
     if (!CRM_Utils_Array::value($caseType, $this->xml)) {
       // first check custom templates directory
diff --git a/tests/phpunit/api/v3/CaseTypeTest.php b/tests/phpunit/api/v3/CaseTypeTest.php
index 5a5a1597f0..432fa29d6b 100644
--- a/tests/phpunit/api/v3/CaseTypeTest.php
+++ b/tests/phpunit/api/v3/CaseTypeTest.php
@@ -102,6 +102,22 @@ class api_v3_CaseTypeTest extends CiviUnitTestCase {
     $this->assertEquals($result['values'][$id]['title'], $params['title'], 'in line ' . __LINE__);
   }
 
+  /**
+   * Create a case with an invalid name
+   */
+  function testCaseTypeCreate_invalidName() {
+    // Create Case Type
+    $params = array(
+      'title' => 'Application',
+      'name' => 'Appl ication', // spaces are not allowed
+      'is_active' => 1,
+      'weight' => 4,
+    );
+
+    $this->callAPIFailure('CaseType', 'create', $params);
+  }
+
+
   /**
    * Test update (create with id) function with valid parameters
    */
-- 
2.25.1