[REF] Simplify membership form code towards simplifying BAO
authoreileen <emcnaughton@wikimedia.org>
Mon, 26 Oct 2020 04:05:58 +0000 (17:05 +1300)
committereileen <emcnaughton@wikimedia.org>
Thu, 10 Dec 2020 02:36:19 +0000 (15:36 +1300)
commite0ec7c6209c81a8d9374cae192494f346a909a45
treee1823e7f754e541e981489bc480505a2e998b9c4
parent5655829bd648df14425054f14878756822e39e39
[REF] Simplify membership form code towards simplifying BAO

On digging into the handling for the param relate_contribution_id in the BAO I found it is called
only from the membership form & it's really a hack for the way price sets were super-imposed on the form.

This has really solid testing in testTwoInheritedMembershipsViaPriceSetInBackend (Thanks @davejenx)
and stepping through there is the best way to make sense of it.

What is happening is that if there are 2 memberships in a price set Membership.create is called twice.

However, because the parameter contribution_status_id is passed in it attempts to create the contribution
twice. If we don't add this the second time then we wind up with the second membership payment not created
because that membership didn't exist the first round. So, an extra param relate_contribution_id got
added which says 'if this is passed in create the MembershipPayment record that didn't get created
the first time with the contribution id that we just did a lookup on.

All of this is needed because we create the contribution between the first Membership create
and the second. This removes both magic params from the create, and instead calls
recordMembershipContribution after both are done. I thought about calling Order.create or
just Contribution.create here but there is stuff to unravel around the soft credit creation
being done in RecordMembershipContribution and I am not yet clear if tests are adequate there
CRM/Member/BAO/Membership.php
CRM/Member/Form/Membership.php
tests/phpunit/CRM/Member/Form/MembershipTest.php