Skip to content

Commit

Permalink
CRM-20577 Fix activities to record actual html if activity per contact
Browse files Browse the repository at this point in the history 
(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)
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed May 17, 2017
1 parent 5936942 commit b177614
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 48 deletions.
48 changes: 23 additions & 25 deletions CRM/Contact/Form/Task/PDFLetterCommon.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,12 @@ public static function postProcess(&$form) {

// 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'])) {
Expand Down Expand Up @@ -465,41 +470,31 @@ public static function postProcess(&$form) {
* @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
* and use-case.
*
* @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,
);
if (!empty($form->_activityId)) {
$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':
Expand All @@ -511,8 +506,11 @@ public static function createActivities($form, $html_message, $contactIds) {
$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('<hr>', $perContactHtml[$contactId]);
}
$activity = civicrm_api3('Activity', 'create', $fullParams);
$activityIds[$contactId] = $activity['id'];
}

break;
Expand Down Expand Up @@ -612,12 +610,12 @@ private static function getMimeType($type) {
* @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'];
}

}
24 changes: 11 additions & 13 deletions CRM/Contribute/Form/Task/PDFLetterCommon.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,16 @@ public static function postProcess(&$form, $formValues = NULL) {
}
}

// 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);

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)) {
Expand Down Expand Up @@ -197,14 +197,14 @@ public static function isHtmlTokenInTableCell($token, $entity, $textToSearch) {
* @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);
Expand Down Expand Up @@ -240,12 +240,11 @@ private static function resolveTokens($html_message, $contact, $contribution, $m
* @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) {
// get contribution information

// basic return attributes needed, see below for there usage
$returnValues = array('contact_id', 'total_amount');
// Basic return attributes available to the template.
$returnValues = array('contact_id', 'total_amount', 'financial_type', 'receive_date');
if (!empty($messageToken['contribution'])) {
$returnValues = array_merge($messageToken['contribution'], $returnValues);
}
Expand Down Expand Up @@ -412,7 +411,6 @@ protected static function generateHtml(&$contact, $contribution, $groupBy, $cont
static $validated = FALSE;
$html = NULL;

$categories = self::getTokenCategories();
self::assignCombinedContributionValues($contact, $contributions, $groupBy, $groupByID);

if (empty($groupBy) || empty($contact['is_sent'][$groupBy][$groupByID])) {
Expand All @@ -421,7 +419,7 @@ protected static function generateHtml(&$contact, $contribution, $groupBy, $cont
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;
Expand Down
6 changes: 5 additions & 1 deletion CRM/Member/Form/Task/PDFLetterCommon.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ public static function postProcessMembers(&$form, $membershipIDs, $skipOnHold, $
$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);

Expand Down
30 changes: 21 additions & 9 deletions tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ protected function setUp() {
*/
public function tearDown() {
$this->quickCleanUpFinancialEntities();
$this->quickCleanup(array('civicrm_uf_match'));
CRM_Utils_Hook::singleton()->reset();
}

Expand Down Expand Up @@ -142,6 +143,7 @@ public function hookTokenValues(&$details, $contactIDs, $jobID, $tokens, $classN
* html returned by postProcess function.
*/
public function testPostProcess() {
$this->createLoggedInUser();
$this->_individualId = $this->individualCreate();
foreach (array('docx', 'odt') as $docType) {
$formValues = array(
Expand Down Expand Up @@ -187,6 +189,7 @@ public function testPostProcess() {
* 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);
Expand All @@ -206,19 +209,22 @@ public function testPostProcessGroupByContact() {
'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'];

$contribution = $this->callAPISuccess('Contribution', 'create', array(
'contact_id' => $this->_individualId2,
'total_amount' => 1,
'financial_type_id' => 'Donation',
'receive_date' => '2016-12-25',
));
$contributionIDs[] = $contribution['id'];

Expand All @@ -237,9 +243,9 @@ public function testPostProcessGroupByContact() {
<!--
-->
<tr>
<td></td>
<td>25 December 2016</td>
<td>$ 100.00</td>
<td></td>
<td>Donation</td>
<td></td>
</tr>
<!--
Expand All @@ -263,17 +269,17 @@ public function testPostProcessGroupByContact() {
<!--
-->
<tr>
<td></td>
<td>25 December 2016</td>
<td>$ 10.00</td>
<td></td>
<td>Donation</td>
<td></td>
</tr>
<!--
-->
<tr>
<td></td>
<td>25 December 2016</td>
<td>$ 1.00</td>
<td></td>
<td>Donation</td>
<td></td>
</tr>
<!--
Expand All @@ -286,16 +292,22 @@ public function testPostProcessGroupByContact() {
</tr>
</tbody>
</table>", $html[2]);

$activities = $this->callAPISuccess('Activity', 'get', array('activity_type_id' => 'Print PDF Letter', 'sequential' => 1));
$this->assertEquals(2, $activities['count']);
$this->assertEquals($html[1], $activities['values'][0]['details']);
$this->assertEquals($html[2], $activities['values'][1]['details']);
// Checking it is not called multiple times.
// once for each contact create + once for the activities.
$this->assertEquals(3, $this->hookTokensCalled);
$this->mut->checkAllMailLog($html);

}

/**
* Implements civicrm_tokens().
*/
function hook_tokens(&$tokens) {
public function hook_tokens(&$tokens) {
$this->hookTokensCalled++;
$tokens['aggregate'] = array('rendered_token' => 'rendered_token');
}
Expand Down Expand Up @@ -326,7 +338,7 @@ public function getHtmlMessage() {
<td>{$date}</td>
<td>{$contribution.total_amount|crmMoney}</td>
<td>{$contribution.financial_type}</td>
<td>{$contribution.source}</td>
<td></td>
</tr>
<!--
{/if}
Expand All @@ -351,7 +363,7 @@ public function getHtmlMessage() {
* @param array $tokens
* @param null $context
*/
function hook_aggregateTokenValues(&$values, $contactIDs, $job = NULL, $tokens = array(), $context = NULL) {
public function hook_aggregateTokenValues(&$values, $contactIDs, $job = NULL, $tokens = array(), $context = NULL) {
foreach ($contactIDs as $contactID) {
CRM_Core_Smarty::singleton()->assign('messageContactID', $contactID);
$values[$contactID]['aggregate.rendered_token'] = CRM_Core_Smarty::singleton()
Expand Down

0 comments on commit b177614

Please sign in to comment.