Merge pull request #20956 from eileenmcnaughton/valid_fix

Fix test to use valid financials

Civi/Test/ContactTestTrait.php

@@ -54,8 +54,6 @@ public function createLoggedInUser(): int {
    *
    * @return int
    *   id of Organisation created
-   *
-   * @throws \CiviCRM_API3_Exception
    */
   public function organizationCreate($params = [], $seq = 0): int {
     if (!$params) {

tests/phpunit/CRM/Contribute/BAO/ContributionTest.php

@@ -20,6 +20,13 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase {
   use CRMTraits_Financial_FinancialACLTrait;
   use CRMTraits_Financial_PriceSetTrait;
 
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = TRUE;
+
   /**
    * Clean up after tests.
    */
@@ -666,7 +673,9 @@ public function checkItemValues($contribution) {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function testAssignProportionalLineItems() {
+  public function testAssignProportionalLineItems(): void {
+    // This test doesn't seem to manage financials properly, possibly by design
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $contribution = $this->addParticipantWithContribution();
     // Delete existing financial_trxns. This is because we are testing a code flow we
     // want to deprecate & remove & the test relies on bad data asa starting point.

tests/phpunit/CRM/Event/Form/ParticipantTest.php

@@ -22,17 +22,11 @@ public function setUp(): void {
   }
 
   /**
-   * CHeck that all tests that have created payments have created them with the right financial entities.
+   * Should financials be checked after the test but before tear down.
    *
-   * Ideally this would be on CiviUnitTestCase but many classes would still fail. Also, it might
-   * be good if it only ran on tests that created at least one contribution.
-   *
-   * @throws \CRM_Core_Exception
+   * @var bool
    */
-  protected function assertPostConditions(): void {
-    $this->validateAllPayments();
-    $this->validateAllContributions();
-  }
+  protected $isValidateFinancialsOnPostAssert = TRUE;
 
   /**
    * Initial test of submit function.

tests/phpunit/CRM/Event/Form/Registration/ConfirmTest.php

@@ -10,6 +10,17 @@ class CRM_Event_Form_Registration_ConfirmTest extends CiviUnitTestCase {
 
   use CRMTraits_Profile_ProfileTrait;
 
+
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * Ideally all tests (or at least all that call any financial api calls ) should do this but there
+   * are some test data issues and some real bugs currently blocking.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = TRUE;
+
   public function setUp(): void {
     $this->useTransaction(TRUE);
     parent::setUp();
@@ -95,6 +106,8 @@ public function testSubmit(): void {
    * @dataProvider getThousandSeparators
    */
   public function testPaidSubmit($thousandSeparator) {
+    // @todo - figure out why this doesn't pass validate financials
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $this->setCurrencySeparators($thousandSeparator);
     $mut = new CiviMailUtils($this);
     $paymentProcessorID = $this->processorCreate();
@@ -222,6 +235,8 @@ public function testPaidSubmit($thousandSeparator) {
    * @throws \Exception
    */
   public function testTaxMultipleParticipant() {
+    // @todo - figure out why this doesn't pass validate financials
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $mut = new CiviMailUtils($this);
     $params = ['is_monetary' => 1, 'financial_type_id' => 1];
     $event = $this->eventCreate($params);

tests/phpunit/CiviTest/CiviUnitTestCase.php

@@ -486,7 +486,6 @@ protected function loadXMLDataSet($fileName) {
    * Create default domain contacts for the two domains added during test class.
    * database population.
    *
-   * @throws \CiviCRM_API3_Exception
    * @throws \API_Exception
    */
   public function createDomainContacts(): void {

tests/phpunit/api/v3/ACLPermissionTest.php

@@ -25,6 +25,16 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase {
 
   use Civi\Test\ACLPermissionTrait;
 
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * The setup methodology in this class bypasses valid financial creation
+   * so we don't check.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = FALSE;
+
   public $DBResetRequired = FALSE;
   protected $_entity;
 

tests/phpunit/api/v3/ContributionTest.php

@@ -3468,8 +3468,6 @@ public function testCompleteTransactionMembershipPriceSet() {
    * @throws \CiviCRM_API3_Exception
    */
   public function testPendingToCompleteContribution(): void {
-    // @todo - figure out why this test is not valid.
-    $this->isValidateFinancialsOnPostAssert = FALSE;
     $this->createPriceSetWithPage('membership');
     $this->setUpPendingContribution($this->_ids['price_field_value'][0]);
     $this->callAPISuccess('membership', 'getsingle', ['id' => $this->_ids['membership']]);
@@ -3729,8 +3727,6 @@ public function testSendMail(): void {
    */
   public function testSendConfirmationPayLater(): void {
     $mut = new CiviMailUtils($this, TRUE);
-    // This probably needs to call the order api in order to generate valid financial entities.
-    $this->isValidateFinancialsOnPostAssert = FALSE;
     // Create contribution page
     $pageParams = [
       'title' => 'Webform Contributions',
@@ -3741,30 +3737,30 @@ public function testSendConfirmationPayLater(): void {
       'pay_later_text' => 'I will send payment by cheque',
       'pay_later_receipt' => 'Send your cheque payable to "CiviCRM LLC" to the office',
     ];
-    $contributionPage = $this->callAPISuccess('contribution_page', 'create', $pageParams);
+    $contributionPage = $this->callAPISuccess('ContributionPage', 'create', $pageParams);
 
     // Create pay later contribution
-    $contribParams = [
+    $contribution = $this->callAPISuccess('Order', 'create', [
       'contact_id' => $this->_individualId,
       'financial_type_id' => 1,
       'is_pay_later' => 1,
       'contribution_status_id' => 2,
       'contribution_page_id' => $contributionPage['id'],
       'total_amount' => '10.00',
-    ];
-    $contribution = $this->callAPISuccess('contribution', 'create', $contribParams);
-
-    // Create line item
-    $lineItemParams = [
-      'contribution_id' => $contribution['id'],
-      'entity_id' => $contribution['id'],
-      'entity_table' => 'civicrm_contribution',
-      'label' => 'My lineitem label',
-      'qty' => 1,
-      'unit_price' => '10.00',
-      'line_total' => '10.00',
-    ];
-    $this->callAPISuccess('LineItem', 'create', $lineItemParams);
+      'line_items' => [
+        [
+          'line_item' => [
+            [
+              'entity_table' => 'civicrm_contribution',
+              'label' => 'My lineitem label',
+              'qty' => 1,
+              'unit_price' => '10.00',
+              'line_total' => '10.00',
+            ],
+          ],
+        ],
+      ],
+    ]);
 
     // Create email
     try {
@@ -3785,7 +3781,7 @@ public function testSendConfirmationPayLater(): void {
     // Retrieve mail & check it has the pay_later_receipt info
     $mut->getMostRecentEmail('raw');
     $mut->checkMailLog([
-      (string) $contribParams['total_amount'],
+      (string) 10,
       $pageParams['pay_later_receipt'],
     ], [
       'Event',

tests/phpunit/api/v3/LineItemTest.php

@@ -19,6 +19,13 @@ class api_v3_LineItemTest extends CiviUnitTestCase {
   protected $params;
   protected $_entity = 'line_item';
 
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = TRUE;
+
   /**
    * Prepare for test.
    *
@@ -143,7 +150,9 @@ public function testGetBasicLineItem($version): void {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testDeleteLineItem($version): void {
+  public function testDeleteLineItem(int $version): void {
+    // Deleting line items does not leave valid financial data.
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $this->_apiversion = $version;
     $getParams = [
       'entity_table' => 'civicrm_contribution',

tests/phpunit/api/v3/MembershipPaymentTest.php

@@ -23,6 +23,17 @@ class api_v3_MembershipPaymentTest extends CiviUnitTestCase {
   protected $_membershipStatusID;
   protected $_contribution = [];
 
+  /**
+   * Should financials be checked after the test but before tear down.
+   *
+   * This test class is opted out as this method should not be called outside
+   * of the LineItem::create function and the test is artificial & not creating
+   * valid financials.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = FALSE;
+
   public function setUp(): void {
     parent::setUp();
     $this->useTransaction(TRUE);

tests/phpunit/api/v3/ProfileTest.php

@@ -542,6 +542,8 @@ public function testMembershipGetFieldsOrder(): void {
    * @throws \CiviCRM_API3_Exception
    */
   public function testProfileSubmitMembershipBatch(): void {
+    // @todo - figure out why this doesn't pass validate financials
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $this->_contactID = $this->individualCreate();
     $this->callAPISuccess('profile', 'submit', [
       'profile_id' => 'membership_batch_entry',