Create-membership forms on back end are throwing exception
We just upgraded to 5.24.6 from 5.19.2, and hit a problem with creating DDs in the back end. It works fine from a front-end Contribution Page, but not from a contact record. I traced it back to a problem with the payment_instrument_id. What basically happens is this:
- During buildform, CRM_Member_Form (or its ultimate parent) looks up the default payment_instrument_id. In our case the default payment_instrument_id is not SmartDebit.
- Eventually this progresses to CRM_Core_Payment_Form::setPaymentFieldsByProcessor()
- This passes the payment_instrument_id on to CRM_Core_Payment::setPaymentInstrumentID()
- Prior to this commit, setPaymentInstrumentID() would ignore the passed value and use the payment_instrument_id of the payment processor. The latter would be correct as the payment processor is SmartDebit. But since the commit, setPaymentInstrumentID() uses the payment instrument ID it's been passed. So in our situation it sets $this->paymentInstrumentID to the default value, rather than the value of the payment processor.
- setPaymentInstrumentID then actually returns getPaymentInstrumentID(), rather than the value directly
- getPaymentInstrumentID duly returns $this->paymentInstrumentID. Although there is logic in there to return the payment_instrument_id of the payment processor, if $this->payment_instrument_id is null.
- So we end up with CRM_Core_Payment_Smartdebit->paymentInstrumentID being the default payment instrument, rather than the correct one
- This ends up creating a recurring contribution with the wrong payment_instrument_id
- It all sorta works until the final stage, where CRM_Smartdebit_Mandates::retrieve calls CRM_Smartdebit_Api::buildUrl() using the values of the recurring contribution. This then can't find a url for the API and throws an exception
- The form stalls.
This seems to be linked to artfulrobot's PropertyBag work, which I don't understand very well.
My 'fix' is to change CRM_Core_Payment::setPaymentInstrumentID() to:
if ($this->_paymentProcessor['payment_instrument_id']) {
$this->paymentInstrumentID = $this->_paymentProcessor['payment_instrument_id'];
} else {
$this->paymentInstrumentID = (int) $paymentInstrumentID;
}
And this seems not to break anything else, but I've no idea whether it's sensible.
Edited by Andrew West