dev/financial#74 Create ParticipantPayment record for every participant when registering multiple participants for event

[mattwire]

Overview

We have ParticipantPayment and MembershipPayment records in CiviCRM which link a contribution to a participant / membership and show which contribution was used to "pay" for the participant/membership. ParticipantPayment is not actually used very much, compared with MembershipPayment which is used for various UI forms when showing related memberships / contributions etc.
It was identified at the sprint in Barcelona that when registering multiple participants a ParticipantPayment record was only being created for the primary participant instead of for all participants.

Before

Only one participantPayment record created for multiple participant registration

After

One participantPayment record created for each participant in a multiple participant registration

Technical Details

The Order API creates one per participant. The MembershipPayment / ParticipantPayment records are not supposed to show who paid (that can be identified from the contribution) but is supposed to show that "this membership/participant" was paid for from this contribution.

Comments

@mecachisenros @eileenmcnaughton This is what we found at the sprint
Thanks @sarvesh211999 for the unit test

[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]

@mattwire

PHP Fatal error:  Cannot declare class CRM_Event_Form_Registration_ConfirmTest, because the name is already in use in /home/jenkins/bknix-dfl/build/core-15555-i9ur/web/sites/all/modules/civicrm/tests/phpunit/CRM/Event/Form/Registration/ConfirmMultipleParticipantTest.php on line 103

[eileenmcnaughton]

Looking at the code I find it more convincing that this was a bug since this appears to be fixing something that was broken rather than changing behaviour - it could even be a recent regression? We should double check when if (empty($ids['contribution'])) { stopped being true

[eileenmcnaughton]

Hmm - OK I looked & I think master is OK for this - I can't see evidence this was a recent regression - it probably regressed a while back but I now believe it IS a regression rather than a behaviour change

[seamuslee001]

@mattwire test failure looks related

[eileenmcnaughton]

@mattwire still related fails :-(

[eileenmcnaughton]

@mattwire still failing :-(

[mattwire]

Closing for now as there is no reasonable chance that I'm going to circle back to this any time soon. Its's being tracked via https://lab.civicrm.org/dev/financial/issues/74 @eileenmcnaughton if you get chance to take a look at any of this, it already has a test here :-) The issue I hit is a rabbit hole of converting follow-on code (ie. receipting code etc) to read from an array instead of an integer variable. I know you've done some cleanup so it might be easier now.