Skip to content

Commit

Permalink
Tax fixes in unit test
Browse files Browse the repository at this point in the history
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
  • Loading branch information
eileenmcnaughton committed May 24, 2021
1 parent a0203d5 commit f3065d1
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 8 deletions.
7 changes: 5 additions & 2 deletions CRM/Contribute/Form/Contribution/Confirm.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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.
Expand Down Expand Up @@ -2113,7 +2116,7 @@ public static function submit($params) {
$priceFields = $priceFields[$priceSetID]['fields'];
$lineItems = [];
$form->processAmountAndGetAutoRenew($priceFields, $paramsProcessedForForm, $lineItems, $priceSetID);
$form->_lineItem = [$priceSetID => $lineItems];
$form->_lineItem = $order->getLineItems();
$membershipPriceFieldIDs = [];
foreach ((array) $lineItems as $lineItem) {
if (!empty($lineItem['membership_type_id'])) {
Expand Down
32 changes: 32 additions & 0 deletions CRM/Financial/BAO/Order.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
21 changes: 15 additions & 6 deletions tests/phpunit/api/v3/ContributionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'];
Expand All @@ -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;
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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' => [
[
Expand Down

0 comments on commit f3065d1

Please sign in to comment.