CRM-21382 - Fix print/merge document on case and support multiple cases

I took the time to audit and remove the code blocks which had been previously commented as "silly but difficult to audit"

diff --git a/CRM/Contact/Form/Task/PDF.php b/CRM/Contact/Form/Task/PDF.php
index 217cf7af112168e731753350dc91433b09480977..82387229475b3ea5394a9ae01cfc3b81218739c0 100644 (file)
--- a/CRM/Contact/Form/Task/PDF.php
+++ b/CRM/Contact/Form/Task/PDF.php
@@ -58,10 +58,14 @@ class CRM_Contact_Form_Task_PDF extends CRM_Contact_Form_Task {
     CRM_Contact_Form_Task_PDFLetterCommon::preProcess($this);
 
     // store case id if present
-    $this->_caseId = CRM_Utils_Request::retrieve('caseid', 'Positive', $this, FALSE);
+    $this->_caseId = CRM_Utils_Request::retrieve('caseid', 'CommaSeparatedIntegers', $this, FALSE);
+    if (!empty($this->_caseId) && strpos($this->_caseId, ',')) {
+      $this->_caseIds = explode(',', $this->_caseId);
+      unset($this->_caseId);
+    }
 
     // retrieve contact ID if this is 'single' mode
-    $cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE);
+    $cid = CRM_Utils_Request::retrieve('cid', 'CommaSeparatedIntegers', $this, FALSE);
 
     if ($cid) {
       // this is true in non-search context / single mode
@@ -73,7 +77,6 @@ class CRM_Contact_Form_Task_PDF extends CRM_Contact_Form_Task {
     if ($cid) {
       CRM_Contact_Form_Task_PDFLetterCommon::preProcessSingle($this, $cid);
       $this->_single = TRUE;
-      $this->_cid = $cid;
     }
     else {
       parent::preProcess();
@@ -118,8 +121,9 @@ class CRM_Contact_Form_Task_PDF extends CRM_Contact_Form_Task {
    */
   public function listTokens() {
     $tokens = CRM_Core_SelectValues::contactTokens();
-    if (isset($this->_caseId)) {
-      $caseTypeId = CRM_Core_DAO::getFieldValue('CRM_Case_DAO_Case', $this->_caseId, 'case_type_id');
+    if (isset($this->_caseId) || isset($this->_caseIds)) {
+      // For a single case, list tokens relevant for only that case type
+      $caseTypeId = isset($this->_caseId) ? CRM_Core_DAO::getFieldValue('CRM_Case_DAO_Case', $this->_caseId, 'case_type_id') : NULL;
       $tokens += CRM_Core_SelectValues::caseTokens($caseTypeId);
     }
     return $tokens;

diff --git a/CRM/Contact/Form/Task/PDFLetterCommon.php b/CRM/Contact/Form/Task/PDFLetterCommon.php
index ce7e33a8f16046a75dcc7e04bf08449675d8fc9f..ae6d83ef53add7b93dc43473832af002da2c6e56 100644 (file)
--- a/CRM/Contact/Form/Task/PDFLetterCommon.php
+++ b/CRM/Contact/Form/Task/PDFLetterCommon.php
@@ -78,9 +78,11 @@ class CRM_Contact_Form_Task_PDFLetterCommon {
    * @param int $cid
    */
   public static function preProcessSingle(&$form, $cid) {
-    $form->_contactIds = array($cid);
+    $form->_contactIds = explode(',', $cid);
     // put contact display name in title for single contact mode
-    CRM_Utils_System::setTitle(ts('Print/Merge Document for %1', array(1 => CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $cid, 'display_name'))));
+    if (count($form->_contactIds) === 1) {
+      CRM_Utils_System::setTitle(ts('Print/Merge Document for %1', array(1 => CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $cid, 'display_name'))));
+    }
   }
 
   /**
@@ -369,12 +371,7 @@ class CRM_Contact_Form_Task_PDFLetterCommon {
 
     // CRM-16725 Skip creation of activities if user is previewing their PDF letter(s)
     if ($isLiveMode) {
-
-      // This seems silly, but the old behavior was to first check `_cid`
-      // and then use the provided `$contactIds`. Probably not even necessary,
-      // but difficult to audit.
-      $contactIds = $form->_cid ? array($form->_cid) : $form->_contactIds;
-      $activityIds = self::createActivities($form, $html_message, $contactIds, $formValues['subject'], CRM_Utils_Array::value('campaign_id', $formValues));
+      $activityIds = self::createActivities($form, $html_message, $form->_contactIds, $formValues['subject'], CRM_Utils_Array::value('campaign_id', $formValues));
     }
 
     if (!empty($formValues['document_file_path'])) {
@@ -506,10 +503,17 @@ class CRM_Contact_Form_Task_PDFLetterCommon {
 
       case 'multiple':
         // One activity per contact.
-        foreach ($contactIds as $contactId) {
+        foreach ($contactIds as $i => $contactId) {
           $fullParams = array(
             'target_contact_id' => $contactId,
           ) + $activityParams;
+          if (!empty($form->_caseId)) {
+            $fullParams['case_id'] = $form->_caseId;
+          }
+          elseif (!empty($form->_caseIds[$i])) {
+            $fullParams['case_id'] = $form->_caseIds[$i];
+          }
+
           if (isset($perContactHtml[$contactId])) {
             $fullParams['details'] = implode('<hr>', $perContactHtml[$contactId]);
           }
@@ -525,21 +529,20 @@ class CRM_Contact_Form_Task_PDFLetterCommon {
         $fullParams = array(
           'target_contact_id' => $contactIds,
         ) + $activityParams;
-        $activity = CRM_Activity_BAO_Activity::create($fullParams);
-        $activityIds[] = $activity->id;
+        if (!empty($form->_caseId)) {
+          $fullParams['case_id'] = $form->_caseId;
+        }
+        elseif (!empty($form->_caseIds)) {
+          $fullParams['case_id'] = $form->_caseIds;
+        }
+        $activity = civicrm_api3('Activity', 'create', $fullParams);
+        $activityIds[] = $activity['id'];
         break;
 
       default:
         throw new CRM_Core_Exception("Unrecognized option in recordGeneratedLetters: " . Civi::settings()->get('recordGeneratedLetters'));
     }
 
-    if (!empty($form->_caseId)) {
-      foreach ($activityIds as $activityId) {
-        $caseActivityParams = array('activity_id' => $activityId, 'case_id' => $form->_caseId);
-        CRM_Case_BAO_Case::processCaseActivity($caseActivityParams);
-      }
-    }
-
     return $activityIds;
   }
 

diff --git a/CRM/Contribute/Form/Task/PDFLetter.php b/CRM/Contribute/Form/Task/PDFLetter.php
index de0afcc2cbfd9ad7ebdf66245bdc0d49e5544b4e..e6b6677d2a7bc714104dafa114095a3e3d3a2693 100644 (file)
--- a/CRM/Contribute/Form/Task/PDFLetter.php
+++ b/CRM/Contribute/Form/Task/PDFLetter.php
@@ -54,17 +54,20 @@ class CRM_Contribute_Form_Task_PDFLetter extends CRM_Contribute_Form_Task {
     $this->skipOnHold = $this->skipDeceased = FALSE;
     CRM_Contact_Form_Task_PDFLetterCommon::preProcess($this);
     // store case id if present
-    $this->_caseId = CRM_Utils_Request::retrieve('caseid', 'Positive', $this, FALSE);
+    $this->_caseId = CRM_Utils_Request::retrieve('caseid', 'CommaSeparatedIntegers', $this, FALSE);
+    if (!empty($this->_caseId) && strpos($this->_caseId, ',')) {
+      $this->_caseIds = explode(',', $this->_caseId);
+      unset($this->_caseId);
+    }
 
     // retrieve contact ID if this is 'single' mode
-    $cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE);
+    $cid = CRM_Utils_Request::retrieve('cid', 'CommaSeparatedIntegers', $this, FALSE);
 
     $this->_activityId = CRM_Utils_Request::retrieve('id', 'Positive', $this, FALSE);
 
     if ($cid) {
       CRM_Contact_Form_Task_PDFLetterCommon::preProcessSingle($this, $cid);
       $this->_single = TRUE;
-      $this->_cid = $cid;
     }
     else {
       parent::preProcess();

diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php
index 87bb38225b22ea9f6b801e46b47daf4f6f55ed01..2aa925542ac0fd81237d08344e80f4f28cf92451 100644 (file)
--- a/CRM/Contribute/Form/Task/PDFLetterCommon.php
+++ b/CRM/Contribute/Form/Task/PDFLetterCommon.php
@@ -105,10 +105,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF
       }
     }
 
-    // This seems silly, but the old behavior was to first check `_cid`
-    // and then use the provided `$contactIds`. Probably not even necessary,
-    // but difficult to audit.
-    $contactIds = $form->_cid ? array($form->_cid) : array_keys($contacts);
+    $contactIds = array_keys($contacts);
     self::createActivities($form, $html_message, $contactIds, CRM_Utils_Array::value('subject', $formValues, ts('Thank you letter')), CRM_Utils_Array::value('campaign_id', $formValues), $contactHtml);
     $html = array_diff_key($html, $emailedHtml);
 

diff --git a/CRM/Member/Form/Task/PDFLetterCommon.php b/CRM/Member/Form/Task/PDFLetterCommon.php
index 47d2dd4e675ac9f6eaf4c062b0071b7ae9fb1156..7ec3c55fff8a66d3024cbf6c99d816bcb72567b5 100644 (file)
--- a/CRM/Member/Form/Task/PDFLetterCommon.php
+++ b/CRM/Member/Form/Task/PDFLetterCommon.php
@@ -31,10 +31,6 @@ class CRM_Member_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDFLett
         $html_message,
         $categories
       );
-    // This seems silly, but the old behavior was to first check `_cid`
-    // and then use the provided `$contactIds`. Probably not even necessary,
-    // but difficult to audit.
-    $contactIDs = $form->_cid ? array($form->_cid) : $contactIDs;
     self::createActivities($form, $html_message, $contactIDs, $formValues['subject'], CRM_Utils_Array::value('campaign_id', $formValues));
 
     CRM_Utils_PDF_Utils::html2pdf($html, "CiviLetter.pdf", FALSE, $formValues);

diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php
index c9a92fa96791f322945b35c3a2e7c7f8ddc40410..9492542464d4d08111fe0454bfce2c9e51b48d60 100644 (file)
--- a/CRM/Utils/Rule.php
+++ b/CRM/Utils/Rule.php
@@ -477,6 +477,20 @@ class CRM_Utils_Rule {
     return FALSE;
   }
 
+  /**
+   * @param $value
+   *
+   * @return bool
+   */
+  public static function commaSeparatedIntegers($value) {
+    foreach (explode(',', $value) as $val) {
+      if (!self::positiveInteger($val)) {
+        return FALSE;
+      }
+    }
+    return TRUE;
+  }
+
   /**
    * @param $value
    *

diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index 43546f5567a86e7e52f682693dbc65d0fe6c9f8c..b10fbc891621a1733a0d3b0ec759431e72590587 100644 (file)
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -412,6 +412,12 @@ class CRM_Utils_Type {
         }
         break;
 
+      case 'CommaSeparatedIntegers':
+        if (CRM_Utils_Rule::commaSeparatedIntegers($data)) {
+          return $data;
+        }
+        break;
+
       case 'Boolean':
         if (CRM_Utils_Rule::boolean($data)) {
           return $data;