Email sent from CiviCRM for a new Case and Activity does not evaluate the $activityTypeName or $manageCaseURL tokens

[jusfreeman]

Overview

Copied from https://www.drupal.org/project/webform_civicrm/issues/2990790

Email sent from CiviCRM for a new Case and Activity does not evaluate the $activityTypeName or $manageCaseURL tokens and possibly other tokens as well.

Steps to reproduce:

1. Set up a fresh install of Drupal (7.59) and CiviCRM (5.40).
2. Installed latest versions of Webform and CiviCRM Webform Integration and supporting modules (Chaos tools, Views etc.)
3. Enabled CiviCase
4. Created a webform and enabled CiviCRM processing with three fields - first/last name, email
5. Enabled 1 activity ('Long-term housing plan' - out of the box 'sample' CiviCRM case activity) and assigned it to Contact 1
6. Enabled 1 case ('Housing Support' - out of the box 'sample' CiviCRM case type)
7. File the Activity enabled in 5. on Case 1.
8. Submit the webform
9. Case and Activity are created correctly
10. Error is that the email resulting from webform submission does not evaluate all Case related tokens ($activityTypeName or $manageCaseURL). Resulting in the email as shown in
    https://www.drupal.org/files/issues/2018-08-06/2018-08-03_19-50-56-webform-case-activity-email.png

If the email is manually initiated from the Activity through the CiviCase in CIviCRM, then the Case related tokens are evaluated and the email is correct, see https://www.drupal.org/files/issues/2018-08-06/2018-08-03_19-51-09-civicrm-case-activity-email.png

Comments

This issue has been reported on drupal.org https://www.drupal.org/project/webform_civicrm/issues/2990790

This bug requires both a fix to Webform CiviCRM and CiviCRM core. Two separate PRs have been submitted:
Webform CiviCRM (PR 187), colemanw/webform_civicrm#187
CiviCRM Core (PR 13324), #13324

Agileware Ref: SUP-5762 and CIVICRM-947

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

paging @colemanw

[colemanw]

I think it would be better to use the CRM_Core_PseudoConstant::getLabel() function (example of that below in the diff of CRM_Case_XMLProcessor_Report).

[jusfreeman]

@colemanw thanks for the review, we'll update the PR in the next few days.

[colemanw]

@agilewarealok I've rebased and force-pushed your branch with the changes Justin & I discussed.
Just a note for the future, could you please

• Refrain from putting internal tracking numbers in commit messages
• Use rebases instead of multiple commits to keep PRs tidy. With the exception of some big complex PRs we generally want to have only 1 commit per PR, so we don't wind up with a VCS history like

commit 1: fix X
commit 2: better fix for X
commit 3: fix typo in previous commit

[colemanw]

• General standards
  • (r-explain) Pass
  • (r-user) Pass: User expects this info in emails.
  • (r-doc) Pass
  • (r-run) Pass: Replicated the bug and confirmed the fix.
• Developer standards
  • (r-tech) Pass: No changes
  • (r-code) Pass
  • (r-maint) Pass
  • (r-test) Pass

Reviewers note: I struggled to understand where the heck the $activityTypeName tpl assignment comes from, it seems to just appear out of the blue. I never fully traced it, but I'd hazard a guess that it's an artifact of the fact that Civi uses Smarty as a singleton, so the variable is assigned to some other template and it lingers in memory long enough to be used in the email, at least from some screens.
Explicit assignment here is much better.

[jusfreeman]

@colemanw thanks mate. On this point: "Refrain from putting internal tracking numbers in commit messages" - it's our company policy to do this so commits are auto-linked with the related tasks in our Jira. Otherwise, we'd have no idea what code changes related to what. Sorry for the hassle.

[monishdeb]

merging as per tag