From 2ebb452b239c28e7ed2c50da69d324a8d40a7e88 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Fri, 24 Apr 2020 16:33:26 +1200
Subject: [PATCH] [REF] Code readability changes on activity tokens.

I've been trying to get to grips with this code and this addresses 2 things I found very confusing - notably

- using activityId when we were referring to a fieldname - we use a combination of  &  for
variables but normally when it's closely related to a query we use activity_id
- the function name getEntityContextSchema() was unclear to me - I concluded that in fact we were retrieving
the nname of the field for the entity ID
---
 CRM/Activity/Form/Task/PDFLetterCommon.php |  4 ++--
 CRM/Activity/Tokens.php                    | 12 ++++++++----
 CRM/Core/TokenTrait.php                    |  3 ++-
 Civi/Token/TokenProcessor.php              |  2 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/CRM/Activity/Form/Task/PDFLetterCommon.php b/CRM/Activity/Form/Task/PDFLetterCommon.php
index 424249fb5c..7af6c006cf 100644
--- a/CRM/Activity/Form/Task/PDFLetterCommon.php
+++ b/CRM/Activity/Form/Task/PDFLetterCommon.php
@@ -53,7 +53,7 @@ class CRM_Activity_Form_Task_PDFLetterCommon extends CRM_Core_Form_Task_PDFLette
     $tp->addMessage('body_html', $html_message, 'text/html');
 
     foreach ($activityIds as $activityId) {
-      $tp->addRow()->context('activityId', $activityId);
+      $tp->addRow()->context('activity_id', $activityId);
     }
     $tp->evaluate();
 
@@ -69,7 +69,7 @@ class CRM_Activity_Form_Task_PDFLetterCommon extends CRM_Core_Form_Task_PDFLette
     return new TokenProcessor(\Civi::dispatcher(), [
       'controller' => get_class(),
       'smarty' => FALSE,
-      'schema' => ['activityId'],
+      'schema' => ['activity_id'],
     ]);
   }
 
diff --git a/CRM/Activity/Tokens.php b/CRM/Activity/Tokens.php
index fff8e6224a..e8d875c5e7 100644
--- a/CRM/Activity/Tokens.php
+++ b/CRM/Activity/Tokens.php
@@ -48,10 +48,12 @@ class CRM_Activity_Tokens extends \Civi\Token\AbstractTokenSubscriber {
   }
 
   /**
+   * Get the name of the field which holds the ID of the given entity.
+   *
    * @return string
    */
-  private function getEntityContextSchema(): string {
-    return 'activityId';
+  private function getEntityIDFieldName(): string {
+    return 'activity_id';
   }
 
   /**
@@ -88,7 +90,7 @@ class CRM_Activity_Tokens extends \Civi\Token\AbstractTokenSubscriber {
     // Find all the entity IDs
     $entityIds
       = $e->getTokenProcessor()->getContextValues('actionSearchResult', 'entityID')
-      + $e->getTokenProcessor()->getContextValues($this->getEntityContextSchema());
+      + $e->getTokenProcessor()->getContextValues($this->getEntityIDFieldName());
 
     if (!$entityIds) {
       return NULL;
@@ -122,6 +124,8 @@ class CRM_Activity_Tokens extends \Civi\Token\AbstractTokenSubscriber {
 
   /**
    * @inheritDoc
+   *
+   * @throws \CRM_Core_Exception
    */
   public function evaluateToken(\Civi\Token\TokenRow $row, $entity, $field, $prefetch = NULL) {
     // maps token name to api field
@@ -130,7 +134,7 @@ class CRM_Activity_Tokens extends \Civi\Token\AbstractTokenSubscriber {
     ];
 
     // Get ActivityID either from actionSearchResult (for scheduled reminders) if exists
-    $activityId = $row->context['actionSearchResult']->entityID ?? $row->context[$this->getEntityContextSchema()];
+    $activityId = $row->context['actionSearchResult']->entityID ?? $row->context[$this->getEntityIDFieldName()];
 
     $activity = (object) $prefetch['activity'][$activityId];
 
diff --git a/CRM/Core/TokenTrait.php b/CRM/Core/TokenTrait.php
index 26110155ef..bd77c7dc69 100644
--- a/CRM/Core/TokenTrait.php
+++ b/CRM/Core/TokenTrait.php
@@ -19,7 +19,7 @@ trait CRM_Core_TokenTrait {
    * @inheritDoc
    */
   public function checkActive(\Civi\Token\TokenProcessor $processor) {
-    return in_array($this->getEntityContextSchema(), $processor->context['schema']) ||
+    return in_array($this->getEntityIDFieldName(), $processor->context['schema'], TRUE) ||
       (!empty($processor->context['actionMapping'])
         && $processor->context['actionMapping']->getEntity() === $this->getEntityTableName());
   }
@@ -51,6 +51,7 @@ trait CRM_Core_TokenTrait {
 
   /**
    * Find the fields that we need to get to construct the tokens requested.
+   *
    * @param  array $tokens list of tokens
    * @return array         list of fields needed to generate those tokens
    */
diff --git a/Civi/Token/TokenProcessor.php b/Civi/Token/TokenProcessor.php
index 1364ed714f..4fa03b9038 100644
--- a/Civi/Token/TokenProcessor.php
+++ b/Civi/Token/TokenProcessor.php
@@ -27,7 +27,7 @@ class TokenProcessor {
    *   - schema: array, a list of fields that will be provided for each row.
    *     This is automatically populated with any general context
    *     keys, but you may need to add extra keys for token-row data.
-   *     ex: ['contactId', 'activityId'].
+   *     ex: ['contactId', 'activity_id']. (Note we are standardising on the latter).
    */
   public $context;
 
-- 
2.25.1