[REF] IPN - move unshared chunk of code out of shared function #18600
Conversation
|
(Standard links)
|
|
Well that confirms the test cover - will check Stacktrace /home/jenkins/bknix-dfl/build/core-18600-8sidk/web/sites/all/modules/civicrm/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php:240 |
We are passing a lot of variables into recur (& going to a lot of work to construct them) when they are not actually processed in a common way - returning them to the calling functions makes it clear we are a skip & a jump from not needing to instantiate objects at all
eileenmcnaughton
force-pushed
the
aipn
branch
from
60c06ed to
55bc843
Compare
|
@mattwire on working on this cleanup I hit a test fail where it turned out
But - I think fixing the code in completeOrder to use our calculated variable makes more sense than doing it on the IPN class anyway - with this change 'relatedObjects' is no longer accessed! |
| $contribution->contribution_recur_id = $ids['contributionRecur']; | ||
| $contribution->receive_date = $input['receive_date']; | ||
| $contribution->currency = $objects['contribution']->currency; | ||
| $contribution->payment_instrument_id = $objects['contribution']->payment_instrument_id; |
There was a problem hiding this comment.
Due to the test fail I didn't copy this ^^ line over & instead fixed completeOrder to calculate it
|
@eileenmcnaughton Are the changes in BAO/Contribution independent to the authorizenetIPN changes or do they both need merging at the same time? I'm not so comfortable reviewing the authorizenetipn code as I don't use it and am not familiar with the level of test coverage on it. |
|
@mattwire so the test cover is in AuthorizeNetIPNTest & it's pretty thorough. The A.net part needs the other part but I think if you open up the file you can see it's just code being moved |
Overview
[REF] IPN - move unshared chunk of code out of shared function
Before
We are passing variables into recur simply to do this processing - but it could be done in the calling function & we could stop constructing arrays just to call it
Also a couple of unused variables
After
Closer to getting rid of $objects
Technical Details
We are passing a lot of variables into recur (& going to a lot of work to construct them) when they are not
actually processed in a common way - returning them to the calling functions makes it clear we are
a skip & a jump from not needing to instantiate objects at all
Comments
Good test cover in CRM_Core_Payment_AuthorizeNetIPNTest
This is part of the completeOrder cleanup - now that we are not passing paymentProcessor object through my definition of 'done' for this round of work is to clean up / remove how that object is loaded in the IPN functions that call completeOrder