Merge pull request civicrm#16033 from seamuslee001/dev_core_1447

dev/core#1447 Fix hard fail on upgrade due to duplicate names in the …
eileenmcnaughton · Dec 7, 2019 · 292da97 · 292da97
2 parents 66728e5 + 76c5033
commit 292da97
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 8 deletions.
diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyOne.php b/CRM/Upgrade/Incremental/php/FiveTwentyOne.php
@@ -40,10 +40,20 @@ public function setPreUpgradeMessage(&$preUpgradeMessage, $rev, $currentVer = NU
    *   an intermediate version; note that setPostUpgradeMessage is called repeatedly with different $revs.
    */
   public function setPostUpgradeMessage(&$postUpgradeMessage, $rev) {
-    // Example: Generate a post-upgrade message.
-    // if ($rev == '5.12.34') {
-    //   $postUpgradeMessage .= '<br /><br />' . ts("By default, CiviCRM now disables the ability to import directly from SQL. To use this feature, you must explicitly grant permission 'import SQL datasource'.");
-    // }
+    if ($rev == '5.21.alpha1') {
+      // Find any option groups that were not converted during the upgrade.
+      $notConverted = [];
+      $optionGroups = \Civi\Api4\OptionGroup::get()->setCheckPermissions(FALSE)->execute();
+      foreach ($optionGroups as $optionGroup) {
+        $trimmedName = trim($optionGroup['name']);
+        if (strpos($trimmedName, ' ') !== FALSE) {
+          $notConverted[] = $optionGroup['title'];
+        }
+      }
+      if (count($notConverted)) {
+        $postUpgradeMessage .= '<br /><br />' . ts("The Following option Groups have not been converted due to there being already another option group with the same name in the database") . "<ul><li>" . implode('</li><li>', $notConverted) . "</li></ul>";
+      }
+    }
   }
 
   /*
@@ -83,12 +93,19 @@ public static function fixOptionGroupName() {
       $name = trim($optionGroup['name']);
       if (strpos($name, ' ') !== FALSE) {
         $fixedName = CRM_Utils_String::titleToVar(strtolower($name));
-        \Civi::log()->debug('5.21 Upgrade Option Group name ' . $name . ' converted to ' . $fixedName);
-        \Civi\Api4\OptionGroup::update()
-          ->addWhere('id', '=', $optionGroup['id'])
-          ->addValue('name', $fixedName)
+        $check = \Civi\Api4\OptionGroup::get()
+          ->addWhere('name', '=', $fixedName)
           ->setCheckPermissions(FALSE)
           ->execute();
+        // Fix hard fail in upgrade due to name already in database dev/core#1447
+        if (!count($check)) {
+          \Civi::log()->debug('5.21 Upgrade Option Group name ' . $name . ' converted to ' . $fixedName);
+          \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
@@ -488,6 +488,35 @@ public function testFixOptionGroupName() {
     $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');
+    $this->callAPISuccess('OptionGroup', 'delete', ['id' => $optionGroup['id']]);
+  }
+
+  /**
+   * Test that if there is an option group name as the same as the proposed fix name that doesn't cause a hard fail in the upgrade
+   */
+  public function testFixOptionGroupNameWithFixedNameInDatabase() {
+    $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'],
+    ]);
+    $optionGroup2 = $this->callAPISuccess('OptionGroup', 'create', [
+      'title' => 'Test Option Group 2',
+      'name' => $name,
+    ]);
+    $preUpgrade = $this->callAPISuccess('OptionGroup', 'getsingle', ['id' => $optionGroup['id']]);
+    $this->assertEquals($name, $preUpgrade['name']);
+    $preUpgrade = $this->callAPISuccess('OptionGroup', 'getsingle', ['id' => $optionGroup2['id']]);
+    $this->assertEquals($fixedName, $preUpgrade['name']);
+    CRM_Upgrade_Incremental_php_FiveTwentyOne::fixOptionGroupName();
+    $this->callAPISuccess('OptionGroup', 'delete', ['id' => $optionGroup['id']]);
+    $this->callAPISuccess('OptionGroup', 'delete', ['id' => $optionGroup2['id']]);
   }
 
 }