From 276e3ec6387dcaa0ef2fa8129854ab72b0c7ab9d Mon Sep 17 00:00:00 2001 From: eileenmcnaughton Date: Sun, 2 Aug 2015 22:36:02 +0000 Subject: [PATCH] CRM-16923 follow up test fixes --- CRM/Contribute/BAO/Contribution.php | 20 +------------------ CRM/Core/Payment/BaseIPN.php | 5 ++++- api/v3/Contribution.php | 6 +++--- .../phpunit/CRM/Core/Payment/BaseIPNTest.php | 20 ++++++++++++++----- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index c75a16b58f..a7459869eb 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -2057,15 +2057,13 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac * Input as delivered from Payment Processor. * @param array $ids * Ids as Loaded by Payment Processor. - * @param bool $required - * Is Payment processor / contribution page required. * @param bool $loadAll * Load all related objects - even where id not passed in? (allows API to call this). * * @return bool * @throws Exception */ - public function loadRelatedObjects(&$input, &$ids, $required = FALSE, $loadAll = FALSE) { + public function loadRelatedObjects(&$input, &$ids, $loadAll = FALSE) { if ($loadAll) { $ids = array_merge($this->getComponentDetails($this->id), $ids); if (empty($ids['contact']) && isset($this->contact_id)) { @@ -2178,18 +2176,6 @@ WHERE contribution_id = %1 "; } } } - - if (!$paymentProcessorID) { - //fail to load payment processor id. - // this seems a bit dubious.... - if (empty($ids['pledge_payment'])) { - $loadObjectSuccess = TRUE; - if ($required) { - throw new Exception("Could not find contribution page for contribution record: " . $this->id); - } - return $loadObjectSuccess; - } - } } else { // we are in event mode @@ -2232,10 +2218,6 @@ WHERE contribution_id = %1 "; $ids['paymentProcessor'] = $paymentProcessorID; $this->_relatedObjects['paymentProcessor'] = $paymentProcessor; } - elseif ($required) { - throw new Exception("Could not find payment processor for contribution record: " . $this->id); - } - return TRUE; } diff --git a/CRM/Core/Payment/BaseIPN.php b/CRM/Core/Payment/BaseIPN.php index 9192b867d6..bead2afeb2 100644 --- a/CRM/Core/Payment/BaseIPN.php +++ b/CRM/Core/Payment/BaseIPN.php @@ -158,7 +158,10 @@ class CRM_Core_Payment_BaseIPN { $objects['contribution'] = &$contribution; } try { - $success = $contribution->loadRelatedObjects($input, $ids, $required); + $success = $contribution->loadRelatedObjects($input, $ids); + if ($required && empty($contribution->_relatedObjects['paymentProcessor'])) { + throw new CRM_Core_Exception("Could not find payment processor for contribution record: " . $contribution->id); + } } catch (Exception $e) { $success = FALSE; diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php index 0cf23a94c8..fd59a8c0f6 100644 --- a/api/v3/Contribution.php +++ b/api/v3/Contribution.php @@ -351,7 +351,7 @@ function civicrm_api3_contribution_sendconfirmation($params) { throw new Exception('Contribution does not exist'); } $input = $ids = $cvalues = array('receipt_from_email' => $params['receipt_from_email']); - $contribution->loadRelatedObjects($input, $ids, FALSE, TRUE); + $contribution->loadRelatedObjects($input, $ids, TRUE); $contribution->composeMessageArray($input, $ids, $cvalues, FALSE, FALSE); } @@ -420,7 +420,7 @@ function civicrm_api3_contribution_completetransaction(&$params) { throw new API_Exception('A valid contribution ID is required', 'invalid_data'); } - if (!$contribution->loadRelatedObjects($input, $ids, FALSE, TRUE)) { + if (!$contribution->loadRelatedObjects($input, $ids, TRUE)) { throw new API_Exception('failed to load related objects'); } elseif ($contribution->contribution_status_id == CRM_Core_OptionGroup::getValue('contribution_status', 'Completed', 'name')) { @@ -493,7 +493,7 @@ function civicrm_api3_contribution_repeattransaction(&$params) { } $original_contribution = clone $contribution; try { - if (!$contribution->loadRelatedObjects($input, $ids, FALSE, TRUE)) { + if (!$contribution->loadRelatedObjects($input, $ids, TRUE)) { throw new API_Exception('failed to load related objects'); } diff --git a/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php b/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php index 48c8d03328..bfc6a0e673 100644 --- a/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/BaseIPNTest.php @@ -112,7 +112,7 @@ class CRM_Core_Payment_BaseIPNTest extends CiviUnitTestCase { $contribution = new CRM_Contribute_BAO_Contribution(); $contribution->id = $this->_contributionId; $contribution->find(TRUE); - $contribution->loadRelatedObjects($this->input, $this->ids, FALSE, TRUE); + $contribution->loadRelatedObjects($this->input, $this->ids, TRUE); $this->assertFalse(empty($contribution->_relatedObjects['membership'])); $this->assertArrayHasKey($this->_membershipTypeID, $contribution->_relatedObjects['membership']); $this->assertTrue(is_a($contribution->_relatedObjects['membership'][$this->_membershipTypeID], 'CRM_Member_BAO_Membership')); @@ -290,7 +290,6 @@ class CRM_Core_Payment_BaseIPNTest extends CiviUnitTestCase { */ public function testRequiredWithoutProcessorID() { $this->_setUpPledgeObjects(); - $values = array(); $result = $this->IPN->loadObjects($this->input, $this->ids, $this->objects, TRUE, NULL, array('return_error' => 1)); $this->assertArrayHasKey('error_message', $result); $this->assertEquals('Could not find payment processor for contribution record: 1', $result['error_message']); @@ -303,13 +302,24 @@ class CRM_Core_Payment_BaseIPNTest extends CiviUnitTestCase { } /** - * * Test that an error is not if required set & no processor ID */ public function testRequiredWithContributionPage() { $this->_setUpContributionObjects(TRUE); $result = $this->IPN->loadObjects($this->input, $this->ids, $this->objects, TRUE, NULL, array('return_error' => 1)); - $this->assertFalse(is_array($result), $result['error_message']); + $this->assertEquals(1, $result['is_error']); + ; + } + + /** + * Test that if part of $input the payment processor loads OK. + * + * It's preferable to pass it in as it cannot be correctly calculated. + */ + public function testPaymentProcessorLoadsAsParam() { + $this->_setUpContributionObjects(); + $this->input = array_merge($this->input, array('payment_processor_id' => $this->_processorId)); + $this->assertTrue($this->IPN->loadObjects($this->input, $this->ids, $this->objects, TRUE, NULL, array('return_error' => 1))); } /** @@ -319,7 +329,7 @@ class CRM_Core_Payment_BaseIPNTest extends CiviUnitTestCase { $this->_setUpContributionObjects(); $result = $this->IPN->loadObjects($this->input, $this->ids, $this->objects, TRUE, NULL, array('return_error' => 1)); $this->assertArrayHasKey('error_message', $result); - $this->assertEquals('Could not find contribution page for contribution record: 1', $result['error_message']); + $this->assertEquals('Could not find payment processor for contribution record: 1', $result['error_message']); // error is only returned if $required set to True $result = $this->IPN->loadObjects($this->input, $this->ids, $this->objects, FALSE, NULL, array('return_error' => 1)); $this->assertFalse(is_array($result)); -- 2.25.1