[REF] Simplify determination on contribution recur contact id
authoreileen <emcnaughton@wikimedia.org>
Thu, 25 Mar 2021 03:11:35 +0000 (16:11 +1300)
committereileen <emcnaughton@wikimedia.org>
Fri, 26 Mar 2021 02:45:55 +0000 (15:45 +1300)
Instead of passing it around use a consistent method

CRM/Member/Form.php
CRM/Member/Form/Membership.php
CRM/Member/Form/MembershipRenewal.php
tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
tests/phpunit/CRM/Member/Form/MembershipTest.php

index 5b65b75372109b9e056ee880d9a3cd02117be327..c406c05bb177ee70c472e1e07ccd876cdeeb7d08 100644 (file)
@@ -329,20 +329,29 @@ class CRM_Member_Form extends CRM_Contribute_Form_AbstractEditPayment {
     }
 
     [$this->_memberDisplayName, $this->_memberEmail] = CRM_Contact_BAO_Contact_Location::getEmailDetails($this->_contactID);
-
+    $this->_contributorContactID = $this->getContributionContactID();
     //CRM-10375 Where the payer differs to the member the payer should get the email.
     // here we store details in order to do that
     if (!empty($formValues['soft_credit_contact_id'])) {
-      $this->_receiptContactId = $this->_contributorContactID = $formValues['soft_credit_contact_id'];
+      $this->_receiptContactId = $formValues['soft_credit_contact_id'];
       [$this->_contributorDisplayName, $this->_contributorEmail] = CRM_Contact_BAO_Contact_Location::getEmailDetails($this->_contributorContactID);
     }
     else {
-      $this->_receiptContactId = $this->_contributorContactID = $this->_contactID;
+      $this->_receiptContactId = $this->_contactID;
       $this->_contributorDisplayName = $this->_memberDisplayName;
       $this->_contributorEmail = $this->_memberEmail;
     }
   }
 
+  /**
+   * Get the contact id for the contribution.
+   *
+   * @return int
+   */
+  protected function getContributionContactID(): int {
+    return (int) ($this->getSubmittedValue('soft_credit_contact_id') ?: $this->getSubmittedValue('contact_id'));
+  }
+
   /**
    * Set variables in a way that can be accessed from different places.
    *
@@ -494,7 +503,12 @@ class CRM_Member_Form extends CRM_Contribute_Form_AbstractEditPayment {
    *
    * @param array $formValues
    */
-  public function testSubmit(array $formValues): void {
+  public function testSubmit(array $formValues = []): void {
+    if (empty($formValues)) {
+      // If getForm is used these will be set - this is now
+      // preferred.
+      $formValues = $this->controller->exportValues($this->_name);
+    }
     $this->exportedValues = $formValues;
     $this->setContextVariables($formValues);
     $this->_memType = !empty($formValues['membership_type_id']) ? $formValues['membership_type_id'][1] : NULL;
index 31bcac4792c63695836c72585139c97ae02cf1e7..d6779d6de492ff2b77fed0d0c296dee349b965fa 100644 (file)
@@ -1771,8 +1771,7 @@ DESC limit 1");
     $params,
     $contributionParams
   ) {
-    $contactID = $contributionParams['contact_id'];
-    $contributionParams['contribution_recur_id'] = $this->legacyProcessRecurringContribution($params, $contactID);
+    $contributionParams['contribution_recur_id'] = $this->legacyProcessRecurringContribution($params);
     return CRM_Contribute_BAO_Contribution::add($contributionParams);
   }
 
@@ -1782,16 +1781,15 @@ DESC limit 1");
    * This function was copied from another form & needs cleanup.
    *
    * @param array $params
-   * @param int $contactID
    *
    * @return int|null
    * @throws \CiviCRM_API3_Exception
    */
-  protected function legacyProcessRecurringContribution(array $params, $contactID): ?int {
+  protected function legacyProcessRecurringContribution(array $params): ?int {
     if (!$this->isCreateRecurringContribution()) {
       return NULL;
     }
-    $recurParams = ['contact_id' => $contactID];
+    $recurParams = ['contact_id' => $this->getContributionContactID()];
     $recurParams['amount'] = $this->order->getTotalAmount();
     $recurParams['auto_renew'] = $params['auto_renew'] ?? NULL;
     $recurParams['frequency_unit'] = $params['frequency_unit'] ?? NULL;
index ed017fdb28c2cf8f707729f7e68c4d780293f6d5..afe70a28ab6f9de9765eb8a929a402f89d29708e 100644 (file)
@@ -458,7 +458,7 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function postProcess() {
+  public function postProcess(): void {
     // get the submitted form values.
     $this->_params = $this->controller->exportValues($this->_name);
     $this->assignBillingName();
index 0b4963ff81c717c2c2fa2f5332eeb2a7eca19b46..2c9c3c00134b77ddcffdeba2b2af0b5de745d6a2 100644 (file)
@@ -140,10 +140,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmit(): void {
-    $form = $this->getForm();
     $loggedInUserID = $this->createLoggedInUser();
     $loggedInUserDisplayName = Contact::get()->addWhere('id', '=', $loggedInUserID)->addSelect('display_name')->execute()->first()['display_name'];
     $params = $this->getBaseSubmitParams();
+    $form = $this->getForm(array_merge($params, ['total_amount' => 50]));
     $form->_contactID = $this->_individualId;
 
     $form->testSubmit(array_merge($params, ['total_amount' => 50]));
@@ -190,10 +190,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
   public function testSubmitWithTax(): void {
     $this->enableTaxAndInvoicing();
     $this->addTaxAccountToFinancialType($this->financialTypeID);
-    $form = $this->getForm();
-    $form->testSubmit(array_merge($this->getBaseSubmitParams(), [
+    $form = $this->getForm(array_merge($this->getBaseSubmitParams(), [
       'total_amount' => '50.00',
     ]));
+    $form->testSubmit();
     $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $this->_individualId, 'is_test' => TRUE, 'return' => ['total_amount', 'tax_amount']]);
     $this->assertEquals(50, $contribution['total_amount']);
     $this->assertEquals(4.55, $contribution['tax_amount']);
@@ -257,7 +257,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitRecur(): void {
-    $form = $this->getForm();
 
     $this->callAPISuccess('MembershipType', 'create', [
       'id' => $this->membershipTypeAnnualFixedID,
@@ -265,10 +264,8 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'duration_interval' => 1,
       'auto_renew' => TRUE,
     ]);
-    $form->preProcess();
-    $this->createLoggedInUser();
-    $params = [
-      'cid' => $this->_individualId,
+    $form = $this->getForm([
+      'contact_id' => $this->_individualId,
       'price_set_id' => 0,
       'join_date' => CRM_Utils_Time::date('m/d/Y'),
       'start_date' => '',
@@ -290,7 +287,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'cvv2' => '123',
       'credit_card_exp_date' => [
         'M' => '9',
-        'Y' => CRM_Utils_Time::date('Y') + 1,
+        'Y' => (int) CRM_Utils_Time::date('Y') + 1,
       ],
       'credit_card_type' => 'Visa',
       'billing_first_name' => 'Test',
@@ -301,11 +298,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'billing_postal_code-5' => '90210',
       'billing_country_id-5' => '1228',
       'send_receipt' => 1,
-    ];
+    ]);
+    $this->createLoggedInUser();
     $form->_mode = 'test';
     $form->_contactID = $this->_individualId;
 
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['contact_id' => $this->_individualId]);
     $this->assertEquals(1, $contributionRecur['is_email_receipt']);
@@ -349,7 +347,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitRecurCompleteInstant(): void {
-    $form = $this->getForm();
     /** @var \CRM_Core_Payment_Dummy $processor */
     $processor = Civi\Payment\System::singleton()->getById($this->_paymentProcessorID);
     $processor->setDoDirectPaymentResult([
@@ -365,13 +362,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'auto_renew' => TRUE,
     ]);
     $this->createLoggedInUser();
-    $form->preProcess();
+    $form = $this->getForm(array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'auto_renew' => '1']));
 
     $form->_contactID = $this->_individualId;
-    $params = array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'auto_renew' => '1']);
     $form->_mode = 'test';
 
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->assertEquals('2020-04-13', $membership['join_date']);
     $this->assertEquals(CRM_Utils_Time::date('Y-01-01'), $membership['start_date']);
@@ -430,7 +426,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \API_Exception
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
-   * @throws \Civi\API\Exception\UnauthorizedException
    * @dataProvider getThousandSeparators
    */
   public function testSubmitRecurCompleteInstantWithMail(string $thousandSeparator): void {
@@ -438,7 +433,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     // Visibility is 'Public Pages and Listings' in order to try to get to a specific line
     // of code to ensure it's tested - it might not be a 'real' use case.
     $this->createCustomGroupWithFieldOfType(['extends' => 'Membership'], 'multi_country', NULL, ['visibility' => 'Public Pages and Listings']);
-    $form = $this->getForm();
     $this->mut = new CiviMailUtils($this, TRUE);
     /** @var \CRM_Core_Payment_Dummy $processor */
     $processor = Civi\Payment\System::singleton()->getById($this->_paymentProcessorID);
@@ -455,17 +449,17 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'auto_renew' => TRUE,
     ]);
     $this->createLoggedInUser();
-    $form->preProcess();
-
-    $form->_contactID = $this->_individualId;
-    $form->_mode = 'live';
-
-    $form->testSubmit(array_merge($this->getBaseSubmitParams(), [
+    $form = $this->getForm(array_merge($this->getBaseSubmitParams(), [
       'is_recur' => 1,
       'send_receipt' => 1,
       'auto_renew' => 1,
       $this->getCustomFieldName('multi_country') => [1006, 1007],
     ]));
+
+    $form->_contactID = $this->_individualId;
+    $form->_mode = 'live';
+
+    $form->testSubmit();
     $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['contact_id' => $this->_individualId]);
     $this->assertEquals(1, $contributionRecur['is_email_receipt']);
     $this->mut->checkMailLog([
@@ -483,11 +477,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitPayLater(): void {
-    $form = $this->getForm(NULL);
     $this->createLoggedInUser();
     $originalMembership = $this->callAPISuccessGetSingle('membership', []);
-    $params = [
-      'cid' => $this->_individualId,
+    $form = $this->getForm([
+      'contact_id' => $this->_individualId,
       'join_date' => CRM_Utils_Time::date('m/d/Y'),
       'start_date' => '',
       'end_date' => '',
@@ -505,10 +498,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'record_contribution' => TRUE,
       'trxn_id' => 777,
       'contribution_status_id' => 2,
-    ];
+    ], NULL);
     $form->_contactID = $this->_individualId;
 
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->assertEquals(strtotime($membership['end_date']), strtotime($originalMembership['end_date']));
     $contribution = $this->callAPISuccessGetSingle('Contribution', [
@@ -533,11 +526,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitPayLaterWithBilling(): void {
-    $form = $this->getForm(NULL);
     $this->createLoggedInUser();
     $originalMembership = $this->callAPISuccessGetSingle('membership', []);
-    $params = [
-      'cid' => $this->_individualId,
+    $form = $this->getForm([
+      'contact_id' => $this->_individualId,
       'start_date' => '',
       'end_date' => '',
       // This format reflects the first value being the organisation & the second being the type.
@@ -561,10 +553,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'billing_state_province_id-5' => '1003',
       'billing_postal_code-5' => '90210',
       'billing_country_id-5' => '1228',
-    ];
+    ], NULL);
     $form->_contactID = $this->_individualId;
 
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->assertEquals(strtotime($membership['end_date']), strtotime($originalMembership['end_date']));
     $this->assertEquals(10, $membership['max_related']);
@@ -594,11 +586,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitComplete(): void {
-    $form = $this->getForm(NULL);
     $this->createLoggedInUser();
     $originalMembership = $this->callAPISuccessGetSingle('membership', []);
-    $params = [
-      'cid' => $this->_individualId,
+    $form = $this->getForm([
+      'contact_id' => $this->_individualId,
       'join_date' => CRM_Utils_Time::date('m/d/Y'),
       'start_date' => '',
       'end_date' => '',
@@ -617,10 +608,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'trxn_id' => 777,
       'contribution_status_id' => 1,
       'fee_amount' => .5,
-    ];
+    ], NULL);
     $form->_contactID = $this->_individualId;
 
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->assertEquals(strtotime($membership['end_date']), strtotime('+ 2 years',
       strtotime($originalMembership['end_date'])));
@@ -643,6 +634,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    *
    * We need to instantiate the form to run preprocess, which means we have to trick it about the request method.
    *
+   * @param array $formValues
    * @param string $mode
    *
    * @return \CRM_Member_Form_MembershipRenewal
@@ -650,10 +642,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  protected function getForm($mode = 'test'): CRM_Member_Form_MembershipRenewal {
-    $form = new CRM_Member_Form_MembershipRenewal();
-    $_SERVER['REQUEST_METHOD'] = 'GET';
-    $form->controller = new CRM_Core_Controller();
+  protected function getForm($formValues = [], $mode = 'test'): CRM_Member_Form_MembershipRenewal {
+    /* @var CRM_Member_Form_MembershipRenewal $form */
+    $form = $this->getFormObject('CRM_Member_Form_MembershipRenewal', $formValues);
+
     $form->_bltID = 5;
     $form->_mode = $mode;
     $form->setEntityId($this->_membershipID);
@@ -669,6 +661,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
   protected function getBaseSubmitParams(): array {
     return [
       'cid' => $this->_individualId,
+      'contact_id' => $this->_individualId,
       // This format reflects the key being the organisation & the value being the type.
       'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'num_terms' => '1',
@@ -682,7 +675,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'cvv2' => '123',
       'credit_card_exp_date' => [
         'M' => '9',
-        'Y' => CRM_Utils_Time::date('Y') + 1,
+        'Y' => (int) CRM_Utils_Time::date('Y') + 1,
       ],
       'credit_card_type' => 'Visa',
       'billing_first_name' => 'Test',
@@ -702,7 +695,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    */
   public function testSubmitRenewExpired(): void {
-    $form = $this->getForm(NULL);
+    $form = $this->getForm([], NULL);
     $this->createLoggedInUser();
     $originalMembership = $this->callAPISuccessGetSingle('membership', []);
     $this->callAPISuccess('Membership', 'create', [
index c85d66598e4fb96157854b0eb42f52c029184c95..0e75800d68db091d5e88af3ce20dab89ff03b97e 100644 (file)
@@ -487,7 +487,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    *
    * @dataProvider getThousandSeparators
    */
-  public function testSubmit(string $thousandSeparator) {
+  public function testSubmit(string $thousandSeparator): void {
     CRM_Core_Session::singleton()->getStatus(TRUE);
     $this->setCurrencySeparators($thousandSeparator);
     $form = $this->getForm();
@@ -496,6 +496,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
     $this->createLoggedInUser();
     $params = [
       'cid' => $this->_individualId,
+      'contact_id' => $this->_individualId,
       'join_date' => date('Y-m-d'),
       'start_date' => '',
       'end_date' => '',
@@ -786,7 +787,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
     $params = $this->getBaseSubmitParams();
     // Change financial_type_id to test our override flows through to the line item.
     $params['financial_type_id'] = FinancialType::get(FALSE)->addWhere('id', '!=', $params['financial_type_id'])->addSelect('id')->execute()->first()['id'];
-    $form = $this->getForm();
+    $form = $this->getForm($params);
     $this->createLoggedInUser();
     $form->_mode = 'test';
     $form->_contactID = $this->_individualId;
@@ -905,12 +906,11 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    *
    * @throws \CRM_Core_Exception
    */
-  public function testSubmitPayLaterWithBilling() {
+  public function testSubmitPayLaterWithBilling(): void {
     $form = $this->getForm();
-    $form->preProcess();
     $this->createLoggedInUser();
     $params = [
-      'cid' => $this->_individualId,
+      'contact_id' => $this->_individualId,
       'join_date' => date('Y-m-d'),
       'start_date' => '',
       'end_date' => '',
@@ -933,7 +933,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
       'trxn_id' => 777,
       'contribution_status_id' => 2,
       'billing_first_name' => 'Test',
-      'billing_middlename' => 'Last',
+      'billing_middle_name' => 'Last',
       'billing_street_address-5' => '10 Test St',
       'billing_city-5' => 'Test',
       'billing_state_province_id-5' => '1003',
@@ -1010,9 +1010,9 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    * @throws \CiviCRM_API3_Exception
    * @throws \CRM_Core_Exception
    */
-  public function testSubmitRecurCompleteInstant() {
-    $form = $this->getForm();
+  public function testSubmitRecurCompleteInstant(): void {
     $mut = new CiviMailUtils($this, TRUE);
+    /* @var \CRM_Core_Payment_Dummy $processor */
     $processor = Civi\Payment\System::singleton()->getById($this->_paymentProcessorID);
     $processor->setDoDirectPaymentResult([
       'payment_status_id' => 1,
@@ -1026,12 +1026,11 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
       'duration_interval' => 1,
       'auto_renew' => TRUE,
     ]);
-    $form->preProcess();
+    $form = $this->getForm($this->getBaseSubmitParams());
     $this->createLoggedInUser();
-    $params = $this->getBaseSubmitParams();
     $form->_mode = 'test';
     $form->_contactID = $this->_individualId;
-    $form->testSubmit($params);
+    $form->testSubmit();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->callAPISuccessGetCount('ContributionRecur', ['contact_id' => $this->_individualId], 1);
 
@@ -1053,7 +1052,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
       '===========================================================
 Billing Name and Address
 ===========================================================
-Test
+Test Last
 10 Test St
 Test, AR 90210
 US',
@@ -1218,15 +1217,16 @@ Expires: ',
    * We need to instantiate the form to run preprocess, which means we have to
    * trick it about the request method.
    *
+   * @param array $formValues
+   *
    * @return \CRM_Member_Form_Membership
    * @throws \CRM_Core_Exception
-   * @throws \CiviCRM_API3_Exception
    */
-  protected function getForm() {
+  protected function getForm($formValues = []) {
     if (isset($_REQUEST['cid'])) {
       unset($_REQUEST['cid']);
     }
-    $form = $this->getFormObject('CRM_Member_Form_Membership');
+    $form = $this->getFormObject('CRM_Member_Form_Membership', $formValues);
     $form->preProcess();
     return $form;
   }
@@ -1234,8 +1234,9 @@ Expires: ',
   /**
    * @return array
    */
-  protected function getBaseSubmitParams() {
-    $params = [
+  protected function getBaseSubmitParams(): array {
+    return [
+      'contact_id' => $this->_individualId,
       'cid' => $this->_individualId,
       'price_set_id' => 0,
       'join_date' => date('Y-m-d'),
@@ -1265,7 +1266,7 @@ Expires: ',
       ],
       'credit_card_type' => 'Visa',
       'billing_first_name' => 'Test',
-      'billing_middlename' => 'Last',
+      'billing_middle_name' => 'Last',
       'billing_street_address-5' => '10 Test St',
       'billing_city-5' => 'Test',
       'billing_state_province_id-5' => '1003',
@@ -1273,7 +1274,6 @@ Expires: ',
       'billing_country_id-5' => '1228',
       'send_receipt' => 1,
     ];
-    return $params;
   }
 
   /**