Tax fixes in unit test

When this->isValidateFinancialsOnPostAssert is true the
test class checks that line items and payments are valid.

I'm trying to enable this for this class. However, there are some issues
that I have found fixes for (and at least 1 I'm still working on)
- some tests try to set tax_amount when it is not enabled
which is invalid - removed
- one test tries to use chaining in a way that
we know is not going to do a job of creating the entities
as it adds the payment before the line items. I switched
this to create a pending payment which doesn't alter the
thing under test & brings it closer to the
recommended flow
- one test is deliberately invalid - I marked it as
not eligible for the validation
- the price set id was not being passed to the Confirm->submit
function (accessed by tests, mostly via the ContributionPage.submit
api) - I added functionality to retrieve it

CRM/Contribute/Form/Contribution/Confirm.php

@@ -2040,7 +2040,9 @@ public function setFormAmountFields($priceSetID) {
    *
    * @param array $params
    *
-   * @throws CiviCRM_API3_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
   public static function submit($params) {
     $form = new CRM_Contribute_Form_Contribution_Confirm();
@@ -2060,6 +2062,7 @@ public static function submit($params) {
     $paramsProcessedForForm = $form->_params = self::getFormParams($params['id'], $params);
 
     $order = new CRM_Financial_BAO_Order();
+    $order->setPriceSetIDByContributionPageID($params['id']);
     $order->setPriceSelectionFromUnfilteredInput($params);
     if (isset($params['amount'])) {
       // @todo deprecate receiving amount, calculate on the form.
@@ -2111,11 +2114,10 @@ public static function submit($params) {
       $form->_useForMember = 1;
     }
     $priceFields = $priceFields[$priceSetID]['fields'];
-    $lineItems = [];
-    $form->processAmountAndGetAutoRenew($priceFields, $paramsProcessedForForm, $lineItems, $priceSetID);
-    $form->_lineItem = [$priceSetID => $lineItems];
+
+    $form->_lineItem = [$priceSetID => $order->getLineItems()];
     $membershipPriceFieldIDs = [];
-    foreach ((array) $lineItems as $lineItem) {
+    foreach ($order->getLineItems() as $lineItem) {
       if (!empty($lineItem['membership_type_id'])) {
         $form->set('useForMember', 1);
         $form->_useForMember = 1;

CRM/Financial/BAO/Order.php

@@ -199,6 +199,38 @@ public function setPriceSetID(int $priceSetID) {
     $this->priceSetID = $priceSetID;
   }
 
+  /**
+   * Set price set ID based on the contribution page id.
+   *
+   * @param int $contributionPageID
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function setPriceSetIDByContributionPageID(int $contributionPageID): void {
+    $this->setPriceSetIDByEntity('contribution_page', $contributionPageID);
+  }
+
+  /**
+   * Set price set ID based on the event id.
+   *
+   * @param int $eventID
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function setPriceSetIDByEventPageID(int $eventID): void {
+    $this->setPriceSetIDByEntity('event', $eventID);
+  }
+
+  /**
+   * Set the price set id based on looking up the entity.
+   * @param string $entity
+   * @param int $id
+   *
+   */
+  protected function setPriceSetIDByEntity(string $entity, int $id): void {
+    $this->priceSetID = CRM_Price_BAO_PriceSet::getFor('civicrm_' . $entity, $id);
+  }
+
   /**
    * Getter for price selection.
    *

tests/phpunit/api/v3/ContributionPageTest.php

@@ -249,7 +249,7 @@ public function testSubmitNewBillingNameData() {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testSubmitNewBillingNameDoNotOverwrite() {
+  public function testSubmitNewBillingNameDoNotOverwrite(): void {
     $this->setUpContributionPage();
     $contact = $this->callAPISuccess('Contact', 'create', [
       'contact_type' => 'Individual',
@@ -719,7 +719,7 @@ public function testSubmitMembershipBlockTwoTypesIsSeparatePayment() {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNow() {
+  public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNow(): void {
     // Need to work on valid financials on this test.
     $this->isValidateFinancialsOnPostAssert = FALSE;
     $mut = new CiviMailUtils($this, TRUE);
@@ -742,22 +742,22 @@ public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNow()
       'cvv2' => 123,
     ];
 
-    $this->callAPIAndDocument('contribution_page', 'submit', $submitParams, __FUNCTION__, __FILE__, 'submit contribution page', NULL);
+    $this->callAPIAndDocument('ContributionPage', 'submit', $submitParams, __FUNCTION__, __FILE__, 'submit contribution page', NULL);
     $contributions = $this->callAPISuccess('contribution', 'get', [
       'contribution_page_id' => $this->_ids['contribution_page'],
       'contribution_status_id' => 1,
     ]);
     $this->assertCount(2, $contributions['values']);
     $membershipPayment = $this->callAPISuccess('membership_payment', 'getsingle', []);
-    $this->assertTrue(in_array($membershipPayment['contribution_id'], array_keys($contributions['values'])));
+    $this->assertArrayHasKey($membershipPayment['contribution_id'], $contributions['values']);
     $membership = $this->callAPISuccessGetSingle('membership', ['id' => $membershipPayment['membership_id']]);
     $this->assertEquals($membership['contact_id'], $contributions['values'][$membershipPayment['contribution_id']]['contact_id']);
     $lineItem = $this->callAPISuccessGetSingle('LineItem', ['entity_table' => 'civicrm_membership']);
-    $this->assertEquals($lineItem['entity_id'], $membership['id']);
-    $this->assertEquals($lineItem['contribution_id'], $membershipPayment['contribution_id']);
-    $this->assertEquals($lineItem['qty'], 1);
-    $this->assertEquals($lineItem['unit_price'], 2);
-    $this->assertEquals($lineItem['line_total'], 2);
+    $this->assertEquals($membership['id'], $lineItem['entity_id']);
+    $this->assertEquals($membershipPayment['contribution_id'], $lineItem['contribution_id']);
+    $this->assertEquals(1, $lineItem['qty']);
+    $this->assertEquals(2, $lineItem['unit_price']);
+    $this->assertEquals(2, $lineItem['line_total']);
     foreach ($contributions['values'] as $contribution) {
       $this->assertEquals(.72, $contribution['fee_amount']);
       $this->assertEquals($contribution['total_amount'] - .72, $contribution['net_amount']);
@@ -783,12 +783,11 @@ public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNow()
    *
    * @dataProvider getThousandSeparators
    */
-  public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNowChargesCorrectAmounts($thousandSeparator) {
+  public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNowChargesCorrectAmounts($thousandSeparator): void {
     $this->setCurrencySeparators($thousandSeparator);
     $this->setUpMembershipContributionPage(TRUE);
     $processor = Civi\Payment\System::singleton()->getById($this->_paymentProcessor['id']);
     $processor->setDoDirectPaymentResult(['fee_amount' => .72]);
-    $test_uniq = uniqid();
     $contributionPageAmount = 10;
     $submitParams = [
       'price_' . $this->_ids['price_field'][0] => reset($this->_ids['price_field_value']),
@@ -804,29 +803,29 @@ public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNowCha
       'credit_card_type' => 'Visa',
       'credit_card_exp_date' => ['M' => 9, 'Y' => 2040],
       'cvv2' => 123,
-      'TEST_UNIQ' => $test_uniq,
+      'TEST_UNIQ' => 'unique id',
     ];
 
     // set custom hook
     $this->hookClass->setHook('civicrm_alterPaymentProcessorParams', [$this, 'hook_civicrm_alterPaymentProcessorParams']);
 
     $this->callAPISuccess('ContributionPage', 'submit', $submitParams);
-    $this->callAPISuccess('contribution', 'get', [
+    $this->callAPISuccess('Contribution', 'get', [
       'contribution_page_id' => $this->_ids['contribution_page'],
       'contribution_status_id' => 1,
     ]);
 
     $result = civicrm_api3('SystemLog', 'get', [
       'sequential' => 1,
-      'message' => ['LIKE' => "%{$test_uniq}%"],
+      'message' => ['LIKE' => '%unique id%'],
     ]);
-    $this->assertCount(2, $result['values'], "Expected exactly 2 log entries matching {$test_uniq}.");
+    $this->assertCount(2, $result['values'], 'Expected exactly 2 log entries matching unique id.');
 
     // Examine logged entries to ensure correct values.
     $contribution_ids = [];
     $found_membership_amount = $found_contribution_amount = FALSE;
     foreach ($result['values'] as $value) {
-      [$junk, $json] = explode("$test_uniq:", $value['message']);
+      [$junk, $json] = explode('unique id:', $value['message']);
       $logged_contribution = json_decode($json, TRUE);
       $contribution_ids[] = $logged_contribution['contributionID'];
       if (!empty($logged_contribution['total_amount'])) {
@@ -845,7 +844,7 @@ public function testSubmitMembershipBlockIsSeparatePaymentPaymentProcessorNowCha
     }
 
     $distinct_contribution_ids = array_unique($contribution_ids);
-    $this->assertCount(2, $distinct_contribution_ids, "Expected exactly 2 log contributions with distinct contributionIDs.");
+    $this->assertCount(2, $distinct_contribution_ids, 'Expected exactly 2 log contributions with distinct contributionIDs.');
     $this->assertTrue($found_contribution_amount, "Expected one log contribution with amount '$contributionPageAmount' (the contribution page amount)");
     $this->assertTrue($found_membership_amount, "Expected one log contribution with amount '$this->_membershipBlockAmount' (the membership amount)");
   }
@@ -1434,7 +1433,7 @@ public function testSubmitMembershipIsSeparatePaymentNotRecur() {
    *
    * @throws \CRM_Core_Exception
    */
-  public function setUpMembershipContributionPage($isSeparatePayment = FALSE, $isRecur = FALSE, $membershipTypeParams = []) {
+  public function setUpMembershipContributionPage($isSeparatePayment = FALSE, $isRecur = FALSE, $membershipTypeParams = []): void {
     $this->setUpMembershipBlockPriceSet($membershipTypeParams);
     $this->setupPaymentProcessor();
     $this->setUpContributionPage($isRecur);

tests/phpunit/api/v3/ContributionTest.php

@@ -274,7 +274,6 @@ public function testGetContributionReturnFunctionality(): void {
     $params['amount_level'] = 'Unreasonable';
     $params['cancel_reason'] = 'You lose sucker';
     $params['creditnote_id'] = 'sudo rm -rf';
-    $params['tax_amount'] = '1';
     $address = $this->callAPISuccess('Address', 'create', [
       'street_address' => 'Knockturn Alley',
       'contact_id' => $this->_individualId,
@@ -308,6 +307,9 @@ public function testGetContributionReturnFunctionality(): void {
     $this->assertEquals('bouncer', $contribution['contribution_check_number']);
 
     $fields = CRM_Contribute_BAO_Contribution::fields();
+    // Do not check for tax_amount as this test has not enabled invoicing
+    // & hence it is not reliable.
+    unset($fields['tax_amount']);
     // Re-add these 2 to the fields to check. They were locked in but the metadata changed so we
     // need to specify them.
     $fields['address_id'] = $fields['contribution_address_id'];
@@ -319,12 +321,12 @@ public function testGetContributionReturnFunctionality(): void {
       'fee_amount', 'net_amount', 'trxn_id', 'invoice_id', 'currency', 'contribution_cancel_date', 'cancel_reason',
       'receipt_date', 'thankyou_date', 'contribution_source', 'amount_level', 'contribution_recur_id',
       'is_test', 'is_pay_later', 'contribution_status_id', 'address_id', 'check_number', 'contribution_campaign_id',
-      'creditnote_id', 'tax_amount', 'revenue_recognition_date', 'decoy',
+      'creditnote_id', 'revenue_recognition_date', 'decoy',
     ];
     $missingFields = array_diff($fieldsLockedIn, array_keys($fields));
     // If any of the locked in fields disappear from the $fields array we need to make sure it is still
     // covered as the test contract now guarantees them in the return array.
-    $this->assertEquals($missingFields, [29 => 'decoy'], 'A field which was covered by the test contract has changed.');
+    $this->assertEquals([28 => 'decoy'], $missingFields, 'A field which was covered by the test contract has changed.');
     foreach ($fields as $fieldName => $fieldSpec) {
       $contribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID, 'return' => $fieldName]);
       $returnField = $fieldName;
@@ -510,8 +512,15 @@ public function testCreateGetFieldsWithCustom(): void {
     $this->customGroupDelete($idsContact['custom_group_id']);
   }
 
-  public function testCreateContributionNoLineItems() {
-
+  /**
+   * Test creating a contribution without skipLineItems.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testCreateContributionNoLineItems(): void {
+    // Turn off this validation as this test results in invalid
+    // financial entities.
+    $this->isValidateFinancialsOnPostAssert = FALSE;
     $params = [
       'contact_id' => $this->_individualId,
       'receive_date' => '20120511',
@@ -559,7 +568,7 @@ public function testCreateContributionChainedLineItems(): void {
       'trxn_id' => 12345,
       'invoice_id' => 67890,
       'source' => 'SSF',
-      'contribution_status_id' => 1,
+      'contribution_status_id' => 'Pending',
       'skipLineItem' => 1,
       'api.line_item.create' => [
         [