Merge pull request #18838 from eileenmcnaughton/vrenew

dev/core#2024 extra line item issue on membership renewal

diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php
index 87be03190375731a94b518845bf0c13753225a1d..6bcc82fe810bdf1dbfc2aa65de1dea058f4e2f97 100644 (file)
--- a/CRM/Member/Form/MembershipRenewal.php
+++ b/CRM/Member/Form/MembershipRenewal.php
@@ -625,11 +625,6 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
         'membership_id' => $membership->id,
         'contribution_recur_id' => $contributionRecurID,
       ]);
-      //Remove `tax_amount` if it is not calculated.
-      // ?? WHY - I haven't been able to figure out...
-      if (CRM_Utils_Array::value('tax_amount', $temporaryParams) === 0.0) {
-        unset($temporaryParams['tax_amount']);
-      }
       CRM_Member_BAO_Membership::recordMembershipContribution($temporaryParams);
     }
 

diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
index b43e4cbf74b9a5bd7cf543e755328ab202abd207..20de47222c1c99d95931032a16b612ada0e36ba9 100644 (file)
--- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
@@ -1142,10 +1142,10 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
   public function testgetSalesTaxFinancialAccounts() {
     $this->enableTaxAndInvoicing();
     $financialType = $this->createFinancialType();
-    $financialAccount = $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);
+    $financialAccount = $this->addTaxAccountToFinancialType($financialType['id']);
     $expectedResult = [$financialAccount->financial_account_id => $financialAccount->financial_account_id];
     $financialType = $this->createFinancialType();
-    $financialAccount = $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);
+    $financialAccount = $this->addTaxAccountToFinancialType($financialType['id']);
     $expectedResult[$financialAccount->financial_account_id] = $financialAccount->financial_account_id;
     $salesTaxFinancialAccount = CRM_Contribute_BAO_Contribution::getSalesTaxFinancialAccounts();
     $this->assertTrue(($salesTaxFinancialAccount == $expectedResult), 'Function returned wrong values.');
@@ -1306,7 +1306,7 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2";
     $contactId = $this->individualCreate();
     $this->enableTaxAndInvoicing();
     $financialType = $this->createFinancialType();
-    $financialAccount = $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);
+    $financialAccount = $this->addTaxAccountToFinancialType($financialType['id']);
     $form = new CRM_Contribute_Form_Contribution();
 
     $form->testSubmit([

diff --git a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
index cc10a426e825a9aabe82de7b017bfdc3e8c26e8b..37ba9509563cc6f58e0c826bd0550795a7921edd 100644 (file)
--- a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php
@@ -1073,7 +1073,7 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
   public function testSubmitSaleTax($thousandSeparator) {
     $this->setCurrencySeparators($thousandSeparator);
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount($this->_financialTypeId);
+    $this->addTaxAccountToFinancialType($this->_financialTypeId);
     $form = new CRM_Contribute_Form_Contribution();
 
     $form->testSubmit([
@@ -1123,7 +1123,7 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
    */
   public function testSubmitWithOutSaleTax() {
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount($this->_financialTypeId);
+    $this->addTaxAccountToFinancialType($this->_financialTypeId);
     $form = new CRM_Contribute_Form_Contribution();
 
     $form->testSubmit([
@@ -1164,8 +1164,8 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
   public function testReSubmitSaleTax($thousandSeparator) {
     $this->setCurrencySeparators($thousandSeparator);
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount($this->_financialTypeId);
-    list($form, $contribution) = $this->doInitialSubmit();
+    $this->addTaxAccountToFinancialType($this->_financialTypeId);
+    [$form, $contribution] = $this->doInitialSubmit();
     $this->assertEquals(11000, $contribution['total_amount']);
     $this->assertEquals(1000, $contribution['tax_amount']);
     $this->assertEquals(11000, $contribution['net_amount']);
@@ -1225,8 +1225,8 @@ Price Field - Price Field 1        1   $ 100.00      $ 100.00
   public function testReSubmitSaleTaxAlteredAmount($thousandSeparator) {
     $this->setCurrencySeparators($thousandSeparator);
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount($this->_financialTypeId);
-    list($form, $contribution) = $this->doInitialSubmit();
+    $this->addTaxAccountToFinancialType($this->_financialTypeId);
+    [$form, $contribution] = $this->doInitialSubmit();
 
     $mut = new CiviMailUtils($this, TRUE);
     // Testing here if when we edit something trivial like adding a check_number tax, net, total amount stay the same:

diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialItemTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialItemTest.php
index eab21910ccaf7b59275d851cab93a69b0017588b..7be8734bbbcd84237cfcfbdb4a6f7f56445b59f4 100644 (file)
--- a/tests/phpunit/CRM/Financial/BAO/FinancialItemTest.php
+++ b/tests/phpunit/CRM/Financial/BAO/FinancialItemTest.php
@@ -299,7 +299,7 @@ class CRM_Financial_BAO_FinancialItemTest extends CiviUnitTestCase {
     $this->setCurrencySeparators($thousandSeparator);
     $contactId = $this->individualCreate();
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount(1);
+    $this->addTaxAccountToFinancialType(1);
     $form = new CRM_Contribute_Form_Contribution();
     $form->testSubmit([
       'total_amount' => 100,

diff --git a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
index b70bfc665f9d2652cf5b214cc23dd3b3766fbc57..44206a281db27ce9c0c5e18147a90c4167829eda 100644 (file)
--- a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
@@ -10,6 +10,7 @@
  */
 
 use Civi\Api4\Contact;
+use Civi\Api4\LineItem;
 
 /**
  *  Test CRM_Member_Form_Membership functions.
@@ -183,7 +184,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    */
   public function testSubmitWithTax() {
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount($this->financialTypeID);
+    $this->addTaxAccountToFinancialType($this->financialTypeID);
     $form = $this->getForm();
     $form->testSubmit(array_merge($this->getBaseSubmitParams(), [
       'total_amount' => '50.00',
@@ -193,6 +194,30 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $this->assertEquals(4.55, $contribution['tax_amount']);
   }
 
+  /**
+   * Test submitting with tax enabled but a rate of zero.
+   *
+   * https://lab.civicrm.org/dev/core/-/issues/2024
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  public function testSubmitWithTaxOfZero() {
+    $this->enableTaxAndInvoicing();
+    $this->addTaxAccountToFinancialType($this->financialTypeID, ['tax_rate' => 0]);
+    $form = $this->getForm();
+    $form->testSubmit(array_merge($this->getBaseSubmitParams(), [
+      'total_amount' => '50.00',
+    ]));
+    $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $this->_individualId, 'is_test' => TRUE, 'return' => ['total_amount', 'tax_amount']]);
+    $this->assertEquals(50, $contribution['total_amount']);
+    $this->assertEquals(0, $contribution['tax_amount']);
+    $lines = LineItem::get()->addWhere('contribution_id', '=', $contribution['id'])->execute();
+    $this->assertCount(1, $lines);
+  }
+
   /**
    * Test the submit function of the membership form.
    *
@@ -487,7 +512,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'return' => ['tax_amount', 'trxn_id'],
     ]);
     $this->assertEquals($contribution['trxn_id'], 777);
-    $this->assertEquals(NULL, $contribution['tax_amount']);
+    $this->assertEquals(0.00, $contribution['tax_amount']);
 
     $this->callAPISuccessGetCount('LineItem', [
       'entity_id' => $membership['id'],

diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index 3682925eb6c3a1b9246b313b7e0bf21eeb9bc5c5..fb6bc5d7bda6141a4d10df9ecec8a9311ef7a045 100644 (file)
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -1362,7 +1362,7 @@ Expires: ',
    */
   public function testLineItemAmountOnSalesTax() {
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount(2);
+    $this->addTaxAccountToFinancialType(2);
     $form = $this->getForm();
     $form->preProcess();
     $this->mut = new CiviMailUtils($this, TRUE);

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 8d540ed0554aeb5ab05fab91fd5c35ed23c5f689..fd3945006e99a9a52689f8facd8fcdc5e7e2df1b 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3091,21 +3091,24 @@ VALUES
   }
 
   /**
-   * Add Sales Tax relation for financial type with financial account.
+   * Add Sales Tax Account for the financial type.
    *
    * @param int $financialTypeId
    *
-   * @return obj
+   * @param array $accountParams
+   *
+   * @return CRM_Financial_DAO_EntityFinancialAccount
+   * @throws \CRM_Core_Exception
    */
-  protected function relationForFinancialTypeWithFinancialAccount($financialTypeId) {
-    $params = [
+  protected function addTaxAccountToFinancialType(int $financialTypeId, $accountParams = []) {
+    $params = array_merge([
       'name' => 'Sales tax account ' . substr(sha1(rand()), 0, 4),
       'financial_account_type_id' => key(CRM_Core_PseudoConstant::accountOptionValues('financial_account_type', NULL, " AND v.name LIKE 'Liability' ")),
       'is_deductible' => 1,
       'is_tax' => 1,
       'tax_rate' => 10,
       'is_active' => 1,
-    ];
+    ], $accountParams);
     $account = CRM_Financial_BAO_FinancialAccount::add($params);
     $entityParams = [
       'entity_table' => 'civicrm_financial_type',
@@ -3114,7 +3117,7 @@ VALUES
     ];
 
     // set tax rate (as 10) for provided financial type ID to static variable, later used to fetch tax rates of all financial types
-    \Civi::$statics['CRM_Core_PseudoConstant']['taxRates'][$financialTypeId] = 10;
+    \Civi::$statics['CRM_Core_PseudoConstant']['taxRates'][$financialTypeId] = $params['tax_rate'];
 
     //CRM-20313: As per unique index added in civicrm_entity_financial_account table,
     //  first check if there's any record on basis of unique key (entity_table, account_relationship, entity_id)

diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index bf81029ff79efd22f5c8b0de0a3eeb2dc93f6c73..4ffb1fa0272d9a32152a483ce349c6e39842858f 100644 (file)
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -1964,7 +1964,7 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
     $financialType = $this->createFinancialType();
     $financialTypeId = $financialType['id'];
     // This function sets the Tax Rate at 10% - it currently has no way to pass Tax Rate into it - so let's work with 10%
-    $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);
+    $this->addTaxAccountToFinancialType($financialType['id']);
 
     $this->setUpContributionPage();
     $submitParams = [

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 3a434ecb71c59e626894c45c70ee23f98cc2c71d..f566f868059774dae8d889b4172a341dc90cc1fd 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -4725,7 +4725,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
       'is_reserved' => 0,
       'is_active' => 1,
     ]);
-    $this->relationForFinancialTypeWithFinancialAccount($financialType['id']);
+    $this->addTaxAccountToFinancialType($financialType['id']);
     $contribution = $this->setUpRepeatTransaction(
       [],
       'single',

diff --git a/tests/phpunit/api/v3/LineItemTest.php b/tests/phpunit/api/v3/LineItemTest.php
index 48b44d3a6406e53c37e581e6b719ace7396b0e7f..29b83485a70a85ae1d16b7d8050a890e65c5be6c 100644 (file)
--- a/tests/phpunit/api/v3/LineItemTest.php
+++ b/tests/phpunit/api/v3/LineItemTest.php
@@ -78,7 +78,7 @@ class api_v3_LineItemTest extends CiviUnitTestCase {
    */
   public function enableSalesTaxOnFinancialType($type) {
     $this->enableTaxAndInvoicing();
-    $this->relationForFinancialTypeWithFinancialAccount(CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', $type));
+    $this->addTaxAccountToFinancialType(CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', $type));
   }
 
   /**