CRM-21134 fix enotices in payment processor code assignment. #10935
Conversation
9fd064b to
bce6133
Compare
8ccf0ef to
cd2861e
Compare
| if ($assignCCInfo) { | ||
| $paymentFields = $this->_paymentProcessor['object']->getPaymentFormFields(); | ||
| foreach ($paymentFields as $index => $paymentField) { | ||
| if (!isset($this->_params[$paymentField])) { |
There was a problem hiding this comment.
I cannot really review as this block is vastly different from the one it replaces and there are no comments.
| {ts}Direct Debit Information{/ts} | ||
| {else} | ||
| {ts}Credit Card Information{/ts} | ||
| {if $paymentFieldsetLabel} |
There was a problem hiding this comment.
I am sure the paymentFieldsetLabel includes the label 'Direct Debit Information' or 'Direct Debit Information'.
| {/if} | ||
| </div> | ||
| {if $paymentProcessor.payment_type & 2} | ||
| {if $paymentProcessor.payment_type == 2} |
There was a problem hiding this comment.
Yep, agreed, the & operator couldn't be more confusing ...
This is generally a code tidy up to push out the principles we have been implementing (ie. the idea that the processors define their fields. The code can cause e-notices in extensions as is because it is prescriptive about fields. I removed the ANDing in the tpl because actually we are not looking to match numbers like 2 2 is the number we have hard-coded for direct debits (we should be using an option group but that is another story
cd2861e to
9f7f8a5
Compare
|
I've improved the comments on the getPaymentFormFields function & declared the object that calls it using inline var declaration so anyone working with the code in an IDE can easily see what is being retrieved. @monishdeb are you able to look at this one - I know what you want me to look at..... |
|
As per code point of view, the patch looks good. Tested the impact on online contribution w/ desired payment processor and ensured that this patch is clean to merge and doesn't cause any unintended regression or side-effect. |
|
thanks @monishdeb |
Overview
This is a code tidy up that prevents enotices in obscure circumstances
Before
Direct debit (or credit card fields) show appropriately on the Confirm page
After
No visible change.
Technical Details
This is generally a code tidy up to push out the principles we have been implementing (ie.
the idea that the processors define their fields. The code can cause e-notices in extensions as is
because it is prescriptive about fields.
I removed the ANDing in the tpl because actually we are not looking to match numbers like 2
2 is the number we have hard-coded for direct debits (we should be using an option group but that
is another story
Comments
The enotices occur when the payment processor does not neatly fit into credit card or debit card assumptions. For purposes of review I don't believe these need to be replicated.