dev/core#35 Avoid variable leakage on recurring contribution receipts.

This covers pcpParams (which are tested & sets up the data set to extend to honor
params).

See https://lab.civicrm.org/dev/core/issues/35

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 974b2e6945d1d7a2fb0115b265a71a7043db89c1..80c5cfab1ae34cc6574d971d41cbd5237637529b 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -2895,6 +2895,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
    * @return mixed
    */
   public function _assignMessageVariablesToTemplate(&$values, $input, $returnMessageText = TRUE) {
+    // @todo - this should have a better separation of concerns - ie.
+    // gatherMessageValues should build an array of values to be assigned to the template
+    // and this function should assign them (assigning null if not set).
+    // the way the pcpParams & honor Params section works is a baby-step towards this.
     $template = CRM_Core_Smarty::singleton();
     $template->assign('first_name', $this->_relatedObjects['contact']->first_name);
     $template->assign('last_name', $this->_relatedObjects['contact']->last_name);
@@ -2907,6 +2911,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
     //assign honor information to receipt message
     $softRecord = CRM_Contribute_BAO_ContributionSoft::getSoftContribution($this->id);
 
+    $honorParams = ['soft_credit_type' => NULL, 'honor_block_is_active' => NULL];
     if (isset($softRecord['soft_credit'])) {
       //if id of contribution page is present
       if (!empty($values['id'])) {
@@ -2916,8 +2921,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
           'honor_id' => $softRecord['soft_credit'][1]['contact_id'],
         );
 
-        $template->assign('soft_credit_type', $softRecord['soft_credit'][1]['soft_credit_type_label']);
-        $template->assign('honor_block_is_active', CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFJoin', $values['id'], 'is_active', 'entity_id'));
+        $honorParams['soft_credit_type'] = $softRecord['soft_credit'][1]['soft_credit_type_label'];
+        $honorParams['honor_block_is_active'] = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFJoin', $values['id'], 'is_active', 'entity_id');
       }
       else {
         //offline contribution
@@ -2954,25 +2959,29 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
       $values['amount'] = $this->total_amount;
     }
 
-    // add the new contribution values
+    $pcpParams = ['pcpBlock' => NULL, 'pcp_display_in_roll' => NULL, 'pcp_roll_nickname' => NULL, 'pcp_personal_note' => NULL, 'title' => NULL];
+
     if (strtolower($this->_component) == 'contribute') {
       //PCP Info
       $softDAO = new CRM_Contribute_DAO_ContributionSoft();
       $softDAO->contribution_id = $this->id;
       if ($softDAO->find(TRUE)) {
-        $template->assign('pcpBlock', TRUE);
-        $template->assign('pcp_display_in_roll', $softDAO->pcp_display_in_roll);
-        $template->assign('pcp_roll_nickname', $softDAO->pcp_roll_nickname);
-        $template->assign('pcp_personal_note', $softDAO->pcp_personal_note);
+        $pcpParams['pcpBlock'] = TRUE;
+        $pcpParams['pcp_display_in_roll'] = $softDAO->pcp_display_in_roll;
+        $pcpParams['pcp_roll_nickname'] = $softDAO->pcp_roll_nickname;
+        $pcpParams['pcp_personal_note'] = $softDAO->pcp_personal_note;
 
         //assign the pcp page title for email subject
         $pcpDAO = new CRM_PCP_DAO_PCP();
         $pcpDAO->id = $softDAO->pcp_id;
         if ($pcpDAO->find(TRUE)) {
-          $template->assign('title', $pcpDAO->title);
+          $pcpParams['title'] = $pcpDAO->title;
         }
       }
     }
+    foreach (array_merge($honorParams, $pcpParams) as $templateKey => $templateValue) {
+      $template->assign($templateKey, $templateValue);
+    }
 
     if ($this->financial_type_id) {
       $values['financial_type_id'] = $this->financial_type_id;

diff --git a/CRM/Core/BAO/UFJoin.php b/CRM/Core/BAO/UFJoin.php
index f6df703442042914e2c5b8062d2bb528268dfcbf..b5442b60facffbafc5de37a5fc0ad63755bc6e71 100644 (file)
--- a/CRM/Core/BAO/UFJoin.php
+++ b/CRM/Core/BAO/UFJoin.php
@@ -54,7 +54,7 @@ class CRM_Core_BAO_UFJoin extends CRM_Core_DAO_UFJoin {
     }
 
     $dao = new CRM_Core_DAO_UFJoin();
-    $dao->copyValues($params);
+    $dao->copyValues($params, TRUE);
     if ($params['uf_group_id']) {
       $dao->save();
     }

diff --git a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
index 09385e4db92150cac8bf013980c6ba48feba7f76..300f6324ff83cce42129eb6e28c474f57b177209 100644 (file)
--- a/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
+++ b/tests/phpunit/CRM/Core/Payment/AuthorizeNetIPNTest.php
@@ -224,6 +224,15 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
     $mut = new CiviMailUtils($this, TRUE);
     $this->setupMembershipRecurringPaymentProcessorTransaction(array('is_email_receipt' => TRUE));
     $this->addProfile('supporter_profile', $this->_contributionPageID);
+    $this->addProfile('honoree_individual', $this->_contributionPageID, 'soft_credit');
+
+    $this->callAPISuccess('ContributionSoft', 'create', [
+      'contact_id' => $this->individualCreate(),
+      'contribution_id' => $this->_contributionID,
+      'soft_credit_type_id' => 'in_memory_of',
+      'amount' => 200,
+    ]);
+
     $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction());
     $IPN->main();
     $mut->checkAllMailLog(array(
@@ -235,14 +244,18 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
       'First Name: Anthony',
       'Last Name: Anderson',
       'Email Address: anthony_anderson@civicrm.org',
+      'Honor',
       'This membership will be automatically renewed every',
       'Dear Mr. Anthony Anderson II',
       'Thanks for your auto renew membership sign-up',
+      'In Memory of',
     ));
     $mut->clearMessages();
     $this->_contactID = $this->individualCreate(array('first_name' => 'Antonia', 'prefix_id' => 'Mrs.', 'email' => 'antonia_anderson@civicrm.org'));
     $this->_invoiceID = uniqid();
 
+    // Note, the second contribution is not in honor of anyone and the
+    // receipt should not mention honor at all.
     $this->setupMembershipRecurringPaymentProcessorTransaction(array('is_email_receipt' => TRUE));
     $IPN = new CRM_Core_Payment_AuthorizeNetIPN($this->getRecurTransaction(array('x_trans_id' => 'hers')));
     $IPN->main();
@@ -263,6 +276,14 @@ class CRM_Core_Payment_AuthorizeNetIPNTest extends CiviUnitTestCase {
       'Thanks for your auto renew membership sign-up',
     ));
 
+    $shouldNotBeInMailing = array(
+      'Honor',
+      'In Memory of',
+    );
+    $mails = $mut->getAllMessages('raw');
+    foreach ($mails as $mail) {
+      $mut->checkMailForStrings(array(), $shouldNotBeInMailing, '', $mail);
+    }
     $mut->stop();
     $mut->clearMessages();
   }

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index e7cfbbaf19a54bcd1bb29bb54ccbb64aea6b9563..b1be120c16bb8a79eb03710cc583592842154576 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3004,15 +3004,20 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
    *
    * @param string $name
    * @param int $contributionPageID
+   * @param string $module
    */
-  protected function addProfile($name, $contributionPageID) {
-    $this->callAPISuccess('UFJoin', 'create', array(
+  protected function addProfile($name, $contributionPageID, $module = 'CiviContribute') {
+    $params = [
       'uf_group_id' => $name,
-      'module' => 'CiviContribute',
+      'module' => $module,
       'entity_table' => 'civicrm_contribution_page',
       'entity_id' => $contributionPageID,
       'weight' => 1,
-    ));
+    ];
+    if ($module !== 'CiviContribute') {
+      $params['module_data'] = [$module => []];
+    }
+    $this->callAPISuccess('UFJoin', 'create', $params);
   }
 
   /**