Add order fix

civicrm · Aug 25, 2021 · 68a5e5f · 68a5e5f
1 parent 0fc2232
commit 68a5e5f
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 15 deletions.
diff --git a/api/v3/Order.php b/api/v3/Order.php
@@ -100,13 +100,8 @@ function civicrm_api3_order_create(array $params): array {
   }
   else {
     $order->setPriceSetToDefault('contribution');
-    $order->setLineItem([
-      // Historically total_amount in this case could be tax
-      // inclusive if tax is also supplied.
-      // This is inconsistent with the contribution api....
-      'line_total' => ((float) $params['total_amount'] - (float) ($params['tax_amount'] ?? 0)),
-      'financial_type_id' => (int) $params['financial_type_id'],
-    ], 0);
+    $order->setOverrideTotalAmount((float) $params['total_amount']);
+    $order->setLineItem([], 0);
   }
   // Only check the amount if line items are set because that is what we have historically
   // done and total amount is historically only inclusive of tax_amount IF
@@ -118,6 +113,11 @@ function civicrm_api3_order_create(array $params): array {
     }
   }
   $params['total_amount'] = $order->getTotalAmount();
+  if (!isset($params['tax_amount'])) {
+    // @todo always calculate tax amount - left for now
+    // for webform
+    $params['tax_amount'] = $order->getTotalTaxAmount();
+  }
 
   foreach ($order->getEntitiesToCreate() as $entityParams) {
     if ($entityParams['entity'] === 'participant') {

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
@@ -2266,8 +2266,9 @@ public function testCompleteTransactionFeeAmount() {
    * @param string $thousandSeparator
    *   punctuation used to refer to thousands.
    *
-   * @dataProvider getThousandSeparators
+   * @throws \API_Exception
    * @throws \CRM_Core_Exception
+   * @dataProvider getThousandSeparators
    */
   public function testCheckTaxAmount(string $thousandSeparator): void {
     $this->setCurrencySeparators($thousandSeparator);
@@ -2286,10 +2287,10 @@ public function testCheckTaxAmount(string $thousandSeparator): void {
       'sequential' => 1,
     ]);
     $contributionPostPayment = $this->callAPISuccessGetSingle('Contribution', ['id' => $contributionID, 'return' => ['tax_amount', 'fee_amount', 'net_amount']]);
-    $this->assertEquals(5, $contributionPrePayment['tax_amount']);
-    $this->assertEquals(5, $contributionPostPayment['tax_amount']);
+    $this->assertEquals(4.76, $contributionPrePayment['tax_amount']);
+    $this->assertEquals(4.76, $contributionPostPayment['tax_amount']);
     $this->assertEquals('6.00', $contributionPostPayment['fee_amount']);
-    $this->assertEquals('99.00', $contributionPostPayment['net_amount']);
+    $this->assertEquals('94.00', $contributionPostPayment['net_amount']);
     $this->validateAllContributions();
     $this->validateAllPayments();
   }
@@ -4768,6 +4769,7 @@ public function testContributionGetUnique() {
 
   /**
    * Test Repeat Transaction Contribution with Tax amount.
+   *
    * https://lab.civicrm.org/dev/core/issues/806
    *
    * @throws \CRM_Core_Exception
@@ -4792,16 +4794,17 @@ public function testRepeatContributionWithTaxAmount(): void {
       'contribution_status_id' => 'Completed',
       'trxn_id' => 'test',
     ]);
-    $payments = $this->callAPISuccess('Contribution', 'get', ['sequential' => 1])['values'];
+    $payments = $this->callAPISuccess('Contribution', 'get', ['sequential' => 1, 'return' => ['total_amount', 'tax_amount']])['values'];
     //Assert if first payment and repeated payment has the same contribution amount.
     $this->assertEquals($payments[0]['total_amount'], $payments[1]['total_amount']);
     $this->callAPISuccessGetCount('Contribution', [], 2);
 
     //Assert line item records.
     $lineItems = $this->callAPISuccess('LineItem', 'get', ['sequential' => 1])['values'];
     foreach ($lineItems as $lineItem) {
-      $this->assertEquals($lineItem['unit_price'], $this->_params['total_amount']);
-      $this->assertEquals($lineItem['line_total'], $this->_params['total_amount']);
+      $taxExclusiveAmount = $payments[0]['total_amount'] - $payments[0]['tax_amount'];
+      $this->assertEquals($lineItem['unit_price'], $taxExclusiveAmount);
+      $this->assertEquals($lineItem['line_total'], $taxExclusiveAmount);
     }
     $this->callAPISuccessGetCount('Contribution', [], 2);
   }

diff --git a/tests/phpunit/api/v3/OrderTest.php b/tests/phpunit/api/v3/OrderTest.php
@@ -23,6 +23,16 @@ class api_v3_OrderTest extends CiviUnitTestCase {
 
   use CRMTraits_Financial_TaxTrait;
 
+  /**
+   * 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;
+
   protected $_individualId;
 
   protected $_financialTypeId = 1;
@@ -488,6 +498,63 @@ public function testDeleteOrder(): void {
     }
   }
 
+  /**
+   * Test order api treats amount as inclusive when line items not set.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testAPIOrderTaxSpecified(): void {
+    $this->enableTaxAndInvoicing();
+    $this->createFinancialTypeWithSalesTax();
+    $order = $this->callAPISuccess('Order', 'create', [
+      'total_amount' => 105,
+      'financial_type_id' => 'Test taxable financial Type',
+      'contact_id' => $this->individualCreate(),
+      'sequential' => 1,
+      'tax_amount' => 5,
+    ])['values'][0];
+    $this->assertEquals(105, $order['total_amount']);
+    $this->assertEquals(5, $order['tax_amount']);
+  }
+
+  /**
+   * Test order api treats amount as inclusive when line items not set.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testAPIOrderTaxNotSpecified(): void {
+    $this->enableTaxAndInvoicing();
+    $this->createFinancialTypeWithSalesTax();
+    $order = $this->callAPISuccess('Order', 'create', [
+      'total_amount' => 105,
+      'financial_type_id' => 'Test taxable financial Type',
+      'contact_id' => $this->individualCreate(),
+      'sequential' => 1,
+    ])['values'][0];
+    $this->assertEquals(105, $order['total_amount']);
+    $this->assertEquals(5, $order['tax_amount']);
+  }
+
+  /**
+   * Test order api treats amount as inclusive when line items not set.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testAPIContributionTaxSpecified(): void {
+    $this->enableTaxAndInvoicing();
+    $this->createFinancialTypeWithSalesTax();
+
+    $order = $this->callAPISuccess('Contribution', 'create', [
+      'total_amount' => 105,
+      'financial_type_id' => 'Test taxable financial Type',
+      'contact_id' => $this->individualCreate(),
+      'sequential' => 1,
+      'tax_amount' => 5,
+    ])['values'][0];
+    $this->assertEquals(105, $order['total_amount']);
+    $this->assertEquals(5, $order['tax_amount']);
+  }
+
   /**
    * Test cancel order api
    *
@@ -652,7 +719,6 @@ public function testOrderWithMixedTax(): void {
       'invoice_id' => '1859_woocommerce',
       'receive_date' => '2021-05-05 23:24:02',
       'contribution_status_id' => 'Pending',
-      'total_amount' => 109.69,
       'source' => 'Shop',
       'note' => 'Fundraiser Dinner Ticket x 1, Student Membership x 1',
       'line_items' => [