From a1a6b259417d18a79846b3ba0780c3f2fdfc58af Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 16 Nov 2021 09:33:56 +1300 Subject: [PATCH] Remove smarty isset in favour of always set --- CRM/Contact/Import/Form/DataSource.php | 2 +- CRM/Core/Form.php | 4 +-- CRM/Group/Form/Search.php | 3 +++ templates/CRM/Group/Form/Search.tpl | 4 +-- tests/phpunit/CRM/Core/Page/HookTest.php | 34 ++++++++++++++++++++---- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/CRM/Contact/Import/Form/DataSource.php b/CRM/Contact/Import/Form/DataSource.php index e36b492d12..070affb7c8 100644 --- a/CRM/Contact/Import/Form/DataSource.php +++ b/CRM/Contact/Import/Form/DataSource.php @@ -37,7 +37,7 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Core_Form { * * @return array */ - public function getOptionalSmartyElements(): array { + public function getOptionalQuickFormElements(): array { return ['disableUSPS']; } diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index c61a92992f..1143be4580 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -1061,7 +1061,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page { * * @return array */ - public function getOptionalSmartyElements(): array { + public function getOptionalQuickFormElements(): array { return []; } @@ -1078,7 +1078,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page { $content['formName'] = $this->getName(); // CRM-15153 $content['formClass'] = CRM_Utils_System::getClassName($this); - foreach ($this->getOptionalSmartyElements() as $string) { + foreach (array_merge($this->getOptionalQuickFormElements(), $this->expectedSmartyVariables) as $string) { if (!array_key_exists($string, $content)) { $content[$string] = NULL; } diff --git a/CRM/Group/Form/Search.php b/CRM/Group/Form/Search.php index a56623fefc..9ff60f05aa 100644 --- a/CRM/Group/Form/Search.php +++ b/CRM/Group/Form/Search.php @@ -17,6 +17,9 @@ class CRM_Group_Form_Search extends CRM_Core_Form { public function preProcess() { + // This variable does not appear to be set in core civicrm + // and is possibly obsolete? It probably relates to the multisite extension. + $this->expectedSmartyVariables[] = 'showOrgInfo'; parent::preProcess(); CRM_Core_Resources::singleton()->addPermissions('edit groups'); diff --git a/templates/CRM/Group/Form/Search.tpl b/templates/CRM/Group/Form/Search.tpl index 6f24d0f67a..d87e0261f4 100644 --- a/templates/CRM/Group/Form/Search.tpl +++ b/templates/CRM/Group/Form/Search.tpl @@ -136,7 +136,7 @@ d.status = groupStatus, d.savedSearch = $('.crm-group-search-form-block select#saved_search').val(), d.component_mode = $(".crm-group-search-form-block select#component_mode").val(), - d.showOrgInfo = {/literal}{if isset($showOrgInfo)}"{$showOrgInfo}"{else}"0"{/if}{literal}, + d.showOrgInfo = {/literal}{if $showOrgInfo}"{$showOrgInfo}"{else}"0"{/if}{literal}, d.parentsOnly = parentsOnly } }, @@ -186,7 +186,7 @@ // show hide children var context = $('#crm-main-content-wrapper'); $('table.crm-group-selector', context).on( 'click', 'span.show-children', function(){ - var showOrgInfo = {/literal}{if isset($showOrgInfo)}"{$showOrgInfo}"{else}"0"{/if}{literal}; + var showOrgInfo = {/literal}{if $showOrgInfo}"{$showOrgInfo}"{else}"0"{/if}{literal}; var rowID = $(this).parents('tr').prop('id'); var parentRow = rowID.split('_'); var parent_id = parentRow[1]; diff --git a/tests/phpunit/CRM/Core/Page/HookTest.php b/tests/phpunit/CRM/Core/Page/HookTest.php index fc58945c3f..61d76c4e4e 100644 --- a/tests/phpunit/CRM/Core/Page/HookTest.php +++ b/tests/phpunit/CRM/Core/Page/HookTest.php @@ -56,20 +56,21 @@ class CRM_Core_Page_HookTest extends CiviUnitTestCase { continue; } $class = is_array($item['page_callback']) ? $item['page_callback'][0] : $item['page_callback']; - if (in_array($class, $this->skip)) { + if (in_array($class, $this->skip, TRUE)) { continue; } - if (is_subclass_of($class, 'CRM_Core_Page_Basic')) { + if ($class instanceof CRM_Core_Page_Basic) { $this->basicPages[] = $class; } } + CRM_Core_Smarty::singleton()->ensureVariablesAreAssigned($this->getSmartyNoticeVariables()); parent::setUp(); } /** * Make sure form hooks are only invoked once. */ - public function testFormsCallBuildFormOnce() { + public function testFormsCallBuildFormOnce(): void { CRM_Utils_Hook_UnitTests::singleton()->setHook('civicrm_buildForm', [$this, 'onBuildForm']); CRM_Utils_Hook_UnitTests::singleton()->setHook('civicrm_preProcess', [$this, 'onPreProcess']); $_REQUEST = ['action' => 'add']; @@ -80,14 +81,37 @@ class CRM_Core_Page_HookTest extends CiviUnitTestCase { 'preProcess' => [], ]; $page = new $pageName(); - $page->assign('formTpl', NULL); ob_start(); - $page->run(); + try { + $page->run(); + } + catch (Exception $e) { + throw new CRM_Core_Exception($pageName . ' ' . $e->getTraceAsString()); + } ob_end_clean(); $this->runTestAssert($pageName); } } + /** + * Get all the variables that we have learnt will cause notices in this test. + * + * This test is like a sledge hammer - it calls each form without figuring + * out what the form needs to run. For example the first form is + * CRM_Group_Page_Group - when called without an action this winds up calling + * CRM_Group_Form_Edit = but without all the right magic for the form to call + * the template for that form - so it renders the page template with only + * the form values assigned & we wind up in e-notice hell. These notices + * relate to the test not the core forms so we should fix them in the test. + * + * However, it's pretty hard to see how we would turn off e-notices in Smarty + * reliably for just this one test so instead we embark on a journey of + * whack-a-mole where we assign away until the test passes. + */ + protected function getSmartyNoticeVariables(): array { + return ['formTpl', 'showOrgInfo', 'rows', 'gName']; + } + /** * Go through the record of hook invocation and make sure that each hook has * run once and no more than once per class. -- 2.25.1