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

CRM-20252 Improve text when processing possibly-delayed-payments #9971

Merged
merged 1 commit into from
Mar 18, 2017
Merged

CRM-20252 Improve text when processing possibly-delayed-payments #9971

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
25 changes: 23 additions & 2 deletions CRM/Contribute/Form/Contribution/ThankYou.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public function buildQuickForm() {
if ($invoicing) {
$getTaxDetails = FALSE;
$taxTerm = CRM_Utils_Array::value('tax_term', $invoiceSettings);
foreach ($this->_lineItem as $key => $value) {
foreach ($this->_lineItem as $value) {
foreach ($value as $v) {
if (isset($v['tax_rate'])) {
if ($v['tax_rate'] != '') {
Expand All @@ -131,7 +131,6 @@ public function buildQuickForm() {
$this->assign('totalTaxAmount', $params['tax_amount']);
}
if (!empty($this->_values['honoree_profile_id']) && !empty($params['soft_credit_type_id'])) {
$honorName = NULL;
$softCreditTypes = CRM_Core_OptionGroup::values("soft_credit_type", FALSE);

$this->assign('soft_credit_type', $softCreditTypes[$params['soft_credit_type_id']]);
Expand Down Expand Up @@ -284,6 +283,28 @@ public function buildQuickForm() {
$this->assign('friendURL', $url);
}

$isPendingOutcome = TRUE;
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton should this be FALSE as the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps it should default to !is_pay_later - ie if we are dealing with pay later then by default FALSE, otherwise TRUE & then we check

Copy link
Contributor

Choose a reason for hiding this comment

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

That could make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested & it is fine for pay later. So, for non-paylater it should reflect the outcome. I think it's safer if any doubt to give the 'we don't know yet' message

try {
// A payment notification update could have come in at any time. Check at the last minute.
$contributionStatusID = civicrm_api3('Contribution', 'getvalue', array(
'id' => CRM_Utils_Array::value('contributionID', $params),
'return' => 'contribution_status_id',
'invoice_id' => CRM_Utils_Array::value('invoiceID', $params),
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah - the point is the ipn could have updated the status - so looking to find out what it actually is

Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't that be able to be found through just the ID, i'm just wondering why the invoice_id is needed specifically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could usefully also react to failure here.... but as a follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah invoice id - because when I deployed it I found sometimes on the IPN return we have contribution id but not invoice id

));
if (CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $contributionStatusID) === 'Pending'
&& !empty($params['payment_processor_id'])
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton is this to exclude pay later options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, currently false is kinda the default & then if there is a payment processor & it is pending then we change to pending. I just double checked & the text is fine when pay later is in use

) {
$isPendingOutcome = TRUE;
}
else {
$isPendingOutcome = FALSE;
}
}
catch (CiviCRM_API3_Exception $e) {

}
$this->assign('isPendingOutcome', $isPendingOutcome);

$this->freeze();

// can we blow away the session now to prevent hackery
Expand Down
2 changes: 1 addition & 1 deletion templates/CRM/Contribute/Form/Contribution/ThankYou.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
{/if}
</div>
{/if}
{elseif $contributeMode EQ 'notify' OR ($contributeMode EQ 'direct' && $is_recur) }
{elseif $isPendingOutcome}
<div>{ts 1=$paymentProcessor.name}Your contribution has been submitted to %1 for processing. Please print this page for your records.{/ts}</div>
{if $is_email_receipt}
<div>
Expand Down