From abb9bd725538ae6ccd1e4da2138e744c3e1e8664 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 15 Nov 2021 07:23:31 +1300 Subject: [PATCH] Ensure groupElementType is always set --- CRM/Contact/Form/Contact.php | 13 ++++++++ CRM/Contact/Form/Edit/TagsAndGroups.php | 13 ++++---- CRM/Core/Form.php | 20 +++++++++--- .../CRM/Contact/Form/Edit/TagsAndGroups.tpl | 26 +++++++-------- templates/CRM/common/TabHeader.tpl | 6 ++-- .../CRM/Contact/Form/IndividualTest.php | 32 +++++++++---------- 6 files changed, 66 insertions(+), 44 deletions(-) diff --git a/CRM/Contact/Form/Contact.php b/CRM/Contact/Form/Contact.php index 2bbb7a36ef..7168f0a9f8 100644 --- a/CRM/Contact/Form/Contact.php +++ b/CRM/Contact/Form/Contact.php @@ -126,6 +126,19 @@ class CRM_Contact_Form_Contact extends CRM_Core_Form { 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 index a3d060fc47..e9ffa6f667 100644 --- a/CRM/Contact/Form/Edit/TagsAndGroups.php +++ b/CRM/Contact/Form/Edit/TagsAndGroups.php @@ -61,7 +61,8 @@ class CRM_Contact_Form_Edit_TagsAndGroups { 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 @@ class CRM_Contact_Form_Edit_TagsAndGroups { $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 @@ class CRM_Contact_Form_Edit_TagsAndGroups { } } - 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, ' 
'); $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 index 68574f3aa6..41f97aaf7b 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -328,11 +328,6 @@ class CRM_Core_Form extends HTML_QuickForm_Page { 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 @@ class CRM_Core_Form extends HTML_QuickForm_Page { 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 @@ class CRM_Core_Form extends HTML_QuickForm_Page { 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 index 03c71d3b8e..211a078971 100644 --- a/templates/CRM/Contact/Form/Edit/TagsAndGroups.tpl +++ b/templates/CRM/Contact/Form/Edit/TagsAndGroups.tpl @@ -14,7 +14,7 @@ {/if} - {if empty($type) || $type eq 'tag'} + {if $form.tag} {/if} - {if empty($type) || $type eq 'group'} + {if $form.group} {/if} diff --git a/templates/CRM/common/TabHeader.tpl b/templates/CRM/common/TabHeader.tpl index fef8e706e3..7d8ae7ad0d 100644 --- 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 *}
- {if !empty($tabHeader) and count($tabHeader)} + {if $tabHeader}
    {foreach from=$tabHeader key=tabName item=tabValue}
  • {if $tabValue.active} - - {if !empty($tabValue.icon)}{/if} + + {if $tabValue.icon}{/if} {$tabValue.title} {if is_numeric($tabValue.count)}{$tabValue.count}{/if} diff --git a/tests/phpunit/CRM/Contact/Form/IndividualTest.php b/tests/phpunit/CRM/Contact/Form/IndividualTest.php index 234b32e5a6..f33aed6366 100644 --- 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('
{if !empty($title)}{$form.tag.label}
{/if} @@ -25,24 +25,22 @@ {/if}
- {if isset($groupElementType) && $groupElementType eq 'select'} + {if $groupElementType eq 'select'}
- {if !empty($title)}{$form.group.label}
{/if} + {if $title}{$form.group.label}
{/if} {$form.group.html}
{else} - {if isset($form.group)} - {foreach key=key item=item from=$tagGroup.group} -
- {$form.group.$key.html} - {if $item.description} -
{$item.description}
- {/if} -
- {/foreach} - {/if} + {foreach key=key item=item from=$tagGroup.group} +
+ {$form.group.$key.html} + {if $item.description} +
{$item.description}
+ {/if} +
+ {/foreach} {/if}