[REF] Improve function signature for retrieve() in PaypalProIPN , add test for when trxn_id is present

[eileenmcnaughton]

Overview

Fix retrieve() in PaypalProIPN , add test for when trxn_id is present

Before

retrieve signature has deprecated location parameter which only exists to log misinformation

Also the function is called statically

After

old (already deprecated) location removed from the retrieve function signature, calls made non-static. Test added that shows the reported problem not occurring - when trxn_id can be retrieved

Technical Details

In trying to understand https://lab.civicrm.org/dev/core/-/issues/4158 I wrote a unit test that replicates the problem NOT occurring & I was able to confirm the vital thing that prevents it is retrieving trxn_id in this line
However, that line was deepl confusing as it implied the value came from $_POST which is not necessarily true & not helpful in the course of the code.

inputParameters is set here or possibly here if sites configured the old url & it somehow still works

Comments

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

Grepping around a bit, I agree that this looks more consistent - ie it makes the signature of PayPalProIPN::retrieve() align with PayPalIPN::retrieve().

(EDIT: In universe, there are some extensions that share the older signature. But AFAICS, the signatures of retrieve() are actually independent -- so it's more a question of what style to use in each; and this looks like a cleaner/newer style.)

[totten]

@eileenmcnaughton One thing I'm confused about in the description - the PR title describes this as "fix"ing the retrieve() method; but in "Technical Details" it seems to be about "trying to understand' (de-obfuscating some confusing code). Should this PR be viewed more as a "(REF)" or as a possible bugfix?

[eileenmcnaughton]

@totten it's definitely a REF - I guess I could add that - I have tended away from NFC and REF a bit cos I keep remembering Chris Burgess pointing out to me how much code-speak we use in CiviCRM-speak (ie words that are only meaningful to us) - but yeah maybe in this case it would make it clearer