Skip to content

Commit

Permalink
Remove legacy handling for 'fixing' line_item.entity_id
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed Aug 15, 2020
1 parent e1c19ea commit c9cf3eb
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 14 deletions.
11 changes: 1 addition & 10 deletions CRM/Price/BAO/LineItem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,18 +426,9 @@ public static function processPriceSet($entityId, $lineItems, $contributionDetai
}
if (!empty($contributionDetails->id)) {
$line['contribution_id'] = $contributionDetails->id;
if ($line['entity_table'] == 'civicrm_contribution') {
if ($line['entity_table'] === 'civicrm_contribution') {
$line['entity_id'] = $contributionDetails->id;
}
// CRM-19094: entity_table is set to civicrm_membership then ensure
// the entityId is set to membership ID not contribution by default
elseif ($line['entity_table'] == 'civicrm_membership' && !empty($line['entity_id']) && $line['entity_id'] == $contributionDetails->id) {
$membershipId = CRM_Core_DAO::getFieldValue('CRM_Member_DAO_MembershipPayment', $contributionDetails->id, 'membership_id', 'contribution_id');
if ($membershipId && (int) $membershipId !== (int) $line['entity_id']) {
$line['entity_id'] = $membershipId;
Civi::log()->warning('Per https://lab.civicrm.org/dev/core/issues/15 this data fix should not be required. Please log a ticket at https://lab.civicrm.org/dev/core with steps to get this.', ['civi.tag' => 'deprecated']);
}
}
}

// if financial type is not set and if price field value is NOT NULL
Expand Down
75 changes: 74 additions & 1 deletion tests/phpunit/CRMTraits/Financial/OrderTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public function createRepeatMembershipOrder() {
$orderID = $this->callAPISuccess('Order', 'create', [
'total_amount' => '200',
'financial_type_id' => 'Donation',
'contribution_status_id' => 'Pending',
'contact_id' => $this->_contactID,
'contribution_page_id' => $this->_contributionPageID,
'payment_processor_id' => $this->_paymentProcessorID,
Expand Down Expand Up @@ -89,6 +88,80 @@ public function createRepeatMembershipOrder() {
$this->ids['Contribution'][0] = $orderID;
}

/**
* Create an order with more than one membership.
*
* @throws \CRM_Core_Exception
*/
protected function createMultipleMembershipOrder() {
$this->createExtraneousContribution();
$this->ids['contact'][0] = $this->individualCreate();
$this->ids['contact'][1] = $this->individualCreate();
$this->ids['membership_type'][0] = $this->membershipTypeCreate();
$this->ids['membership_type'][1] = $this->membershipTypeCreate(['name' => 'Type 2']);
$priceFieldID = $this->callAPISuccessGetValue('price_field', [
'return' => 'id',
'label' => 'Membership Amount',
'options' => ['limit' => 1, 'sort' => 'id DESC'],
]);
$generalPriceFieldValueID = $this->callAPISuccessGetValue('price_field_value', [
'return' => 'id',
'label' => 'General',
'options' => ['limit' => 1, 'sort' => 'id DESC'],
]);

$orderID = $this->callAPISuccess('Order', 'create', [
'total_amount' => 400,
'financial_type_id' => 'Member Dues',
'contact_id' => $this->_contactID,
'is_test' => 0,
'payment_instrument_id' => 'Check',
'receive_date' => '2019-07-25 07:34:23',
'line_items' => [
[
'params' => [
'contact_id' => $this->ids['contact'][0],
'membership_type_id' => $this->ids['membership_type'][0],
'source' => 'Payment',
],
'line_item' => [
[
'label' => 'General',
'qty' => 1,
'unit_price' => 200,
'line_total' => 200,
'financial_type_id' => 1,
'entity_table' => 'civicrm_membership',
'price_field_id' => $priceFieldID,
'price_field_value_id' => $generalPriceFieldValueID,
],
],
],
[
'params' => [
'contact_id' => $this->ids['contact'][1],
'membership_type_id' => $this->ids['membership_type'][0],
'source' => 'Payment',
],
'line_item' => [
[
'label' => 'General',
'qty' => 1,
'unit_price' => 200,
'line_total' => 200,
'financial_type_id' => 1,
'entity_table' => 'civicrm_membership',
'price_field_id' => $priceFieldID,
'price_field_value_id' => $generalPriceFieldValueID,
],
],
],
],
])['id'];

$this->ids['Contribution'][0] = $orderID;
}

/**
* Create an extraneous contribution to throw off any 'number one bugs'.
*
Expand Down
30 changes: 27 additions & 3 deletions tests/phpunit/api/v3/MembershipTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
* @group headless
*/
class api_v3_MembershipTest extends CiviUnitTestCase {
protected $_apiversion;

use CRMTraits_Financial_OrderTrait;

protected $_contactID;
protected $_membershipID;
protected $_membershipID2;
Expand All @@ -37,7 +39,6 @@ class api_v3_MembershipTest extends CiviUnitTestCase {
*/
public function setUp() {
parent::setUp();
$this->_apiversion = 3;
$this->_contactID = $this->individualCreate();
$this->_membershipTypeID = $this->membershipTypeCreate(['member_of_contact_id' => $this->_contactID]);
$this->_membershipTypeID2 = $this->membershipTypeCreate([
Expand Down Expand Up @@ -1006,7 +1007,7 @@ public function testMembershipUpdateCreateHookCRM15746() {
* @throws \Exception
*/
public function hook_civicrm_pre_update_create_membership($op, $objectName, $id, &$params) {
if ($objectName == 'Membership' && $op == 'edit') {
if ($objectName === 'Membership' && $op === 'edit') {
$existingMembership = $this->callAPISuccessGetSingle('membership', ['id' => $params['id']]);
unset($params['id'], $params['membership_id']);
$params['join_date'] = $params['membership_start_date'] = $params['start_date'] = date('Ymd000000', strtotime($existingMembership['start_date']));
Expand Down Expand Up @@ -1544,4 +1545,27 @@ public function testGetOptionsMembershipTypeID() {
$this->assertEquals(NULL, array_pop($options['values']));
}

/**
* Test that a contribution linked to multiple memberships results in all being updated.
*
* @throws \CRM_Core_Exception
* @throws \CiviCRM_API3_Exception
* @throws \Civi\Payment\Exception\PaymentProcessorException
*/
public function testMultipleMembershipContribution() {
$contactId1 = $this->individualCreate();

// Pending Payment Status
$this->createMultipleMembershipOrder();
$this->callAPISuccess('Payment', 'create', [
'contribution_id' => $this->ids['Contribution'][0],
'payment_instrument_id' => 'Check',
'total_amount' => 400,
]);

// check for Membership 1

$memberships = $this->callAPISuccess('membership', 'get')['values'];
}

}

0 comments on commit c9cf3eb

Please sign in to comment.