New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CRM-19767 - Add case tokens to email activities #10252
Conversation
How's about a test :-) |
Looks like those bloated functions are untestable as-is. Would take some
refactoring. I'll see what I can do.
…On 04/25/2017 08:21 PM, Eileen McNaughton wrote:
How's about a test :-)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10252 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACveINXT8XrVrG60whWcGv0v0e1FoO-Bks5rzo4ngaJpZM4NINPt>.
|
CRM_Contribute_Form_AdditionalPaymentTest or CRM_Contribute_Form_Task_InvoiceTest might provide a starting point |
test this please |
@@ -1576,7 +1576,7 @@ public static function replaceEntityTokens($entity, $entityArray, $str, $knownTo | |||
|
|||
/** | |||
* @param int $caseId | |||
* @param int $str | |||
* @param string $str | |||
* @param array $knownTokens | |||
* @param bool $escapeSmarty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be space here before the @return string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -352,6 +352,8 @@ public static function processMessageTemplate($formValues) { | |||
* Process the form after the input has been submitted and validated. | |||
* | |||
* @param CRM_Core_Form $form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a space here also before @throws \CRM_Core_Exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Also in addition to what was already said, would be great to have some tests for this change. :) |
@guanhuan will you assign adding the test please? |
I'm going to close this because it needs rebasing & I think it's better to re-open once (if) rebased than to keep it open |
Drat. Github won't allow me to reopen a PR after rebasing it because of the force push it considers the branch to be different :(. I've opened a new one instead. |
In CiviCRM core, case tokens had only been partially implemented.
{case.id}
, etc, were available.This fixes that.