Prevent error on price set membership update

[eileenmcnaughton]

Overview

Recut of #13937 fixes a fatal error when (as replicated in the test) a price set has 2 multiple membership lines

Before

The error occurs in a scenario I'm not even sure is really valid - ie a price set purchase where 2 of the price fields are purchases of the same membership type. This results in a DB error when repeattransaction is called

After

DB Error fixed. In addition a test is added and the code makes more sense

Technical Details

Fixes an obscure bug which @pradpnayak wrote a test for a while back in #13937

I dug into if & feel it's work committing. The underlying issue is that code quite deep down tries to compensate for earlier code not 'doing it's work' properly. In this case we have the situation where LineItem.create calls MembershipPayment.create to ensure MembershipPayment records are created and MemberhipPayment tries to ensure the lineitems exist. We can say that 'if MembershipPayment.create is called from LineItem.create THEN the line item part should already be correct.'

In addition we can see that a hack was added here #8717 that caused it to try to 'guess' the membership id when the entity type is set. I was going to try to narrow that in this but decided separately would be better

Comments

@pradpnayak @monishdeb

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

[davejenx]

Just noting how this scenario came about. A Drupal Civi webform had been set up with an option to create up to 4 additional memberships, for additional contacts, in addition to the main membership. Why they didn't use inheritance, we don't know. The result was numerous recurring contributions that covered one main membership plus up to 4 additional memberships.

[eileenmcnaughton]

@davejenx if you or
@pradpnayak confirm this I'm Ok to merge it - I think it's pretty safe

[pradpnayak]

Hi @eileenmcnaughton

I did UI test today using with and without priceset. The entries are created successfully in line item table. This is GOOD TO MERGE.

Thanks
Pradeep

[pradpnayak]

jenkins test this please

[eileenmcnaughton]

@pradpnayak & you think the code approach makes sense?

[eileenmcnaughton]

it's similar but not the same as your original one

[pradpnayak]

@eileenmcnaughton yes it does make sense for me.