Merge pull request #17888 from demeritcowboy/groups-warnings

dev/core#1888 and dev/core#1885 - Fatal error on advanced search and warnings and missing group display on contact form
civicrm · Jul 20, 2020 · 204c286 · 204c286
2 parents d2302f9 + ba2ad46
commit 204c286
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 30 deletions.
diff --git a/CRM/Contact/BAO/Group.php b/CRM/Contact/BAO/Group.php
@@ -1055,7 +1055,7 @@ public static function getGroupsHierarchy(
     $groups = [];
     $args = [1 => [$groupIdString, 'String']];
     $query = "
-SELECT id, title, description, visibility, parents, saved_search_id
+SELECT id, title, description, visibility, parents
 FROM   civicrm_group
 WHERE  id IN $groupIdString
 ";
@@ -1081,15 +1081,15 @@ public static function getGroupsHierarchy(
         $parent = self::filterActiveGroups($parentArray);
         $tree[$parent][] = [
           'id' => $dao->id,
-          'title' => empty($dao->saved_search_id) ? $dao->title : '* ' . $dao->title,
+          'title' => $dao->title,
           'visibility' => $dao->visibility,
           'description' => $dao->description,
         ];
       }
       else {
         $roots[] = [
           'id' => $dao->id,
-          'title' => empty($dao->saved_search_id) ? $dao->title : '* ' . $dao->title,
+          'title' => $dao->title,
           'visibility' => $dao->visibility,
           'description' => $dao->description,
         ];
@@ -1123,12 +1123,11 @@ private static function buildGroupHierarchy(&$hierarchy, $group, $tree, $titleOn
       $hierarchy[$group['id']] = $spaces . $group['title'];
     }
     else {
-      $hierarchy[] = array(
-        'id' => $group['id'],
-        'text' => $spaces . $group['title'],
+      $hierarchy[$group['id']] = [
+        'title' => $spaces . $group['title'],
         'description' => $group['description'],
         'visibility' => $group['visibility'],
-      );
+      ];
     }
 
     // For performance reasons we use a for loop rather than a foreach.

diff --git a/CRM/Contact/Form/Contact.php b/CRM/Contact/Form/Contact.php
@@ -869,9 +869,8 @@ public function postProcess() {
       $params['preferred_communication_method'] = 'null';
     }
 
-    $group = CRM_Utils_Array::value('group', $params);
-    if (!empty($group)) {
-      $group = is_array($group) ? $group : explode(',', $group);
+    $group = $params['group'] ?? NULL;
+    if (!empty($group) && is_array($group)) {
       unset($params['group']);
       foreach ($group as $key => $value) {
         $params['group'][$value] = 1;

diff --git a/CRM/Contact/Form/Edit/TagsAndGroups.php b/CRM/Contact/Form/Edit/TagsAndGroups.php
@@ -102,7 +102,7 @@ public static function buildQuickForm(
           }
 
           if ($groupElementType == 'select') {
-            $groupsOptions[$id] = $group;
+            $groupsOptions[$id] = $group['title'];
           }
           else {
             $form->_tagGroup[$fName][$id]['description'] = $group['description'];
@@ -111,8 +111,8 @@ public static function buildQuickForm(
         }
 
         if ($groupElementType == 'select' && !empty($groupsOptions)) {
-          $form->add('select2', $fName, $groupName, $groupsOptions, FALSE,
-            ['placeholder' => '- select -', 'multiple' => TRUE, 'class' => 'twenty']
+          $form->add('select', $fName, $groupName, $groupsOptions, FALSE,
+            ['id' => $fName, 'multiple' => 'multiple', 'class' => 'crm-select2 twenty']
           );
           $form->assign('groupCount', count($groupsOptions));
         }

diff --git a/CRM/Contact/Form/GroupContact.php b/CRM/Contact/Form/GroupContact.php
@@ -73,7 +73,7 @@ public function buildQuickForm() {
         if ($onlyPublicGroups && $group['visibility'] == 'User and User Admin Only') {
           continue;
         }
-        $allGroups[$group['id']] = $group;
+        $allGroups[$id] = $group;
       }
     }
     else {

diff --git a/CRM/Contact/Form/Search/Basic.php b/CRM/Contact/Form/Search/Basic.php
@@ -49,14 +49,13 @@ public function buildQuickForm() {
 
     // add select for groups
     // Get hierarchical listing of groups, respecting ACLs for CRM-16836.
-    $groupHierarchy = CRM_Contact_BAO_Group::getGroupsHierarchy($this->_group, NULL, '&nbsp;&nbsp;');
+    $groupHierarchy = CRM_Contact_BAO_Group::getGroupsHierarchy($this->_group, NULL, '&nbsp;&nbsp;', TRUE);
     if (!empty($searchOptions['groups'])) {
       $this->addField('group', [
         'entity' => 'group_contact',
         'label' => ts('in'),
         'placeholder' => ts('- any group -'),
         'options' => $groupHierarchy,
-        'type' => 'Select2',
       ]);
     }
 

diff --git a/CRM/Contact/Form/Search/Criteria.php b/CRM/Contact/Form/Search/Criteria.php
@@ -42,10 +42,10 @@ public static function basic(&$form) {
       // multiselect for groups
       if ($form->_group) {
         // Arrange groups into hierarchical listing (child groups follow their parents and have indentation spacing in title)
-        $groupHierarchy = CRM_Contact_BAO_Group::getGroupsHierarchy($form->_group, NULL, '&nbsp;&nbsp;');
+        $groupHierarchy = CRM_Contact_BAO_Group::getGroupsHierarchy($form->_group, NULL, '&nbsp;&nbsp;', TRUE);
 
-        $form->add('select2', 'group', ts('Groups'), $groupHierarchy, FALSE,
-          ['placeholder' => ts('- select -'), 'multiple' => TRUE, 'class' => 'twenty']
+        $form->add('select', 'group', ts('Groups'), $groupHierarchy, FALSE,
+          ['id' => 'group', 'multiple' => 'multiple', 'class' => 'crm-select2']
         );
         $groupOptions = CRM_Core_BAO_OptionValue::getOptionValuesAssocArrayFromName('group_type');
         $form->add('select', 'group_type', ts('Group Types'), $groupOptions, FALSE,

diff --git a/ang/crmMailing/Recipients.js b/ang/crmMailing/Recipients.js
@@ -95,7 +95,6 @@
           }
           var option = convertValueToObj(item.id);
           var icon = (option.entity_type === 'civicrm_mailing') ? 'fa-envelope' : 'fa-users';
-          var smartGroupMarker = item.is_smart ? '* ' : '';
           var spanClass = (option.mode == 'exclude') ? 'crmMailing-exclude' : 'crmMailing-include';
           if (option.entity_type != 'civicrm_mailing' && isMandatory(option.entity_id)) {
             spanClass = 'crmMailing-mandatory';
@@ -155,7 +154,7 @@
               mids.push(0);
             }
 
-            CRM.api3('Group', 'getlist', { params: { id: { IN: gids }, options: { limit: 0 } }, extra: ["is_hidden"] }).then(
+            CRM.api3('Group', 'getlist', { params: { id: { IN: gids }, options: { limit: 0 } }, extra: ["is_hidden"] } ).then(
               function(glist) {
                 CRM.api3('Mailing', 'getlist', { params: { id: { IN: mids }, options: { limit: 0 } } }).then(
                   function(mlist) {
@@ -166,7 +165,6 @@
 
                     $(glist.values).each(function (idx, group) {
                       var key = group.id + ' civicrm_group include';
-
                       groupNames.push({id: parseInt(group.id), title: group.label, is_hidden: group.extra.is_hidden});
                       if (values.indexOf(key) >= 0) {
                         datamap.push({id: key, text: group.label});
@@ -234,9 +232,6 @@
               if('civicrm_mailing' === rcpAjaxState.entity) {
                 params["api.MailingRecipients.getcount"] = {};
               }
-              else if ('civicrm_group' === rcpAjaxState.entity) {
-                params.extra = ["saved_search_id"];
-              }
 
               return params;
             },
@@ -260,8 +255,8 @@
                                text: obj.label } : '';
                   }
                   else {
-                    return {   id: obj.id + ' ' + rcpAjaxState.entity + ' ' + rcpAjaxState.type, text: obj.label,
-                              is_smart: (!_.isEmpty(obj.extra.saved_search_id)) };
+                    return {   id: obj.id + ' ' + rcpAjaxState.entity + ' ' + rcpAjaxState.type,
+                               text: obj.label };
                   }
                 })
               };

diff --git a/tests/phpunit/CRM/Contact/Page/View/UserDashboard/GroupContactTest.php b/tests/phpunit/CRM/Contact/Page/View/UserDashboard/GroupContactTest.php
@@ -177,7 +177,7 @@ public function testBrowseDisplaysCorrectListOfAVailableGroups() {
     $group_id_field_html = $form['group_id']['html'];
     $this->assertContains($publicSmartGroupTitle, $group_id_field_html, "Group '$publicSmartGroupTitle' should be in listed available groups, but is not.");
     $this->assertContains($publicStdGroupTitle, $group_id_field_html, "Group '$publicStdGroupTitle' should be in listed available groups, but is not.");
-    $this->assertNotContains('* ' . $adminSmartGroupTitle, $group_id_field_html, "Group '$adminSmartGroupTitle' should not be in listed available groups, but is.");
+    $this->assertNotContains($adminSmartGroupTitle, $group_id_field_html, "Group '$adminSmartGroupTitle' should not be in listed available groups, but is.");
     $this->assertNotContains($adminStdGroupTitle, $group_id_field_html, "Group '$adminStdGroupTitle' should not be in listed available groups, but is.");
 
     // Add current user to the test groups.
@@ -199,9 +199,9 @@ public function testBrowseDisplaysCorrectListOfAVailableGroups() {
 
     $form = CRM_Core_Smarty::singleton()->get_template_vars('form');
     $group_id_field_html = $form['group_id']['html'];
-    $this->assertNotContains('* ' . $publicSmartGroupTitle, $group_id_field_html, "Group '$publicSmartGroupTitle' should not be in listed available groups, but is.");
+    $this->assertNotContains($publicSmartGroupTitle, $group_id_field_html, "Group '$publicSmartGroupTitle' should not be in listed available groups, but is.");
     $this->assertNotContains($publicStdGroupTitle, $group_id_field_html, "Group '$publicStdGroupTitle' should not be in listed available groups, but is.");
-    $this->assertNotContains('* ' . $adminSmartGroupTitle, $group_id_field_html, "Group '$adminSmartGroupTitle' should not be in listed available groups, but is.");
+    $this->assertNotContains($adminSmartGroupTitle, $group_id_field_html, "Group '$adminSmartGroupTitle' should not be in listed available groups, but is.");
     $this->assertNotContains($adminStdGroupTitle, $group_id_field_html, "Group '$adminStdGroupTitle' should not be in listed available groups, but is.");
   }