improve flawed test

diff --git a/CRM/Contact/Form/Search/Custom/FullText.php b/CRM/Contact/Form/Search/Custom/FullText.php
index c86d265af93ad5bac528684696ac1ef71ab433f1..12b793fa2fd81741459df170551d7505a3397991 100644 (file)
--- a/CRM/Contact/Form/Search/Custom/FullText.php
+++ b/CRM/Contact/Form/Search/Custom/FullText.php
@@ -316,6 +316,7 @@ WHERE      t.table_name = 'Activity' AND
     $form->assign('limit', self::LIMIT);
 
     // set form defaults
+    $form->assign('table', '');
     if (!empty($form->_formValues)) {
       $defaults = [];
 

diff --git a/templates/CRM/Contact/Form/Search/Custom/FullText.tpl b/templates/CRM/Contact/Form/Search/Custom/FullText.tpl
index f2bcfdebb169d2202bd1ae94ce1d3bd9b8d8e21a..7c12f721ccdcce40a900006fc84764891c939450 100644 (file)
--- a/templates/CRM/Contact/Form/Search/Custom/FullText.tpl
+++ b/templates/CRM/Contact/Form/Search/Custom/FullText.tpl
@@ -34,7 +34,8 @@
   {include file="CRM/Contact/Form/Search/Custom/EmptyResults.tpl"}
 {/if}
 
-{assign var=table value=$form.table.value.0}
+{* @TODO: This is confusing - the variable `table` is already set and used above, and now we set it again to something that is technically different but has the same value, except on a blank form where it doesn't exist, but effectively then is the same value that it already has which is ''. So do we need this line even? *}
+{if (isset($form.table.value.0))}{assign var=table value=$form.table.value.0}{/if}
 {assign var=text  value=$form.text.value}
 {if !empty($summary.Contact) }
   <div class="section">

diff --git a/templates/CRM/Price/Page/Field.tpl b/templates/CRM/Price/Page/Field.tpl
index 88ec508d58c9d1df04ae5f488a6da3d8627aebdd..665ab2add5bef1640016862c168df777bb5d5d98 100644 (file)
--- a/templates/CRM/Price/Page/Field.tpl
+++ b/templates/CRM/Price/Page/Field.tpl
@@ -26,7 +26,8 @@
   </div>
 {/if}
 
-{if $action NEQ 8 and $priceField}
+{* priceField is set when e.g. in browse mode *}
+{if $action NEQ 8 and !empty($priceField)}
 <div class="crm-content-block crm-block">
   <div class="action-link">
     {if !$isReserved}

diff --git a/templates/CRM/Tag/Form/Edit.tpl b/templates/CRM/Tag/Form/Edit.tpl
index 16c1032b4a69efc059f59d108179488a9897728f..fffeb0b46f7e6c4cccd158e4dad71b7ddb642df8 100644 (file)
--- a/templates/CRM/Tag/Form/Edit.tpl
+++ b/templates/CRM/Tag/Form/Edit.tpl
@@ -31,7 +31,8 @@
           <td class="label">{$form.used_for.label}</td>
           <td>{$form.used_for.html} <br />
             <span class="description">
-              {if $is_parent}{ts}You can change the types of records which this tag can be used for by editing the 'Parent' tag.{/ts}
+              {* @TODO: I don't think is_parent is ever true because this form is never used for editing a tag itself, and you can't nest tagsets. And when used to add a new child tag, the used_for element doesn't exist. *}
+              {if !empty($is_parent)}{ts}You can change the types of records which this tag can be used for by editing the 'Parent' tag.{/ts}
               {else}{ts}What types of record(s) can this tag be used for?{/ts}
               {/if}
             </span>

diff --git a/tests/phpunit/CRM/Core/FormTest.php b/tests/phpunit/CRM/Core/FormTest.php
index 34abdec5bbaa1b43b52de741ad236814ca4de942..d6f3fb2e85975a6ebd9a7d3595f77f7f48a6451b 100644 (file)
--- a/tests/phpunit/CRM/Core/FormTest.php
+++ b/tests/phpunit/CRM/Core/FormTest.php
@@ -11,24 +11,31 @@ class CRM_Core_FormTest extends CiviUnitTestCase {
    * on the screen, but which are hidden when using popup forms.
    * So no assertions required.
    *
-   * @param string $classname
-   * @param callable $additionalSetup
-   *   Function that performs some additional setup steps specific to the form
+   * @param string $url
    *
-   * @dataProvider formClassList
+   * @dataProvider formList
    */
-  public function testOpeningForms(string $classname, callable $additionalSetup) {
-    $form = $this->getFormObject($classname);
+  public function testOpeningForms(string $url) {
+    $this->createLoggedInUser();
 
-    // call the callable parameter we were passed in
-    $additionalSetup($form);
+    $_SERVER['REQUEST_URI'] = $url;
+    $urlParts = explode('?', $url);
+    $_GET['q'] = $urlParts[0];
 
-    // typical quickform/smarty flow
-    $form->preProcess();
-    $form->buildQuickForm();
-    $form->setDefaultValues();
-    $form->assign('action', $form->_action ?? CRM_Core_Action::UPDATE);
-    $form->getTemplate()->fetch($form->getTemplateFileName());
+    $parsed = [];
+    parse_str($urlParts[1], $parsed);
+    foreach ($parsed as $param => $value) {
+      $_REQUEST[$param] = $value;
+    }
+
+    $item = CRM_Core_Invoke::getItem([$_GET['q']]);
+    ob_start();
+    CRM_Core_Invoke::runItem($item);
+    ob_end_clean();
+
+    foreach ($parsed as $param => $dontcare) {
+      unset($_REQUEST[$param]);
+    }
   }
 
   /**
@@ -36,51 +43,55 @@ class CRM_Core_FormTest extends CiviUnitTestCase {
    * TODO: Add more forms!
    *
    * @return array
-   *   See first one below for description.
    */
-  public function formClassList() {
+  public function formList(): array {
     return [
       // Array key is descriptive term to make it clearer which form it is when it fails.
       'Add New Tag' => [
-        // classname
-        'CRM_Tag_Form_Edit',
-        // Function that performs some class-specific additional setup steps.
-        // If there's a lot of complex steps then that suggests it should have
-        // its own test elsewhere and doesn't fit well here.
-        function(CRM_Core_Form $form) {},
+        'civicrm/tag/edit?action=add&parent_id=',
       ],
       'Assign Account to Financial Type' => [
-        'CRM_Financial_Form_FinancialTypeAccount',
-        function(CRM_Core_Form $form) {
-          $form->set('id', 1);
-          $form->set('aid', 1);
-          $form->_action = CRM_Core_Action::ADD;
-        },
+        'civicrm/admin/financial/financialType/accounts?action=add&reset=1&aid=1',
       ],
-      // This one is a bit flawed but the only point of this test is to catch
-      // simple stuff. This will catch e.g. "undefined index" and similar.
       'Find Contacts' => [
-        'CRM_Contact_Form_Search_Basic',
-        function(CRM_Core_Form $form) {
-          $form->_action = CRM_Core_Action::BASIC;
-        },
-      ],
-      'New Price Field' => [
-        'CRM_Price_Form_Field',
-        function(CRM_Core_Form $form) {
-          $form->set('sid', 1);
-          $form->_action = CRM_Core_Action::ADD;
-        },
+        'civicrm/contact/search?reset=1',
       ],
-      // Also a bit flawed, but catches simple stuff.
       'Fulltext search' => [
-        'CRM_Contact_Form_Search_Custom',
-        function(CRM_Core_Form $form) {
-          $form->_action = CRM_Core_Action::BASIC;
-          $form->set('csid', 15);
-        },
+        'civicrm/contact/search/custom?csid=15&reset=1',
       ],
     ];
   }
 
+  public function testNewPriceField() {
+    $this->createLoggedInUser();
+
+    $priceSetId = $this->callAPISuccess('PriceSet', 'create', [
+      'is_active' => 1,
+      // extends contribution
+      'extends' => 2,
+      'is_quick_config' => 0,
+      // donation
+      'financial_type_id' => 1,
+      'name' => 'priciest',
+      'title' => 'Priciest Price Set',
+    ])['id'];
+
+    $_SERVER['REQUEST_URI'] = "civicrm/admin/price/field?reset=1&action=add&sid={$priceSetId}";
+    $_GET['q'] = 'civicrm/admin/price/field';
+    $_REQUEST['reset'] = 1;
+    $_REQUEST['action'] = 'add';
+    $_REQUEST['sid'] = $priceSetId;
+
+    $item = CRM_Core_Invoke::getItem([$_GET['q']]);
+    ob_start();
+    CRM_Core_Invoke::runItem($item);
+    ob_end_clean();
+
+    unset($_REQUEST['reset']);
+    unset($_REQUEST['action']);
+    unset($_REQUEST['sid']);
+
+    $this->callAPISuccess('PriceSet', 'delete', ['id' => $priceSetId]);
+  }
+
 }