CRM-21436 - Fix fatal error on pay later #11286
Conversation
jitendrapurohit
changed the title
|
jenkins, retest this please |
| @@ -580,7 +580,7 @@ public function assignToTemplate() { | |||
|
|
|||
| // The concept of contributeMode is deprecated. | |||
| // The payment processor object can provide info about the fields it shows. | |||
| if ($assignCCInfo) { | |||
| if ($assignCCInfo && $this->_paymentProcessor) { | |||
There was a problem hiding this comment.
This is not needed here for the fix, added just to be safe.
|
@jitendrapurohit this looks good - I'm less inclined to include the extra 'just in case' because they can be hard to reverse engineer later. In the first one
Could it not just be $isMonetary? it seems that if it is monetary there MUST be a payment processor or else it must be pay later. In the second case I think perhaps we can rename $assignCCInfo to $isMonetary for clarity? |
|
Thanks @eileenmcnaughton. I've tested the above suggestion and updated the PR. While testing I also saw that when no payment processor or pay later is enabled for a contribution page - instead of throwing an exception, the online contribution page(tested on dmaster) actually loads and lets user to complete the contribution process with pay later processor. Fix included. |
| CRM_Core_DAO::VALUE_SEPARATOR, | ||
| CRM_Utils_Array::value('payment_processor', $this->_values) | ||
| ); | ||
| )); |
There was a problem hiding this comment.
explode(' ', NULL); returns array(0 => NULL) which loads processor key 0(pay later) on the contribution page.
| } | ||
| } | ||
|
|
||
| // The concept of contributeMode is deprecated. | ||
| // The payment processor object can provide info about the fields it shows. | ||
| if ($assignCCInfo) { | ||
| if ($isMonetary) { |
There was a problem hiding this comment.
I think we do need the && $this->_paymentProcessor here. Prior to CRM-21134 it had code that did that, and it breaks if you use CiviDiscount with 100% discount as no payment processor will be submitted - there may be other cases too.
Agree with the variable name change.
| CRM_Core_DAO::VALUE_SEPARATOR, | ||
| CRM_Utils_Array::value('payment_processor', $this->_values) | ||
| ); | ||
| )); |
|
@jitendrapurohit @eileenmcnaughton Follow on PR once this is merged. I found similar issues, but caused by a slightly different trigger: #11292 |
|
I feel confident about the changes in here being correct / improvements. There is an outstanding question raised by @mattwire about CiviDiscount which I will review next |
|
Hey, is still have this issue with the current 4.7.28 update. I see this error with a contribution page where "Pay later option" is enabled with "Require Membership Signup" also checked and a priceset is used. Here i try to rebuild my settings: http://dmaster.demo.civicrm.org/civicrm/contribute/transact?reset=1&id=4 (the name of the contributionpage is "pay later test") I'm sorry for asking here for help - i cannot get a account on the issue/jira system. Any Ideas where to fix it? Would save me the go back to a previous version with a 4 weeks old backup :| Edit: I try to disable the hole block of code via the $isMonetary flag |
|
@jitendrapurohit is away this week so unlikely to get a quick response/fix from him |
|
There is follow on discussion here #11292 To get help with JIRA accounts please ask on chat.civicrm.org |
CRM-21436 - Fix fatal error on pay later
Overview
This PR fixes fatal error on pay later contribution page regressed from previous versions.
Before
Errors with the message
After
works fine
Technical Details
This change adds pay later processor to
$this->_paymentProcessorsobject. Git blame of this line shows it has not been changed from a very long time. So, it was never assigned, but it came to our notice after changes made in CRM-21134.Comments
Can this be considered for 4.7.2-rc merge?