From 3280f32744cc8143012aeb9ea5e877701a1dd89e Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 15 May 2017 18:47:59 +1200 Subject: [PATCH] CRM-20577 Fix activities to record actual html if activity per contact (At least, this covers contributions at this stage). I have also tried to move towards clearer inputs & outpus on createActivities, with the weirdness around contactIds being moved up to the forms themselves and not being passed in. Would prefer not to see form passed in either but, next time. Unit tests cover emails & activity create (for the first time) --- CRM/Contact/Form/Task/PDFLetterCommon.php | 48 +++++++++---------- CRM/Contribute/Form/Task/PDFLetterCommon.php | 27 +++++------ CRM/Member/Form/Task/PDFLetterCommon.php | 6 ++- .../Form/Task/PDFLetterCommonTest.php | 30 ++++++++---- 4 files changed, 62 insertions(+), 49 deletions(-) diff --git a/CRM/Contact/Form/Task/PDFLetterCommon.php b/CRM/Contact/Form/Task/PDFLetterCommon.php index 21614edf97..71df969cb3 100644 --- a/CRM/Contact/Form/Task/PDFLetterCommon.php +++ b/CRM/Contact/Form/Task/PDFLetterCommon.php @@ -365,7 +365,12 @@ class CRM_Contact_Form_Task_PDFLetterCommon { // CRM-16725 Skip creation of activities if user is previewing their PDF letter(s) if ($buttonName == '_qf_PDF_upload') { - $activityIds = self::createActivities($form, $html_message, $form->_contactIds); + + // 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)); } if (!empty($formValues['document_file_path'])) { @@ -465,6 +470,10 @@ class CRM_Contact_Form_Task_PDFLetterCommon { * @param CRM_Core_Form $form * @param string $html_message * @param array $contactIds + * @param string $subject + * @param int $campaign_id + * @param array $perContactHtml + * * @return array * List of activity IDs. * There may be 1 or more, depending on the system-settings @@ -472,22 +481,13 @@ class CRM_Contact_Form_Task_PDFLetterCommon { * * @throws CRM_Core_Exception */ - public static function createActivities($form, $html_message, $contactIds) { - //Added for CRM-12682: Add activity subject and campaign fields - $formValues = $form->controller->exportValues($form->getName()); + public static function createActivities($form, $html_message, $contactIds, $subject, $campaign_id, $perContactHtml = array()) { - $session = CRM_Core_Session::singleton(); - $userID = $session->get('userID'); - $activityTypeID = CRM_Core_OptionGroup::getValue( - 'activity_type', - 'Print PDF Letter', - 'name' - ); $activityParams = array( - 'subject' => $formValues['subject'], - 'campaign_id' => CRM_Utils_Array::value('campaign_id', $formValues), - 'source_contact_id' => $userID, - 'activity_type_id' => $activityTypeID, + 'subject' => $subject, + 'campaign_id' => $campaign_id, + 'source_contact_id' => CRM_Core_Session::singleton()->getLoggedInContactID(), + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Print PDF Letter'), 'activity_date_time' => date('YmdHis'), 'details' => $html_message, ); @@ -495,11 +495,6 @@ class CRM_Contact_Form_Task_PDFLetterCommon { $activityParams += array('id' => $form->_activityId); } - // 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; - $activityIds = array(); switch (Civi::settings()->get('recordGeneratedLetters')) { case 'none': @@ -511,8 +506,11 @@ class CRM_Contact_Form_Task_PDFLetterCommon { $fullParams = array( 'target_contact_id' => $contactId, ) + $activityParams; - $activity = CRM_Activity_BAO_Activity::create($fullParams); - $activityIds[$contactId] = $activity->id; + if (isset($perContactHtml[$contactId])) { + $fullParams['details'] = implode('
', $perContactHtml[$contactId]); + } + $activity = civicrm_api3('Activity', 'create', $fullParams); + $activityIds[$contactId] = $activity['id']; } break; @@ -612,12 +610,12 @@ class CRM_Contact_Form_Task_PDFLetterCommon { * @return array */ protected static function getTokenCategories() { - if (self::$tokenCategories === NULL) { + if (!isset(Civi::$statics[__CLASS__]['token_categories'])) { $tokens = array(); CRM_Utils_Hook::tokens($tokens); - self::$tokenCategories = array_keys($tokens); + Civi::$statics[__CLASS__]['token_categories'] = array_keys($tokens); } - return self::$tokenCategories; + return Civi::$statics[__CLASS__]['token_categories']; } } diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php index 681c07590c..87bb38225b 100644 --- a/CRM/Contribute/Form/Task/PDFLetterCommon.php +++ b/CRM/Contribute/Form/Task/PDFLetterCommon.php @@ -65,7 +65,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF } list($contributions, $contacts) = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->_includesSoftCredits); $html = array(); - $contactHtml = array(); + $contactHtml = $emailedHtml = array(); foreach ($contributions as $contributionId => $contribution) { $contact = &$contacts[$contribution['contact_id']]; $grouped = FALSE; @@ -105,17 +105,17 @@ 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); + 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); if (!empty($formValues['is_unit_test'])) { return $html; } - //createActivities requires both $form->_contactIds and $contacts - - //@todo - figure out why - $form->_contactIds = array_keys($contacts); - self::createActivities($form, $html_message, $form->_contactIds); - - $html = array_diff_key($html, $emailedHtml); //CRM-19761 if (!empty($html)) { $type = $formValues['document_type']; @@ -197,14 +197,14 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF * @param array $contact * @param array $contribution * @param array $messageToken - * @param array $categories * @param bool $grouped * Does this letter represent more than one contribution. * @param string $separator * What is the preferred letter separator. * @return string */ - private static function resolveTokens($html_message, $contact, $contribution, $messageToken, $categories, $grouped, $separator) { + private static function resolveTokens($html_message, $contact, $contribution, $messageToken, $grouped, $separator) { + $categories = self::getTokenCategories(); $tokenHtml = CRM_Utils_Token::replaceContactTokens($html_message, $contact, TRUE, $messageToken); if ($grouped) { $tokenHtml = CRM_Utils_Token::replaceMultipleContributionTokens($separator, $tokenHtml, $contribution, TRUE, $messageToken); @@ -240,10 +240,10 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF * @return array */ public static function buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $isIncludeSoftCredits) { - $contributions = $contacts = $notSent = array(); + $contributions = $contacts = array(); foreach ($contributionIDs as $item => $contributionId) { - // basic return attributes needed, see below for there usage - $returnValues = array('contact_id', 'total_amount', 'contribution_campaign_title'); + // Basic return attributes available to the template. + $returnValues = array('contact_id', 'total_amount', 'financial_type', 'receive_date', 'contribution_campaign_title'); if (!empty($messageToken['contribution'])) { $returnValues = array_merge($messageToken['contribution'], $returnValues); } @@ -411,7 +411,6 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF static $validated = FALSE; $html = NULL; - $categories = self::getTokenCategories(); self::assignCombinedContributionValues($contact, $contributions, $groupBy, $groupByID); if (empty($groupBy) || empty($contact['is_sent'][$groupBy][$groupByID])) { @@ -420,7 +419,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF CRM_Core_Session::setStatus(ts('You have selected the table cell separator, but one or more token fields are not placed inside a table cell. This would result in invalid HTML, so comma separators have been used instead.')); } $validated = TRUE; - $html = str_replace($separator, $realSeparator, self::resolveTokens($html_message, $contact, $contribution, $messageToken, $categories, $grouped, $separator)); + $html = str_replace($separator, $realSeparator, self::resolveTokens($html_message, $contact, $contribution, $messageToken, $grouped, $separator)); } return $html; diff --git a/CRM/Member/Form/Task/PDFLetterCommon.php b/CRM/Member/Form/Task/PDFLetterCommon.php index 320303ba3f..47d2dd4e67 100644 --- a/CRM/Member/Form/Task/PDFLetterCommon.php +++ b/CRM/Member/Form/Task/PDFLetterCommon.php @@ -31,7 +31,11 @@ class CRM_Member_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDFLett $html_message, $categories ); - self::createActivities($form, $html_message, $contactIDs); + // 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/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php index 110e93af5c..93faf4db3b 100644 --- a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php @@ -63,6 +63,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { */ public function tearDown() { $this->quickCleanUpFinancialEntities(); + $this->quickCleanup(array('civicrm_uf_match')); CRM_Utils_Hook::singleton()->reset(); } @@ -146,6 +147,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { * html returned by postProcess function. */ public function testPostProcess() { + $this->createLoggedInUser(); $this->_individualId = $this->individualCreate(); foreach (array('docx', 'odt') as $docType) { $formValues = array( @@ -191,6 +193,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { * recent contact rather than a total aggregate, since we are using group by. */ public function testPostProcessGroupByContact() { + $this->createLoggedInUser(); $this->hookClass->setHook('civicrm_tokenValues', array($this, 'hook_aggregateTokenValues')); $this->hookClass->setHook('civicrm_tokens', array($this, 'hook_tokens')); $this->mut = new CiviMailUtils($this, TRUE); @@ -210,12 +213,14 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { 'contact_id' => $this->_individualId, 'total_amount' => 100, 'financial_type_id' => 'Donation', + 'receive_date' => '2016-12-25', )); $contributionIDs[] = $contribution['id']; $contribution = $this->callAPISuccess('Contribution', 'create', array( 'contact_id' => $this->_individualId2, 'total_amount' => 10, 'financial_type_id' => 'Donation', + 'receive_date' => '2016-12-25', )); $contributionIDs[] = $contribution['id']; @@ -223,6 +228,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { 'contact_id' => $this->_individualId2, 'total_amount' => 1, 'financial_type_id' => 'Donation', + 'receive_date' => '2016-12-25', )); $contributionIDs[] = $contribution['id']; @@ -241,9 +247,9 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { - + 25 December 2016 $ 100.00 - + Donation - + 25 December 2016 $ 10.00 - + Donation - + 25 December 2016 $ 1.00 - + Donation