Skip to content

Commit

Permalink
Remove smarty isset in favour of always set
Browse files Browse the repository at this point in the history
  • Loading branch information
eileenmcnaughton committed Nov 17, 2021
1 parent dafec03 commit a1a6b25
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CRM/Contact/Import/Form/DataSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}

Expand Down
4 changes: 2 additions & 2 deletions CRM/Core/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ public function setOptions($options) {
*
* @return array
*/
public function getOptionalSmartyElements(): array {
public function getOptionalQuickFormElements(): array {
return [];
}

Expand All @@ -1078,7 +1078,7 @@ public function toSmarty() {
$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;
}
Expand Down
3 changes: 3 additions & 0 deletions CRM/Group/Form/Search.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions templates/CRM/Group/Form/Search.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
},
Expand Down Expand Up @@ -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];
Expand Down
34 changes: 29 additions & 5 deletions tests/phpunit/CRM/Core/Page/HookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,21 @@ public function setUp(): void {
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'];
Expand All @@ -80,14 +81,37 @@ public function testFormsCallBuildFormOnce() {
'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.
Expand Down

0 comments on commit a1a6b25

Please sign in to comment.