From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Mon, 24 May 2021 01:48:24 +0000 (+1200)
Subject: Tax fixes in unit test
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=d6c86ce5c22e76a79596a6af0b930a3b91c33537;p=civicrm-core.git

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
---

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index f592e20ffb..5ef25b5d68 100644
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -2040,7 +2040,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
    *
    * @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,8 +2062,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $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'])) {
+    if (isset($params['amount']) && !CRM_Contribute_BAO_ContributionPage::getIsMembershipPayment($form->_id)) {
       // @todo deprecate receiving amount, calculate on the form.
       $order->setOverrideTotalAmount($params['amount']);
     }
@@ -2111,11 +2114,10 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       $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;
diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php
index 4ef6de2f5b..fb6c3a7f13 100644
--- a/CRM/Financial/BAO/Order.php
+++ b/CRM/Financial/BAO/Order.php
@@ -199,6 +199,38 @@ class CRM_Financial_BAO_Order {
     $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.
    *
diff --git a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
index 16802a9a5a..9307ea432e 100644
--- a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
@@ -983,7 +983,7 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
     $financialTransactions = $this->callAPISuccess('FinancialTrxn', 'get', ['sequential' => TRUE]);
     $this->assertEquals(3, $financialTransactions['count']);
 
-    list($oldTrxn, $reversedTrxn, $latestTrxn) = $financialTransactions['values'];
+    [$oldTrxn, $reversedTrxn, $latestTrxn] = $financialTransactions['values'];
 
     $this->assertEquals(1200.55, $oldTrxn['total_amount']);
     $this->assertEquals('123AX', $oldTrxn['check_number']);
@@ -1459,61 +1459,65 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
   }
 
   /**
-   * CRM-21711 Test that custom fields on relevant memberships get updated wehn updating multiple memberships
+   * CRM-21711 Test that custom fields on relevant memberships get updated wehn
+   * updating multiple memberships
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public function testCustomFieldsOnMembershipGetUpdated() {
+  public function testCustomFieldsOnMembershipGetUpdated(): void {
     $contactID = $this->individualCreate();
     $contactID1 = $this->organizationCreate();
     $contactID2 = $this->organizationCreate();
 
     // create membership types
-    $membershipTypeOne = civicrm_api3('membership_type', 'create', [
+    $membershipTypeOne = civicrm_api3('MembershipType', 'create', [
       'domain_id' => 1,
-      'name' => "One",
+      'name' => 'One',
       'member_of_contact_id' => $contactID1,
-      'duration_unit' => "year",
+      'duration_unit' => 'year',
       'minimum_fee' => 50,
       'duration_interval' => 1,
-      'period_type' => "fixed",
-      'fixed_period_start_day' => "101",
-      'fixed_period_rollover_day' => "1231",
+      'period_type' => 'fixed',
+      'fixed_period_start_day' => '101',
+      'fixed_period_rollover_day' => '1231',
       'financial_type_id' => 1,
       'weight' => 50,
       'is_active' => 1,
-      'visibility' => "Public",
+      'visibility' => 'Public',
     ]);
 
-    $membershipTypeTwo = civicrm_api3('membership_type', 'create', [
+    $membershipTypeTwo = civicrm_api3('MembershipType', 'create', [
       'domain_id' => 1,
-      'name' => "Two",
+      'name' => 'Two',
       'member_of_contact_id' => $contactID2,
-      'duration_unit' => "year",
+      'duration_unit' => 'year',
       'minimum_fee' => 50,
       'duration_interval' => 1,
-      'period_type' => "fixed",
-      'fixed_period_start_day' => "101",
-      'fixed_period_rollover_day' => "1231",
+      'period_type' => 'fixed',
+      'fixed_period_start_day' => '101',
+      'fixed_period_rollover_day' => '1231',
       'financial_type_id' => 1,
       'weight' => 51,
       'is_active' => 1,
-      'visibility' => "Public",
+      'visibility' => 'Public',
     ]);
 
     //create custom Fields
     $membershipCustomFieldsGroup = civicrm_api3('CustomGroup', 'create', [
-      'title' => "Custom Fields on Membership",
-      'extends' => "Membership",
+      'title' => 'Custom Fields on Membership',
+      'extends' => 'Membership',
     ]);
 
     $membershipCustomField = civicrm_api3('CustomField', 'create', [
-      "custom_group_id" => $membershipCustomFieldsGroup['id'],
-      "name" => "my_membership_custom_field",
-      "label" => "Membership Custom Field",
-      "data_type" => "String",
-      "html_type" => "Text",
-      "is_active" => "1",
-      "is_view" => "0",
-      "text_length" => "255",
+      'custom_group_id' => $membershipCustomFieldsGroup['id'],
+      'name' => 'my_membership_custom_field',
+      'label' => 'Membership Custom Field',
+      'data_type' => 'String',
+      'html_type' => 'Text',
+      'is_active' => TRUE,
+      'text_length' => 255,
     ]);
 
     // create profile
@@ -1529,7 +1533,7 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
     ]);
 
     // add custom fields to profile
-    $membershipCustomFieldsProfileFields = civicrm_api3('UFField', 'create', [
+    civicrm_api3('UFField', 'create', [
       "uf_group_id" => $membershipCustomFieldsProfile['id'],
       "field_name" => "custom_" . $membershipCustomField['id'],
       "is_active" => "1",
@@ -1554,7 +1558,6 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
       "is_pay_later" => "1",
       "pay_later_text" => "I will send payment by check",
       "is_partial_payment" => "0",
-      "is_allow_other_amount" => "0",
       "is_email_receipt" => "0",
       "is_active" => "1",
       "amount_block_is_active" => "0",
@@ -1574,14 +1577,12 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
       'extends' => "CiviMember",
       'is_active' => 1,
       "financial_type_id" => "1",
-      "is_quick_config" => "0",
-      "is_reserved" => "0",
-      "entity" => ["civicrm_contribution_page" => [$contribPage1]],
     ]);
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_price_set_entity (entity_table, entity_id, price_set_id) VALUES('civicrm_contribution_page', $contribPage1, {$priceSet['id']})");
 
     $priceField = civicrm_api3('PriceField', 'create', [
-      "price_set_id" => $priceSet['id'],
-      "name" => "mt",
+      'price_set_id' => $priceSet['id'],
+      'name' => 'mt',
       "label" => "Membership Types",
       "html_type" => "CheckBox",
       "is_enter_qty" => "0",
@@ -1624,7 +1625,7 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
     ]);
 
     // assign profile with custom fields to contribution page
-    $profile = civicrm_api3('UFJoin', 'create', [
+    civicrm_api3('UFJoin', 'create', [
       'module' => "CiviContribute",
       'weight' => "1",
       'uf_group_id' => $membershipCustomFieldsProfile['id'],
@@ -1635,14 +1636,13 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
     $form = new CRM_Contribute_Form_Contribution_Confirm();
     $form->_params = [
       'id' => $contribPage1,
-      "qfKey" => "donotcare",
+      'qfKey' => "donotcare",
       "custom_{$membershipCustomField['id']}" => "Hello",
-      "email-5" => "admin@example.com",
       "priceSetId" => $priceSet['id'],
       'price_set_id' => $priceSet['id'],
       "price_" . $priceField['id'] => [$priceFieldOption1['id'] => 1, $priceFieldOption2['id'] => 1],
-      "invoiceID" => "9a6f7b49358dc31c3604e463b225c5be",
-      "email" => "admin@example.com",
+      'invoiceID' => "9a6f7b49358dc31c3604e463b225c5be",
+      'email' => "admin@example.com",
       "currencyID" => "USD",
       'description' => "Membership Contribution",
       'contact_id' => $contactID,
diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index 3217464543..112228b1e9 100644
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -249,7 +249,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
    *
    * @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 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
    * @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 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
       '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,7 +783,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
    *
    * @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']);
@@ -811,7 +811,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $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,
     ]);
@@ -1434,7 +1434,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
    *
    * @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);
diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 04f94744c9..1ffea21269 100644
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -274,7 +274,6 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $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 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $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 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
       '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 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
     $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 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
       'trxn_id' => 12345,
       'invoice_id' => 67890,
       'source' => 'SSF',
-      'contribution_status_id' => 1,
+      'contribution_status_id' => 'Pending',
       'skipLineItem' => 1,
       'api.line_item.create' => [
         [