Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch to calling Payment.create api when processing a refund from AdditionalPayment form #14317

Merged
merged 1 commit into from
Jun 1, 2019
Merged

Switch to calling Payment.create api when processing a refund from AdditionalPayment form #14317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 11 additions & 20 deletions CRM/Contribute/BAO/Contribution.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ public static function &exportableFields($checkPermission = TRUE) {
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
protected static function recordPaymentActivity($contributionId, $participantId, $totalAmount, $currency, $trxnDate) {
public static function recordPaymentActivity($contributionId, $participantId, $totalAmount, $currency, $trxnDate) {
$activityType = ($totalAmount < 0) ? 'Refund' : 'Payment';

if ($participantId) {
Expand Down Expand Up @@ -3974,34 +3974,25 @@ public static function validateFinancialType($financialTypeId, $relationName = '
* @param int $participantId
* @param bool $updateStatus
*
* @return null|object
* @return int
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
*/
public static function recordAdditionalPayment($contributionId, $trxnsData, $paymentType = 'owed', $participantId = NULL, $updateStatus = TRUE) {

if ($paymentType == 'owed') {
$financialTrxn = CRM_Financial_BAO_Payment::recordPayment($contributionId, $trxnsData, $participantId);
if (!empty($financialTrxn)) {
self::recordPaymentActivity($contributionId, $participantId, $financialTrxn->total_amount, $financialTrxn->currency, $financialTrxn->trxn_date);
return $financialTrxn->id;
}
}
elseif ($paymentType == 'refund') {
$trxnsData['total_amount'] = -$trxnsData['total_amount'];
$financialTrxn = CRM_Financial_BAO_Payment::recordRefundPayment($contributionId, $trxnsData, $updateStatus);
if ($participantId) {
// update participant status
// @todo this doesn't make sense...
$participantStatuses = CRM_Event_PseudoConstant::participantStatus();
$ids = CRM_Event_BAO_Participant::getParticipantIds($contributionId);
foreach ($ids as $val) {
$participantUpdate['id'] = $val;
$participantUpdate['status_id'] = array_search('Registered', $participantStatuses);
CRM_Event_BAO_Participant::add($participantUpdate);
}
}
$trxnsData['participant_id'] = $participantId;
return civicrm_api3('Payment', 'create', $trxnsData)['id'];
}
Copy link
Member

@monishdeb monishdeb Jun 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be on the safe side if $payment NOT IN 'refund', 'owed' should we throw a exception message on else{...} loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GIven we don't throw one now I don't think we should add one in 'just in case'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok no problem


if (!empty($financialTrxn)) {
self::recordPaymentActivity($contributionId, $participantId, $financialTrxn->total_amount, $financialTrxn->currency, $financialTrxn->trxn_date);
return $financialTrxn;
}

}

/**
Expand Down
8 changes: 5 additions & 3 deletions CRM/Contribute/Form/AdditionalPayment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,12 @@ public function submit($submittedValues) {
'id' => $this->_contributionId,
]);
$contributionStatusId = CRM_Utils_Array::value('contribution_status_id', $contribution);
$result = CRM_Contribute_BAO_Contribution::recordAdditionalPayment($this->_contributionId, $this->_params, $this->_paymentType, $participantId);
$paymentID = CRM_Contribute_BAO_Contribution::recordAdditionalPayment($this->_contributionId, $this->_params, $this->_paymentType, $participantId);
// Fetch the contribution & do proportional line item assignment
$params = ['id' => $this->_contributionId];
$contribution = CRM_Contribute_BAO_Contribution::retrieve($params, $defaults, $params);
// @todo - this line needs to be moved to the Payment.create api - it's not form layer appropriate.
// testing required.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

CRM_Contribute_BAO_Contribution::addPayments([$contribution], $contributionStatusId);
if ($this->_contributionId && CRM_Core_Permission::access('CiviMember')) {
$membershipPaymentCount = civicrm_api3('MembershipPayment', 'getCount', ['contribution_id' => $this->_contributionId]);
Expand All @@ -382,8 +384,8 @@ public function submit($submittedValues) {

$statusMsg = ts('The payment record has been processed.');
// send email
if (!empty($result) && !empty($this->_params['is_email_receipt'])) {
$sendResult = civicrm_api3('Payment', 'sendconfirmation', ['id' => $result->id])['values'][$result->id];
if (!empty($paymentID) && !empty($this->_params['is_email_receipt'])) {
$sendResult = civicrm_api3('Payment', 'sendconfirmation', ['id' => $paymentID])['values'][$paymentID];
if ($sendResult['is_sent']) {
$statusMsg .= ' ' . ts('A receipt has been emailed to the contributor.');
}
Expand Down
3 changes: 2 additions & 1 deletion CRM/Financial/BAO/Payment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public static function create($params) {
}
elseif ($params['total_amount'] < 0) {
$trxn = self::recordRefundPayment($params['contribution_id'], $params, FALSE);
CRM_Contribute_BAO_Contribution::recordPaymentActivity($params['contribution_id'], CRM_Utils_Array::value('participant_id', $params), $params['total_amount'], $trxn->currency, $trxn->trxn_date);
}

if ($isPaymentCompletesContribution) {
Expand Down Expand Up @@ -319,7 +320,7 @@ public static function filterUntestedTemplateVariables($params) {
*
* @return CRM_Financial_DAO_FinancialTrxn
*/
public static function recordRefundPayment($contributionId, $trxnData, $updateStatus) {
protected static function recordRefundPayment($contributionId, $trxnData, $updateStatus) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm you missed a spot

CRM//Contribute/BAO/Contribution.php:4017:      $financialTrxn = CRM_Financial_BAO_Payment::recordRefundPayment($contributionId, $trxnsData, $updateStatus);

which expect it to be public.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monishdeb I don't see that line.. at that line I see
if (!empty($financialTrxn)) {

Are you on latest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton yes I am checking on the latest instance heres the line https://github.com/civicrm/civicrm-core/blob/master/CRM/Contribute/BAO/Contribution.php#L4017

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monishdeb but isn't that line removed in this pr?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh right ... (arghhh).. sorry my bad :(

list($contributionDAO, $params) = self::getContributionAndParamsInFormatForRecordFinancialTransaction($contributionId);

$params['payment_instrument_id'] = CRM_Utils_Array::value('payment_instrument_id', $trxnData, CRM_Utils_Array::value('payment_instrument_id', $params));
Expand Down