REF Deprecate API3 _ipn_process_transaction()

[mattwire]

Overview

The _ipn_process_transaction() function is a wrapper method called by both API3 completetransaction and repeattransaction before calling completeOrder(). We would like to stop calling completeOrder() at all for repeattransaction and deprecate completetransaction in favour of calling API3 Payment.create.

_ipn_process_transaction() is basically just manipulating parameters before passing along to completeOrder(). This duplicates the contents of that function and puts it directly in the calling API. This allows us to deprecate _ipn_process_transaction() and work towards simplification of the repeattransaction/completetransaction API functions.

Before

Both API calls use _ipn_process_transaction().

After

CiviCRM core does not use _ipn_process_transaction(). @artfulrobot GoCardless is currently calling this function directly and when this PR is merged you'll see deprecation notices so that will need fixing at some point (perhaps copy the contents of the function initially?).

Technical Details

Explained above.

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[artfulrobot]

Thanks, @mattwire

Tracking at https://lab.civicrm.org/extensions/gocardless/-/issues/127

[mattwire]

Marked as "ok-without-test" because this is well tested code and we are not changing anything - just duplicating code.

[artfulrobot]

Just flagging that I've not considered what this means for GC. There must have been a reason I reached for that code, and needed to avoid the api calls that also include it. I just need to spend some time on this and haven't got that time at the mo.

[mattwire]

@artfulrobot per your comment in the code:

Handle the case where the original Contribution failed, but then a successful one comes in. (Issue #82)

Which is almost certainly because Contribution.repeattransaction doesn't work unless there is a previous "Completed" contribution which is not always the case (Eg. the previous one failed, was refunded etc.). That will be fixed by #22487 and then you'd be able to call repeattransaction directly.

I should note that deprecating does not mean removing as yet! We'd normally wait about six months before actually removing the function and I'm proposing that you simply make a copy of it in the GoCardless code until you are ready to update/clean it up.

[demeritcowboy]

I haven't r-run'd and usually stay away from this area but

• Appears to be a straight copy.
• Variable names are all the same.
• No pass-by-ref in the original calling function to think about.
• It comes at the end of the functions so doesn't affect vars after it.
• Universe shows the depracatee (deprecee?) is only used here and in gocardless. It should still function as before but with a notice.

Jenkins retest this please.

[mattwire]

Thanks @demeritcowboy

[eileenmcnaughton]

Yep agree - that function was definitely never supported to be called from outside of core but since you identified a place calling it then deprecation = better than removal.

The repeatransaction function is still called within completeOrder but is ALMOST completely separated from the rest of the function (if it only supported 'Pending' as status it would return)