Ensure groupElementType is always set

civicrm · Nov 29, 2021 · de51640 · de51640
1 parent 85d62e0
commit de51640
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 44 deletions.
diff --git a/CRM/Contact/Form/Contact.php b/CRM/Contact/Form/Contact.php
@@ -126,6 +126,19 @@ public function getDefaultContext() {
     return 'create';
   }
 
+  /**
+   * Get any smarty elements that may not be present in the form.
+   *
+   * To make life simpler for smarty we ensure they are set to null
+   * rather than unset. This is done at the last minute when $this
+   * is converted to an array to be assigned to the form.
+   *
+   * @return array
+   */
+  public function getOptionalSmartyElements(): array {
+    return ['group'];
+  }
+
   /**
    * Build all the data structures needed to build the form.
    */

diff --git a/CRM/Contact/Form/Edit/TagsAndGroups.php b/CRM/Contact/Form/Edit/TagsAndGroups.php
@@ -61,7 +61,8 @@ public static function buildQuickForm(
     if (!isset($form->_tagGroup)) {
       $form->_tagGroup = [];
     }
-
+    $form->addExpectedSmartyVariable('type');
+    $form->addOptionalQuickFormElement('group');
     // NYSS 5670
     if (!$contactId && !empty($form->_contactId)) {
       $contactId = $form->_contactId;
@@ -99,12 +100,12 @@ public static function buildQuickForm(
           $id = $group['id'];
           // make sure that this group has public visibility
           if ($visibility &&
-            $group['visibility'] == 'User and User Admin Only'
+            $group['visibility'] === 'User and User Admin Only'
           ) {
             continue;
           }
 
-          if ($groupElementType == 'select') {
+          if ($groupElementType === 'select') {
             $groupsOptions[$key] = $group;
           }
           else {
@@ -113,23 +114,23 @@ public static function buildQuickForm(
           }
         }
 
-        if ($groupElementType == 'select' && !empty($groupsOptions)) {
+        if ($groupElementType === 'select' && !empty($groupsOptions)) {
           $form->add('select2', $fName, $groupName, $groupsOptions, FALSE,
             ['placeholder' => ts('- select -'), 'multiple' => TRUE, 'class' => 'twenty']
           );
           $form->assign('groupCount', count($groupsOptions));
         }
 
-        if ($groupElementType == 'checkbox' && !empty($elements)) {
+        if ($groupElementType === 'checkbox' && !empty($elements)) {
           $form->addGroup($elements, $fName, $groupName, '&nbsp;<br />');
           $form->assign('groupCount', count($elements));
           if ($isRequired) {
             $form->addRule($fName, ts('%1 is a required field.', [1 => $groupName]), 'required');
           }
         }
-        $form->assign('groupElementType', $groupElementType);
       }
     }
+    $form->assign('groupElementType', $groupElementType ?? NULL);
 
     if ($type & self::TAG) {
       $tags = CRM_Core_BAO_Tag::getColorTags('civicrm_contact');

diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php
@@ -328,11 +328,6 @@ public function __construct(
     if (!isset(self::$_template)) {
       self::$_template = CRM_Core_Smarty::singleton();
     }
-    // Smarty $_template is a static var which persists between tests, so
-    // if something calls clearTemplateVars(), the static still exists but
-    // our ensured variables get blown away, so we need to set them even if
-    // it's already been initialized.
-    self::$_template->ensureVariablesAreAssigned($this->expectedSmartyVariables);
 
     // Workaround for CRM-15153 - give each form a reasonably unique css class
     $this->addClass(CRM_Utils_System::getClassName($this));
@@ -711,6 +706,12 @@ public function buildForm() {
     if ($this->submitOnce) {
       $this->setAttribute('data-submit-once', 'true');
     }
+    // Smarty $_template is a static var which persists between tests, so
+    // if something calls clearTemplateVars(), the static still exists but
+    // our ensured variables get blown away, so we need to set them even if
+    // it's already been initialized.
+    self::$_template->ensureVariablesAreAssigned($this->expectedSmartyVariables);
+
   }
 
   /**
@@ -1091,6 +1092,15 @@ public function getOptionalQuickFormElements(): array {
     return $this->optionalQuickFormElements;
   }
 
+  /**
+   * Add an expected smarty variable to the array.
+   *
+   * @param string $elementName
+   */
+  public function addExpectedSmartyVariable(string $elementName): void {
+    $this->expectedSmartyVariables[] = $elementName;
+  }
+
   /**
    * Render form and return contents.
    *

diff --git a/templates/CRM/Contact/Form/Edit/TagsAndGroups.tpl b/templates/CRM/Contact/Form/Edit/TagsAndGroups.tpl
@@ -14,7 +14,7 @@
 {/if}
     <table class="form-layout-compressed{if $context EQ 'profile'} crm-profile-tagsandgroups{/if}">
       <tr>
-        {if empty($type) || $type eq 'tag'}
+        {if $form.tag}
           <td>
             <div class="crm-section tag-section">
               {if !empty($title)}{$form.tag.label}<br>{/if}
@@ -25,24 +25,22 @@
             {/if}
           </td>
         {/if}
-        {if empty($type) || $type eq 'group'}
+        {if $form.group}
           <td>
-            {if isset($groupElementType) && $groupElementType eq 'select'}
+            {if $groupElementType eq 'select'}
               <div class="crm-section group-section">
-              {if !empty($title)}{$form.group.label}<br>{/if}
+              {if $title}{$form.group.label}<br>{/if}
               {$form.group.html}
             </div>
             {else}
-              {if isset($form.group)} 
-                {foreach key=key item=item from=$tagGroup.group}
-                  <div class="group-wrapper">
-                    {$form.group.$key.html}
-                      {if $item.description}
-                      <div class="description">{$item.description}</div>
-                    {/if}
-                  </div>
-                {/foreach}
-              {/if}
+              {foreach key=key item=item from=$tagGroup.group}
+                <div class="group-wrapper">
+                  {$form.group.$key.html}
+                    {if $item.description}
+                    <div class="description">{$item.description}</div>
+                  {/if}
+                </div>
+              {/foreach}
             {/if}
           </td>
         {/if}

diff --git a/templates/CRM/common/TabHeader.tpl b/templates/CRM/common/TabHeader.tpl
@@ -9,14 +9,14 @@
 *}
 {* enclose all tabs and its content in a block *}
 <div class="crm-block crm-content-block">
-  {if !empty($tabHeader) and count($tabHeader)}
+  {if $tabHeader}
     <div id="mainTabContainer">
     <ul>
        {foreach from=$tabHeader key=tabName item=tabValue}
           <li id="tab_{$tabName}" class="crm-tab-button ui-corner-all{if !$tabValue.valid} disabled{/if} {$tabValue.class}" {$tabValue.extra}>
           {if $tabValue.active}
-             <a href="{if !empty($tabValue.template)}#panel_{$tabName}{else}{$tabValue.link}{/if}" title="{$tabValue.title|escape}{if !$tabValue.valid} ({ts}disabled{/ts}){/if}">
-               {if !empty($tabValue.icon)}<i class="{$tabValue.icon}"></i>{/if}
+             <a href="{if $tabValue.template}#panel_{$tabName}{else}{$tabValue.link}{/if}" title="{$tabValue.title|escape}{if !$tabValue.valid} ({ts}disabled{/ts}){/if}">
+               {if $tabValue.icon}<i class="{$tabValue.icon}"></i>{/if}
                <span>{$tabValue.title}</span>
                {if is_numeric($tabValue.count)}<em>{$tabValue.count}</em>{/if}
              </a>

diff --git a/tests/phpunit/CRM/Contact/Form/IndividualTest.php b/tests/phpunit/CRM/Contact/Form/IndividualTest.php
@@ -15,21 +15,20 @@ class CRM_Contact_Form_IndividualTest extends CiviUnitTestCase {
    * variable in the form so it has a typo, and then this test will throw an
    * exception.
    */
-  public function testOpeningNewIndividualForm() {
-    $form = new CRM_Contact_Form_Contact();
-    $form->controller = new CRM_Core_Controller_Simple('CRM_Contact_Form_Contact', 'New Individual');
-
-    $form->set('reset', '1');
-    $form->set('ct', 'Individual');
+  public function testOpeningNewIndividualForm(): void {
+    $_REQUEST['reset'] = 1;
+    $_REQUEST['ct'] = 'Individual';
+    $form = $this->getFormObject('CRM_Contact_Form_Contact');
+    $form->assign('urlIsPublic');
+    $form->assign('linkButtons', []);
 
     ob_start();
     $form->controller->_actions['display']->perform($form, 'display');
-    $contents = ob_get_contents();
-    ob_end_clean();
+    $contents = ob_get_clean();
 
     // There's a bunch of other stuff we could check for in $contents, but then
     // it's tied to the html output and has the same maintenance problems
-    // as webtests. Mostly what we're doing in this test is checking that it
+    // as web tests. Mostly what we're doing in this test is checking that it
     // doesn't generate any E_NOTICES/WARNINGS or other errors. But let's do
     // one check.
     $this->assertStringContainsString('<label for="first_name">', $contents);
@@ -40,32 +39,33 @@ public function testOpeningNewIndividualForm() {
    * defined. It maybe doesn't need to be a separate test but might make it
    * easier to track down problems if one fails but not the other.
    */
-  public function testOpeningNewIndividualFormWithCustomField() {
+  public function testOpeningNewIndividualFormWithCustomField(): void {
     $custom_group = $this->customGroupCreate([]);
-    $custom_field1 = $this->customFieldCreate(['custom_group_id' => $custom_group['id']]);
-    $custom_field2 = $this->customFieldCreate([
+    $this->customFieldCreate(['custom_group_id' => $custom_group['id']]);
+    $this->customFieldCreate([
       'custom_group_id' => $custom_group['id'],
       'label' => 'f2',
       'html_type' => 'Select',
       // being lazy, just re-use activity type choices
       'option_group_id' => 'activity_type',
     ]);
-    $custom_field3 = $this->customFieldCreate([
+    $this->customFieldCreate([
       'custom_group_id' => $custom_group['id'],
       'label' => 'f3',
       'html_type' => 'Radio',
       'option_group_id' => 'gender',
     ]);
-    $form = new CRM_Contact_Form_Contact();
+    $form = $this->getFormObject('CRM_Contact_Form_Contact');
     $form->controller = new CRM_Core_Controller_Simple('CRM_Contact_Form_Contact', 'New Individual');
+    $form->assign('urlIsPublic');
+    $form->assign('linkButtons', []);
 
     $form->set('reset', '1');
     $form->set('ct', 'Individual');
 
     ob_start();
     $form->controller->_actions['display']->perform($form, 'display');
-    $contents = ob_get_contents();
-    ob_end_clean();
+    $contents = ob_get_clean();
 
     $this->assertStringContainsString('<label for="first_name">', $contents);
   }