From e6766df92ce13c22a1067a0deac9ef8c15c330f9 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Wed, 23 Jun 2021 01:05:54 -0400
Subject: [PATCH] Ensure custom group name does not conflict with existing
 field

---
 CRM/Core/BAO/CustomGroup.php                  | 26 +++++++++++---
 CRM/Core/DAO/AllCoreTables.php                |  6 ++--
 .../phpunit/CRM/Core/BAO/CustomGroupTest.php  | 36 +++++++++++++++++++
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php
index d8c189a09e..8dda74008d 100644
--- a/CRM/Core/BAO/CustomGroup.php
+++ b/CRM/Core/BAO/CustomGroup.php
@@ -142,14 +142,16 @@ class CRM_Core_BAO_CustomGroup extends CRM_Core_DAO_CustomGroup {
       $group->created_id = $params['created_id'] ?? NULL;
       $group->created_date = $params['created_date'] ?? NULL;
 
-      // we do this only once, so name never changes
-      if (isset($params['name'])) {
-        $group->name = CRM_Utils_String::munge($params['name'], '_', 64);
+      // Process name only during create, so it never changes
+      if (!empty($params['name'])) {
+        $group->name = CRM_Utils_String::munge($params['name']);
       }
       else {
-        $group->name = CRM_Utils_String::munge($group->title, '_', 64);
+        $group->name = CRM_Utils_String::munge($group->title);
       }
 
+      self::validateCustomGroupName($group);
+
       if (isset($params['table_name'])) {
         $tableName = $params['table_name'];
 
@@ -225,6 +227,22 @@ class CRM_Core_BAO_CustomGroup extends CRM_Core_DAO_CustomGroup {
     return CRM_Core_DAO::commonRetrieve('CRM_Core_DAO_CustomGroup', $params, $defaults);
   }
 
+  /**
+   * Ensure group name does not conflict with an existing field
+   *
+   * @param CRM_Core_DAO_CustomGroup $group
+   */
+  public static function validateCustomGroupName(CRM_Core_DAO_CustomGroup $group) {
+    $extends = in_array($group->extends, CRM_Contact_BAO_ContactType::basicTypes(TRUE)) ? 'Contact' : $group->extends;
+    $extendsDAO = CRM_Core_DAO_AllCoreTables::getFullName($extends);
+    if ($extendsDAO) {
+      $fields = array_column($extendsDAO::fields(), 'name');
+      if (in_array($group->name, $fields)) {
+        $group->name .= '0';
+      }
+    }
+  }
+
   /**
    * Update the is_active flag in the db.
    *
diff --git a/CRM/Core/DAO/AllCoreTables.php b/CRM/Core/DAO/AllCoreTables.php
index f117d68148..2db1a16acf 100644
--- a/CRM/Core/DAO/AllCoreTables.php
+++ b/CRM/Core/DAO/AllCoreTables.php
@@ -311,13 +311,13 @@ class CRM_Core_DAO_AllCoreTables {
   /**
    * Given a brief-name, determine the full class-name.
    *
-   * @param string $daoName
+   * @param string $briefName
    *   Ex: 'Contact'.
    * @return string|CRM_Core_DAO|NULL
    *   Ex: 'CRM_Contact_DAO_Contact'.
    */
-  public static function getFullName($daoName) {
-    return CRM_Utils_Array::value($daoName, self::daoToClass());
+  public static function getFullName($briefName) {
+    return self::daoToClass()[$briefName] ?? NULL;
   }
 
   /**
diff --git a/tests/phpunit/CRM/Core/BAO/CustomGroupTest.php b/tests/phpunit/CRM/Core/BAO/CustomGroupTest.php
index 568f8e3ea5..7a8fc3c2c2 100644
--- a/tests/phpunit/CRM/Core/BAO/CustomGroupTest.php
+++ b/tests/phpunit/CRM/Core/BAO/CustomGroupTest.php
@@ -651,4 +651,40 @@ class CRM_Core_BAO_CustomGroupTest extends CiviUnitTestCase {
     $this->assertEquals($extractedGetParams[$fieldName], '2017-06-13');
   }
 
+  /**
+   * @return array
+   */
+  public function getGroupNames(): array {
+    $data = [
+      ['Individual', 'first_name', FALSE],
+      ['Organization', 'first_name', FALSE],
+      ['Contact', 'not_first_name', TRUE],
+      ['Contact', 'gender_id', FALSE],
+      ['Activity', 'activity_type_id', FALSE],
+      ['Campaign', 'activity_type_id', TRUE],
+      ['Campaign', 'campaign_type_id', FALSE],
+    ];
+    // Add descriptive keys to data
+    $dataSet = [];
+    foreach ($data as $item) {
+      $dataSet[$item[0] . '.' . $item[1]] = $item;
+    }
+    return $dataSet;
+  }
+
+  /**
+   * @param string $extends
+   * @param string $name
+   * @param bool $isAllowed
+   * @dataProvider getGroupNames
+   */
+  public function testAllowedGroupNames(string $extends, string $name, bool $isAllowed) {
+    $group = new CRM_Core_DAO_CustomGroup();
+    $group->name = $name;
+    $group->extends = $extends;
+    $expectedName = $isAllowed ? $name : $name . '0';
+    CRM_Core_BAO_CustomGroup::validateCustomGroupName($group);
+    $this->assertEquals($expectedName, $group->name);
+  }
+
 }
-- 
2.25.1