Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove legacy handling for 'fixing' line_item.entity_id #18155

Merged
merged 1 commit into from
Aug 15, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 15, 2020

Overview

Removes code deprecated in 2018 that causes the entity_id on a line item attached to a membership to be incorrect in edge circumstances, in an attempt to fix it in other edge cases. The deprecation notice has never been 'noticed' so is probably not hit

Before

When an Order is created with 2 line items, each linked to a membership (in this case linked to different contact ids) the second line item is altered to have the same membership id as the first. A deprecation notice is hit

After

Line items unchanged in processPriceSet

Technical Details

I tried to see what was happening with the test in #18113
and spotted that it wasn't using Order.api so the line items were likely wrong. However, once I set
it up to use the Order api I found the code we added back in 2018 to attempt to cope with other code
setting entity_id incorrectly was actually itself setting entity_id incorrectly. The issue
happens when there are 2 Memberships linked to one contribution & the removed code 'decides'
one is wrong and alters it. #11816

This line of code throws a deprecation notice - which has not been reported / apparently seen in the last
couple of years so I'm assuming it's not actually doing any good and without removing it this test actually
fails

Comments

@civibot
Copy link

civibot bot commented Aug 15, 2020

(Standard links)

I tried to see what was happening with the test in civicrm#18113
and spotted that it wasn't using Order.api so the line items were likely wrong. However, once I set
it up to use the Order api I found the code we added back in 2018 to attempt to cope with other code
setting entity_id incorrectly was actually itself setting entity_id incorrectly. The issue
happens when there are 2 Memberships linked to one contribution & the removed code 'decides'
one is wrong and alters it. civicrm#11816

This line of code throws a deprecation notice - which has not been reported / apparently seen in the last
couple of years so I'm assuming it's not actually doing any good and without removing it this test actually
fails
@mattwire mattwire merged commit fa1fd85 into civicrm:master Aug 15, 2020
@eileenmcnaughton eileenmcnaughton deleted the member_order branch August 15, 2020 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants