Remove legacy handling for 'fixing' line_item.entity_id

I tried to see what was happening with the test in https://github.com/civicrm/civicrm-core/pull/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. https://github.com/civicrm/civicrm-core/pull/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

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index f381e971fd6421d03babd9085e67d99f04aba021..b299fc7ad58c312e4000ef600c7c02b72205205c 100644 (file)
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -426,18 +426,9 @@ WHERE li.contribution_id = %1";
         }
         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

diff --git a/tests/phpunit/CRMTraits/Financial/OrderTrait.php b/tests/phpunit/CRMTraits/Financial/OrderTrait.php
index ea82bf58ebae43874d3b698f8e926f7af5313b4c..5bd1292a5665c87392ff70f8772f1dec8930f44f 100644 (file)
--- a/tests/phpunit/CRMTraits/Financial/OrderTrait.php
+++ b/tests/phpunit/CRMTraits/Financial/OrderTrait.php
@@ -45,7 +45,6 @@ trait CRMTraits_Financial_OrderTrait {
     $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,
@@ -89,6 +88,80 @@ trait CRMTraits_Financial_OrderTrait {
     $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'.
    *

diff --git a/tests/phpunit/api/v3/MembershipTest.php b/tests/phpunit/api/v3/MembershipTest.php
index bd956ebf6dfa554e050149d30f09f2c19139292d..7e03eee9a3e57dce63e529d1f3d84da0df2d2cba 100644 (file)
--- a/tests/phpunit/api/v3/MembershipTest.php
+++ b/tests/phpunit/api/v3/MembershipTest.php
@@ -21,7 +21,9 @@
  * @group headless
  */
 class api_v3_MembershipTest extends CiviUnitTestCase {
-  protected $_apiversion;
+
+  use CRMTraits_Financial_OrderTrait;
+
   protected $_contactID;
   protected $_membershipID;
   protected $_membershipID2;
@@ -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([
@@ -1006,7 +1007,7 @@ class api_v3_MembershipTest extends CiviUnitTestCase {
    * @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']));
@@ -1528,6 +1529,22 @@ class api_v3_MembershipTest extends CiviUnitTestCase {
 
   }
 
+  /**
+   * Test that a contribution linked to multiple memberships results in all being updated.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testMultipleMembershipContribution() {
+    $this->createMultipleMembershipOrder();
+    $this->callAPISuccess('Payment', 'create', [
+      'contribution_id' => $this->ids['Contribution'][0],
+      'payment_instrument_id' => 'Check',
+      'total_amount' => 400,
+    ]);
+    $memberships = $this->callAPISuccess('membership', 'get')['values'];
+    $this->assertCount(2, $memberships);
+  }
+
   /**
    * Test that all membership types are returned when getoptions is called.
    *