dev/core#1405 Strip any spaces from Option Group name parameter and r…

…emove the name field from front end form as will be auto generated Ensure that option group names are lower csae
civicrm · Nov 23, 2019 · 5bd1bff · 5bd1bff
1 parent 47da5c2
commit 5bd1bff
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 14 deletions.
diff --git a/CRM/Admin/Form/OptionGroup.php b/CRM/Admin/Form/OptionGroup.php
@@ -38,17 +38,6 @@ public function buildQuickForm() {
     CRM_Utils_System::setTitle(ts('Dropdown Options'));
 
     $this->applyFilter('__ALL__', 'trim');
-    $this->add('text',
-      'name',
-      ts('Name'),
-      CRM_Core_DAO::getAttribute('CRM_Core_DAO_OptionGroup', 'name'),
-      TRUE
-    );
-    $this->addRule('name',
-      ts('Name already exists in Database.'),
-      'objectExists',
-      ['CRM_Core_DAO_OptionGroup', $this->_id]
-    );
 
     $this->add('text',
       'title',
@@ -90,6 +79,33 @@ public function buildQuickForm() {
     }
 
     $this->assign('id', $this->_id);
+    $this->addFormRule(['CRM_Admin_Form_OptionGroup', 'formRule'], $this);
+  }
+
+  /**
+   * Global form rule.
+   *
+   * @param array $fields
+   *   The input form values.
+   *
+   * @param $files
+   * @param $self
+   *
+   * @return bool|array
+   *   true if no errors, else array of errors
+   */
+  public static function formRule($fields, $files, $self) {
+    $errors = [];
+    if ($self->_id) {
+      $name = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGrou', $self->_id, 'name');
+    }
+    else {
+      $name = CRM_Utils_String::titleToVar(strtolower($fields['title']));
+    }
+    if (!CRM_Core_DAO::objectExists($name, 'CRM_Core_DAO_OptionGroup', $self->_id)) {
+      $errors['title'] = ts('Option Group name already exists in the database. Option Group Names need to be unique');
+    }
+    return empty($errors) ? TRUE : $errors;
   }
 
   /**

diff --git a/CRM/Core/BAO/OptionGroup.php b/CRM/Core/BAO/OptionGroup.php
@@ -76,6 +76,12 @@ public static function add(&$params, $ids = []) {
       CRM_Core_Error::deprecatedFunctionWarning('no $ids array');
       $params['id'] = $ids['optionGroup'];
     }
+    if (empty($params['name']) && empty($params['id'])) {
+      $params['name'] = CRM_Utils_String::titleToVar(strtolower($params['title']));
+    }
+    elseif (!empty($params['name']) && strpos($params['name'], ' ')) {
+      $params['name'] = CRM_Utils_String::titleToVar(strtolower($params['name']));
+    }
     $optionGroup = new CRM_Core_DAO_OptionGroup();
     $optionGroup->copyValues($params);;
     $optionGroup->save();

diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyOne.php b/CRM/Upgrade/Incremental/php/FiveTwentyOne.php
@@ -65,8 +65,32 @@ public function setPostUpgradeMessage(&$postUpgradeMessage, $rev) {
   //    // The above is an exception because 'Upgrade DB to %1: SQL' is generic & reusable.
   //  }
 
-  // public static function taskFoo(CRM_Queue_TaskContext $ctx, ...) {
-  //   return TRUE;
-  // }
+  /**
+   * Upgrade function.
+   *
+   * @param string $rev
+   */
+  public function upgrade_5_21_alpha1($rev) {
+    $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev);
+    $this->addTask('dev/core#1405 Fix option group names that contain spaces', 'fixOptionGroupName');
+  }
+
+  public static function fixOptionGroupName() {
+    $optionGroups = \Civi\Api4\OptionGroup::get()
+      ->setCheckPermissions(FALSE)
+      ->execute();
+    foreach ($optionGroups as $optionGroup) {
+      $name = trim($optionGroup['name']);
+      if (strpos($name, ' ') !== FALSE) {
+        $fixedName = CRM_Utils_String::titleToVar(strtolower($name));
+        \Civi\Api4\OptionGroup::update()
+          ->addWhere('id', '=', $optionGroup['id'])
+          ->addValue('name', $fixedName)
+          ->setCheckPermissions(FALSE)
+          ->execute();
+      }
+    }
+    return TRUE;
+  }
 
 }
diff --git a/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php b/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php
@@ -467,4 +467,27 @@ public function testConvertUpgradeContributeSettings() {
     $this->assertEquals(1, Civi::settings()->get('deferred_revenue_enabled'));
   }
 
+  /**
+   * dev/core#1405 Test fixing option groups with spaces in the name
+   */
+  public function testFixOptionGroupName() {
+    $name = 'This is a test Name';
+    $fixedName = CRM_Utils_String::titleToVar(strtolower($name));
+    $optionGroup = $this->callAPISuccess('OptionGroup', 'create', [
+      'title' => 'Test Option Group',
+      'name' => $name,
+    ]);
+    // API is hardened to strip the spaces to lets re-add in now
+    CRM_Core_DAO::executeQuery("UPDATE civicrm_option_group SET name = %1 WHERE id = %2", [
+      1 => [$name, 'String'],
+      2 => [$optionGroup['id'], 'Positive'],
+    ]);
+    $preUpgrade = $this->callAPISuccess('OptionGroup', 'getsingle', ['id' => $optionGroup['id']]);
+    $this->assertEquals($name, $preUpgrade['name']);
+    CRM_Upgrade_Incremental_php_FiveTwentyOne::fixOptionGroupName();
+    $postUpgrade = $this->callAPISuccess('OptionGroup', 'getsingle', ['id' => $optionGroup['id']]);
+    $this->assertEquals($fixedName, $postUpgrade['name'], 'Ensure that the spaces have been removed from OptionGroup name');
+    $this->assertEquals($postUpgrade['name'], $optionGroup['values'][$optionGroup['id']]['name'], 'Ensure that the fixed name matches what the API would produce');
+  }
+
 }