Skip to content

Commit

Permalink
CRM-21804 improve error handling on duplicate contribution.
Browse files Browse the repository at this point in the history
Duplicate contributions are a really not handled well anywhere in the form layer.
The CRM_Core_Error winds up using the fatal template, which has the same end
result for form users as if an exception were thrown, let's switch.

Note that this appears to be the only scenario when an error would
be returned so we can simply drop that handling
  • Loading branch information
eileenmcnaughton committed Feb 26, 2018
1 parent bc17b73 commit 77beddb
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 30 deletions.
22 changes: 9 additions & 13 deletions CRM/Contribute/BAO/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public function __construct() {
* @param array $ids
* The array that holds all the db ids.
*
* @return CRM_Contribute_BAO_Contribution|\CRM_Core_Error
* @return \CRM_Contribute_BAO_Contribution
* @throws \CRM_Core_Exception
*/
public static function add(&$params, $ids = array()) {
if (empty($params)) {
Expand All @@ -107,14 +108,8 @@ public static function add(&$params, $ids = array()) {
$contributionID = CRM_Utils_Array::value('contribution', $ids, CRM_Utils_Array::value('id', $params));
$duplicates = array();
if (self::checkDuplicate($params, $duplicates, $contributionID)) {
$error = CRM_Core_Error::singleton();
$d = implode(', ', $duplicates);
$error->push(CRM_Core_Error::DUPLICATE_CONTRIBUTION,
'Fatal',
array($d),
"Duplicate error - existing contribution record(s) have a matching Transaction ID or Invoice ID. Contribution record ID(s) are: $d"
);
return $error;
$message = ts("Duplicate error - existing contribution record(s) have a matching Transaction ID or Invoice ID. Contribution record ID(s) are: " . implode(', ', $duplicates));
throw new CRM_Core_Exception($message);
}

// first clean up all the money fields
Expand Down Expand Up @@ -515,11 +510,12 @@ public static function create(&$params, $ids = array()) {

$transaction = new CRM_Core_Transaction();

$contribution = self::add($params, $ids);

if (is_a($contribution, 'CRM_Core_Error')) {
try {
$contribution = self::add($params, $ids);
}
catch (CRM_Core_Exception $e) {
$transaction->rollback();
return $contribution;
throw $e;
}

$params['contribution_id'] = $contribution->id;
Expand Down
37 changes: 24 additions & 13 deletions CRM/Contribute/Form/Contribution/Confirm.php
Original file line number Diff line number Diff line change
Expand Up @@ -716,14 +716,15 @@ public function setDefaultValues() {
*/
public function postProcess() {
$contactID = $this->getContactID();
$result = $this->processFormSubmission($contactID);
try {
$result = $this->processFormSubmission($contactID);
}
catch (CRM_Core_Exception $e) {
$this->bounceOnError($e->getMessage());
}

if (is_array($result) && !empty($result['is_payment_failure'])) {
// We will probably have the function that gets this error throw an exception on the next round of refactoring.
CRM_Core_Session::singleton()->setStatus(ts("Payment Processor Error message :") .
$result['error']->getMessage());
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact',
"_qf_Main_display=true&qfKey={$this->_params['qfKey']}"
));
$this->bounceOnError($result['error']->getMessage());
}
// Presumably this is for hooks to access? Not quite clear & perhaps not required.
$this->set('params', $this->_params);
Expand Down Expand Up @@ -966,6 +967,7 @@ public static function processFormContribution(
// @todo this is the wrong place for this - it should be done as close to form submission
// as possible
$contributionParams['total_amount'] = $params['amount'];

$contribution = CRM_Contribute_BAO_Contribution::add($contributionParams);

$invoiceSettings = Civi::settings()->get('contribution_invoice_settings');
Expand All @@ -990,10 +992,6 @@ public static function processFormContribution(
$smarty->assign('dataArray', $dataArray);
$smarty->assign('totalTaxAmount', $params['tax_amount']);
}
if (is_a($contribution, 'CRM_Core_Error')) {
$message = CRM_Core_Error::getMessages($contribution);
CRM_Core_Error::fatal($message);
}

// lets store it in the form variable so postProcess hook can get to this and use it
$form->_contributionID = $contribution->id;
Expand All @@ -1020,8 +1018,7 @@ public static function processFormContribution(
//handle custom data.
$params['contribution_id'] = $contribution->id;
if (!empty($params['custom']) &&
is_array($params['custom']) &&
!is_a($contribution, 'CRM_Core_Error')
is_array($params['custom'])
) {
CRM_Core_BAO_CustomValueTable::store($params['custom'], 'civicrm_contribution', $contribution->id);
}
Expand Down Expand Up @@ -2498,4 +2495,18 @@ protected function completeTransaction($result, $contributionID) {
}
}

/**
* Bounce the user back to retry when an error occurs.
*
* @param string $message
*/
protected function bounceOnError($message) {
CRM_Core_Session::singleton()
->setStatus(ts("Payment Processor Error message :") .
$message);
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contribute/transact',
"_qf_Main_display=true&qfKey={$this->_params['qfKey']}"
));
}

}
5 changes: 1 addition & 4 deletions CRM/Event/Cart/Form/Checkout/Payment.php
Original file line number Diff line number Diff line change
Expand Up @@ -670,10 +670,7 @@ public function record_contribution(&$mer_participant, &$params, $event) {
$contribParams['payment_processor'] = $this->_paymentProcessor['id'];
}

$contribution = &CRM_Contribute_BAO_Contribution::add($contribParams);
if (is_a($contribution, 'CRM_Core_Error')) {
CRM_Core_Error::fatal(ts("There was an error creating a contribution record for your event. Please report this error to the webmaster. Details: %1", array(1 => $contribution->getMessages($contribution))));
}
$contribution = CRM_Contribute_BAO_Contribution::add($contribParams);
$mer_participant->contribution_id = $contribution->id;
$params['contributionID'] = $contribution->id;

Expand Down

0 comments on commit 77beddb

Please sign in to comment.