Skip to content

Commit

Permalink
Use payment create API for backend record payment forms
Browse files Browse the repository at this point in the history
  • Loading branch information
@eileenmcnaughton
Jitendra Purohit authored and eileenmcnaughton committed Dec 30, 2018
1 parent 0d061a7 commit 570243c
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 85 deletions.
44 changes: 38 additions & 6 deletions CRM/Contribute/BAO/Contribution.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4495,9 +4495,9 @@ public static function isSingleLineItem($id) {
public static function addPayment($params) {
$contribution = civicrm_api3('Contribution', 'getsingle', ['id' => $params['contribution_id']]);
$contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus($contribution['contribution_status_id'], 'name');
if ($contributionStatus != 'Partially paid'
&& !($contributionStatus == 'Pending' && $contribution['is_pay_later'] == TRUE)
) {
$paymentType = CRM_Utils_Array::value('payment_type', $params, 'owed');

if ($paymentType == 'owed' && $contributionStatus != 'Partially paid' && $contributionStatus != 'Pending') {

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

Why are we passing this in if it's not valid - this has me confused

This comment has been minimized.

Sign in to view

Copy link
@jitendrapurohit

jitendrapurohit Jan 9, 2019

Contributor

I think I overlooked the mail thread containing these set of questions. I'll update the main PR with the suggestions tomorrow. Thanks @eileenmcnaughton.

For the above question - Not sure what you mean by "not valid" but seems the exception here only makes sense when we are processing an owed payment and not refund?

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Jan 9, 2019

Contributor

@jitendrapurohit so it seems to me that 'owed' is a calculated field. The point at which we calculate 'owed' is the point at which that is calculated seems like the point to determine whether or not to add a payment rather than in this fledgling BAO-like class. I'd be more inclined to remove the check all together pending @monishdeb input if it's actively causing a problem

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 9, 2019

Contributor

$contributionStatus is not able to handle a number of scenarios now when there can be multiple payments coming in, some that take days to arrive, and multiple refunds possibly going out, and edits to add and remove items in a way that can mean a Refund is pending even though one of the payments is still pending.

Without delving into details, I should say one relatively common use case that I hadn't heard about or planned against till recently is when duplicate payments are mistakenly made. The way this is handled is apparently to deposit the second payment and then issue a refund. This is the use case that made me allow a payment to be received when contribution status was completed.

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Jan 10, 2019

Contributor

@JoeMurray then I think we should remove this exception handling here altogether

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 10, 2019

Contributor

I must admit that I don't understand how it could be correct so I'd be happy to remove it unless it could be better clarified to me.
throw new API_Exception('Please select a contribution which has a partial or pending payment');
}
else {
Expand All @@ -4519,12 +4519,14 @@ public static function addPayment($params) {
[
'id' => $contribution['id'],
'contribution_status_id' => 'Partially paid',

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 10, 2019

Contributor

Can't payment be made against other statuses?
'is_pay_later' => 0,

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

why?

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 10, 2019

Contributor

payments can be made against both pay later and non-pay later contributions.

This comment has been minimized.

Sign in to view

Copy link
@jitendrapurohit

jitendrapurohit Jan 11, 2019

Contributor

This is to make sure pay later is turned off for the pending contribution when a first payment is recorded. This is also a migration of the code from AdditionalPayment.php. I've updated the main PR #13330 and moved the functions from Contribution class to Financial BAO layer. I would be testing it over the weekend for any issues and check if any further update is needed.
]
);
}
}
if (!$fullyPaidPayLater) {
$trxn = CRM_Core_BAO_FinancialTrxn::getPartialPaymentTrxn($contribution, $params);
unset($params['id']);
$trxn = CRM_Contribute_BAO_Contribution::recordAdditionalPayment($params['contribution_id'], $params, $paymentType, CRM_Utils_Array::value('participant_id', $params));
if (CRM_Utils_Array::value('line_item', $params) && !empty($trxn)) {
foreach ($params['line_item'] as $values) {
foreach ($values as $id => $amount) {
Expand Down Expand Up @@ -4555,12 +4557,41 @@ public static function addPayment($params) {
}
}
elseif (!empty($trxn)) {
CRM_Contribute_BAO_Contribution::assignProportionalLineItems($params, $trxn->id, $contribution['total_amount']);
$defaults = [];

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

why
$fetchParams = ['id' => $params['contribution_id']];
$contributionDAO = CRM_Contribute_BAO_Contribution::retrieve($fetchParams, $defaults, $fetchParams);
CRM_Contribute_BAO_Contribution::addPayments(array($contributionDAO), $contribution['contribution_status_id']);

This comment has been minimized.

Sign in to view

Copy link
@jitendrapurohit

jitendrapurohit Jan 9, 2019

Contributor

@eileenmcnaughton seems we now have two similar named functionaddPayment() and addPayments() after #13370? Reason why it was named as processAdditionalPayment() before just to make the identification easier. Do we have any other suggestion or the current naming is fine?

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Jan 9, 2019

Contributor

@jitendrapurohit @monishdeb so the thing I think we should decide before continuing is how we should structure the payment functions. I'm leaning towards a class called CRM_FInancial_BAO_Payment or perhaps for clarify CRM_Financial_PseudoBAO_Payment that holds the functions 'create', 'sendconfirmation' and perhaps others that we determine have been well written & are a good reflection of the 'right' functions to have (as opposed to functions that have evolved but perhaps are not what we would 'choose' )

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 9, 2019

Contributor

I like this idea. I prefer name to be CRM_Financial_BAO_Payment.
}
}
}
return $trxn;

if (!empty($params['is_email_receipt'])) {
$componentId = $params['contribution_id'];

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

this makes sense in that we are enabling is_email_receipt - it could probably be it's own function.
$componentDetails = self::getComponentDetails($params['contribution_id']);
$entityType = 'contribution';
if (array_key_exists('participant', $componentDetails)) {
$entityType = 'participant';
$componentId = $componentDetails['participant'];
}
$paymentDetails = CRM_Contribute_BAO_Contribution::getPaymentInfo($componentId, $componentDetails['component'], FALSE, TRUE);
list($contributorDisplayName, $contributorEmail) = CRM_Contact_BAO_Contact_Location::getEmailDetails($componentDetails['contact_id']);
$templateVars = [
'id' => $componentId,
'component' => $componentDetails['component'],
'amtTotal' => $paymentDetails['total'],
'paymentType' => $paymentType,
'amtPaid' => $paymentDetails['paid'],

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

So this is the total amount that has now been paid of the entire contribution? Looks like most of the above is just to generate this & could be it's own fn.
'contributorDisplayName' => $contributorDisplayName,
'contactId' => $componentDetails['contact_id'],
'contributorEmail' => $contributorEmail,
];
if (!empty($params['payment_processor_id'])) {
$templateVars['mode'] = 'live';
}
CRM_Contribute_Form_AdditionalPayment::emailReceipt($templateVars, $params);
}

return $trxn;
}

/**
Expand Down Expand Up @@ -4997,6 +5028,7 @@ protected static function getRecurringContributionDescription($contribution, $ev
* @param array $contributions
* @param string $contributionStatusId
*
* @todo this should be deprecated back into the add payment function.
*/
public static function addPayments($contributions, $contributionStatusId = NULL) {
// get financial trxn which is a payment
Expand Down
139 changes: 64 additions & 75 deletions CRM/Contribute/Form/AdditionalPayment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ public function preProcess() {
$this->_amtTotal = $paymentDetails['total'];

if (!empty($paymentInfo['refund_due'])) {
$paymentAmt = $this->_refund = $paymentInfo['refund_due'];
$this->paymentAmount = $this->_refund = $paymentInfo['refund_due'];

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

So we are switching from a function var to a class var - we could spin this change off to it's own little PR & get it merged
$this->_paymentType = 'refund';
}
elseif (!empty($paymentInfo['amount_owed'])) {
$paymentAmt = $this->_owed = $paymentInfo['amount_owed'];
$this->paymentAmount = $this->_owed = $paymentInfo['amount_owed'];
$this->_paymentType = 'owed';
}
else {
Expand All @@ -130,7 +130,7 @@ public function preProcess() {
$this->assign('contributionMode', $this->_mode);
$this->assign('contactId', $this->_contactID);
$this->assign('paymentType', $this->_paymentType);
$this->assign('paymentAmt', abs($paymentAmt));
$this->assign('paymentAmt', abs($this->paymentAmount));

$this->setPageTitle($this->_refund ? ts('Refund') : ts('Payment'));
}
Expand Down Expand Up @@ -336,38 +336,20 @@ public function submit($submittedValues) {
if ($this->_component == 'event') {
$participantId = $this->_id;
}
$contributionStatuses = CRM_Core_PseudoConstant::get('CRM_Contribute_DAO_Contribution',
'contribution_status_id',
array('labelColumn' => 'name')
);
$contributionStatusID = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $this->_contributionId, 'contribution_status_id');
if ($contributionStatuses[$contributionStatusID] == 'Pending') {
civicrm_api3('Contribution', 'create',
array(
'id' => $this->_contributionId,
'contribution_status_id' => array_search('Partially paid', $contributionStatuses),

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 10, 2019

Contributor

This transition of status from Pending to Partially paid when a payment is received I understand. I don't understand if that will still be the case after something like 570243c#diff-7a5b0e2d131dabc49178460fc63328c0R4521
'is_pay_later' => 0,
)
);
}

if ($this->_mode) {
// process credit card
$this->assign('contributeMode', 'direct');
$this->processCreditCard();
}

$defaults = array();
$contribution = civicrm_api3('Contribution', 'getsingle', array(
'return' => array("contribution_status_id"),
'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);
// Fetch the contribution & do proportional line item assignment
$params = array('id' => $this->_contributionId);
$contribution = CRM_Contribute_BAO_Contribution::retrieve($params, $defaults, $params);
CRM_Contribute_BAO_Contribution::addPayments(array($contribution), $contributionStatusId);
$payment = civicrm_api3('Payment', 'create', array_merge([

This comment has been minimized.

Sign in to view

Copy link
@JoeMurray

JoeMurray Jan 10, 2019

Contributor

I like seeing this use of Payment create. I presume it is handling all of the contribution_status_id updating that was being done inelegantly in the code being replaced here.
'contribution_id' => $this->_contributionId,
'total_amount' => $this->_params['total_amount'],
'payment_type' => $this->_paymentType,
'participant_id' => $participantId,
'mode' => $this->_mode,
], $this->_params));
if ($this->_contributionId && CRM_Core_Permission::access('CiviMember')) {
$membershipPaymentCount = civicrm_api3('MembershipPayment', 'getCount', array('contribution_id' => $this->_contributionId));
if ($membershipPaymentCount) {
Expand All @@ -382,16 +364,8 @@ public function submit($submittedValues) {
}

$statusMsg = ts('The payment record has been processed.');
// send email
if (!empty($result) && !empty($this->_params['is_email_receipt'])) {
$this->_params['contact_id'] = $this->_contactId;
$this->_params['contribution_id'] = $this->_contributionId;
// to get 'from email id' for send receipt
$this->fromEmailId = $this->_params['from_email_address'];
$sendReceipt = $this->emailReceipt($this->_params);
if ($sendReceipt) {
$statusMsg .= ' ' . ts('A receipt has been emailed to the contributor.');
}
if (empty($payment['is_error']) && !empty($this->_params['is_email_receipt'])) {
$statusMsg .= ' ' . ts('A receipt has been emailed to the contributor.');
}

CRM_Core_Session::setStatus($statusMsg, ts('Saved'), 'success');
Expand All @@ -402,7 +376,6 @@ public function processCreditCard() {
$session = CRM_Core_Session::singleton();

$now = date('YmdHis');
$fields = array();

// we need to retrieve email address
if ($this->_context == 'standalone' && !empty($this->_params['is_email_receipt'])) {
Expand Down Expand Up @@ -506,78 +479,92 @@ public function processCreditCard() {
*
* @return bool
*/
public function emailReceipt(&$params) {
public static function emailReceipt($values, $params) {

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

Let's figure out where this function should live - either on a new Payment pseudo-bao or somewhere else in the form layer

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

what goes in values? Passing arrays of 'flexible' keys has gotten us into much unpredictability in the past
// email receipt sending
// send message template
if ($this->_component == 'event') {
$template = CRM_Core_Smarty::singleton();
$fromEmails = CRM_Core_BAO_Email::getFromEmail();

if (!empty($values['component']) && $values['component'] == 'event') {

This comment has been minimized.

Sign in to view

Copy link
@eileenmcnaughton

eileenmcnaughton Dec 30, 2018

Contributor

perhaps we should just pass in component if known
// fetch event information from participant ID using API
$eventId = civicrm_api3('Participant', 'getvalue', array(
'return' => "event_id",
'id' => $this->_id,
'id' => $values['id'],
));
$event = civicrm_api3('Event', 'getsingle', array('id' => $eventId));
$fromEmails = CRM_Event_BAO_Event::getFromEmailIds($eventId);

$this->assign('event', $event);
$this->assign('isShowLocation', $event['is_show_location']);
$template->assign('event', $event);
$template->assign('isShowLocation', $event['is_show_location']);
if (CRM_Utils_Array::value('is_show_location', $event) == 1) {
$locationParams = array(
'entity_id' => $eventId,
'entity_table' => 'civicrm_event',
);
$location = CRM_Core_BAO_Location::getValues($locationParams, TRUE);
$this->assign('location', $location);
$template->assign('location', $location);
}
// assign payment info here
$paymentConfig['confirm_email_text'] = CRM_Utils_Array::value('confirm_email_text', $event);
$template->assign('paymentConfig', $paymentConfig);
}

// assign payment info here
$paymentConfig['confirm_email_text'] = CRM_Utils_Array::value('confirm_email_text', $params);
$this->assign('paymentConfig', $paymentConfig);

$this->assign('totalAmount', $this->_amtTotal);

$isRefund = ($this->_paymentType == 'refund') ? TRUE : FALSE;
$this->assign('isRefund', $isRefund);
$template->assign('component', CRM_Utils_Array::value('component', $values));
$template->assign('totalAmount', CRM_Utils_Array::value('amtTotal', $values));
$isRefund = ($values['paymentType'] == 'refund') ? TRUE : FALSE;
$template->assign('isRefund', $isRefund);
if ($isRefund) {
$this->assign('totalPaid', $this->_amtPaid);
$this->assign('refundAmount', $params['total_amount']);
$template->assign('totalPaid', CRM_Utils_Array::value('amtPaid', $values));
$template->assign('refundAmount', $params['total_amount']);
}
else {
$balance = $this->_amtTotal - ($this->_amtPaid + $params['total_amount']);
$balance = CRM_Contribute_BAO_Contribution::getContributionBalance($params['contribution_id']);
$paymentsComplete = ($balance == 0) ? 1 : 0;
$this->assign('amountOwed', $balance);
$this->assign('paymentAmount', $params['total_amount']);
$this->assign('paymentsComplete', $paymentsComplete);
$template->assign('amountOwed', $balance);
$template->assign('paymentAmount', $params['total_amount']);
$template->assign('paymentsComplete', $paymentsComplete);
}
$this->assign('contactDisplayName', $this->_contributorDisplayName);
$template->assign('contactDisplayName', CRM_Utils_Array::value('contributorDisplayName', $values));

// assign trxn details
$this->assign('trxn_id', CRM_Utils_Array::value('trxn_id', $params));
$this->assign('receive_date', CRM_Utils_Array::value('trxn_date', $params));
$this->assign('paidBy', CRM_Core_PseudoConstant::getLabel(
'CRM_Contribute_BAO_Contribution',
'payment_instrument_id',
$params['payment_instrument_id']
));
$this->assign('checkNumber', CRM_Utils_Array::value('check_number', $params));

$template->assign('trxn_id', CRM_Utils_Array::value('trxn_id', $params));
$template->assign('receive_date', CRM_Utils_Array::value('trxn_date', $params));
if (!empty($params['payment_instrument_id'])) {
$template->assign('paidBy', CRM_Core_PseudoConstant::getLabel(
'CRM_Contribute_BAO_Contribution',
'payment_instrument_id',
$params['payment_instrument_id']
));
}
$template->assign('checkNumber', CRM_Utils_Array::value('check_number', $params));
if (!empty($params['mode'])) {
$template->assign('contributeMode', 'direct');
$template->assign('address', CRM_Utils_Address::getFormattedBillingAddressFieldsFromParameters(
$params,
CRM_Core_BAO_LocationType::getBilling()
));
}
$sendTemplateParams = array(
'groupName' => 'msg_tpl_workflow_contribution',
'valueName' => 'payment_or_refund_notification',
'contactId' => $this->_contactId,
'contactId' => CRM_Utils_Array::value('contactId', $values),
'PDFFilename' => ts('notification') . '.pdf',
);

$doNotEmail = NULL;
if (!empty($values['contactId'])) {
$doNotEmail = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $values['contactId'], 'do_not_email');
}
// try to send emails only if email id is present
// and the do-not-email option is not checked for that contact
if ($this->_contributorEmail && !$this->_toDoNotEmail) {
if (array_key_exists($params['from_email_address'], $this->_fromEmails['from_email_id'])) {
if (!empty($values['contributorEmail']) && empty($doNotEmail)) {
list($userName, $receiptFrom) = CRM_Core_BAO_Domain::getDefaultReceiptFrom();
if (!empty($params['from_email_address']) && array_key_exists($params['from_email_address'], $fromEmails)) {
$receiptFrom = $params['from_email_address'];
}

$sendTemplateParams['from'] = $receiptFrom;
$sendTemplateParams['toName'] = $this->_contributorDisplayName;
$sendTemplateParams['toEmail'] = $this->_contributorEmail;
$sendTemplateParams['toName'] = CRM_Utils_Array::value('contributorDisplayName', $values);
$sendTemplateParams['toEmail'] = $values['contributorEmail'];
}
list($mailSent, $subject, $message, $html) = CRM_Core_BAO_MessageTemplate::sendTemplate($sendTemplateParams);
return $mailSent;
Expand Down Expand Up @@ -612,10 +599,12 @@ public function testSubmit($params, $creditCardMode = NULL, $entityType = 'contr
if (!empty($paymentInfo['refund_due'])) {
$this->_refund = $paymentInfo['refund_due'];
$this->_paymentType = 'refund';
$this->paymentAmount = $paymentInfo['refund_due'];
}
elseif (!empty($paymentInfo['amount_owed'])) {
$this->_owed = $paymentInfo['amount_owed'];
$this->_paymentType = 'owed';
$this->paymentAmount = $paymentInfo['amount_owed'];
}
}

Expand Down
11 changes: 10 additions & 1 deletion api/v3/Payment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ function civicrm_api3_payment_create(&$params) {
$params['total_amount'] = $amount;
}
$trxn = CRM_Contribute_BAO_Contribution::addPayment($params);

$values = array();
_civicrm_api3_object_to_array_unique_fields($trxn, $values[$trxn->id]);
return civicrm_api3_create_success($values, $params, 'Payment', 'create', $trxn);
Expand Down Expand Up @@ -169,6 +168,16 @@ function _civicrm_api3_payment_create_spec(&$params) {
'type' => CRM_Utils_Type::T_INT,
'api.aliases' => array('payment_id'),
),
'payment_type' => array(
'title' => 'Payment Type',
'type' => CRM_Utils_Type::T_STRING,
'description' => ts("Type of payment - 'owed' or 'refund'."),
),
'is_email_receipt' => array(
'title' => 'Send Email Receipt',
'type' => CRM_Utils_Type::T_BOOLEAN,
'description' => ts('If true, receipt is automatically emailed to contact after payment.'),
),
);
}

Expand Down
Loading

0 comments on commit 570243c

Please sign in to comment.