[REF] Paypal ipn - cleanup references to completion

[eileenmcnaughton]

Overview

[REF] Paypal ipn - cleanup references to completion

Before

Params passed around

After

function used

Technical Details

This extracts a function to check if the contribution is completed.

I also rationalised the validation - it was using a combo of recur and first to
validate but on thinking it through I realised all it was saying was
'if we are finalising a pending contribution the amount must match'

I think that's fine even for recur with a change in amount - that seems
to me to be something that happens down the track but we still expect
the very first one to come in with the value it originally
had - if that is NOT true then we probably should just remove the check

Comments

Note this leaves a bunch of unused params - I think it's cleaner to remove those once this is merged

[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).

[seamuslee001]

So if someone goes in and alters their PayPal Recurring in the PayPal GUI (as that is the only place to do it) that will mean that any newly created contribution based on the template would have a different amount to what is coming in the input right?

[eileenmcnaughton]

@seamuslee001 right - so the question is - at what point is that valid - I am not sure it would be valid before the first amount is received? (rather than after the recurring has started)

But if we think it is I think we should remove the amount check altogether

[seamuslee001]

Yeh so I don't think it should be valid on the first amount but valid after that. So are we certain that all subsequently created contributions are completed before the payment is processed is the only question I think?

[eileenmcnaughton]

@seamuslee001 does what you said make sense or am I just needing coffee?

[eileenmcnaughton]

Oh I think I get it - so the contribution being checked for completeness is always the first contribution. If the first contribution is completed then it goes on to create a subsequent contribution

[eileenmcnaughton]

@seamuslee001 looks like I need to go in & fix something - what is your vote on the validation

1. don't validate total_amount at all
2. only validate total_amount if an existing pending contribution is being completed (the first in the recurring series or a one-off payment express in practice)
3. some more complicated compromise

[seamuslee001]

I would probably go with 2 I would think, that seems to be the closest to the original right?

[eileenmcnaughton]

@seamuslee001 I guess 3 is closer to the original - 3 permitted amount to change for the first payment for a recurring - but I'm not convinced that is a real scenario or that the code really copes with it beyond this point

[eileenmcnaughton]

@seamuslee001 well tests turned out to be enforcing 3 - either by design or by accident but it's back now