From cf7c5cb8a38794e300bc4b2d8daa387361b7f8cd Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 25 Nov 2019 12:49:04 +1300
Subject: [PATCH] [REF] Minor code cleanup on the setting of contact greetings.

I'm trying to review https://github.com/civicrm/civicrm-core/pull/15725 but through no fault of that PR it hits
on one of the parts of the Export code that I still find unreadable :-( This is a further readability fix & it
starts to point me to how to get past the underlying WTF about this code.

Note that I removed the IF around sharedAddress. From Monish's PR I found out that we have a
fairly long-standing regression where the sharedAddress code doesn't actually work :-( so
I'm comfortable this won't break anything :-). We also have good test cover on this chunk from
probably around when the shared address part got broken....

Looking in the UI there is no implication that the greeting for a shared address would display differently -
which is the only explanation I can think of for different handling here. Of course until we
remove the later IF the shared address would do things differently - erm if it worked.

ALl of which is a long way of saying the removal of the IF won't change anything
---
 CRM/Export/BAO/ExportProcessor.php | 58 ++++++++++++++++++------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/CRM/Export/BAO/ExportProcessor.php b/CRM/Export/BAO/ExportProcessor.php
index 972c55461ce0..26ae7af22ecf 100644
--- a/CRM/Export/BAO/ExportProcessor.php
+++ b/CRM/Export/BAO/ExportProcessor.php
@@ -1914,31 +1914,14 @@ public function buildMasterCopyArray($sql, $sharedAddress = FALSE) {
     while ($dao->fetch()) {
       $masterID = $dao->master_id;
       $copyID = $dao->copy_id;
-      $masterPostalGreeting = $dao->master_postal_greeting;
-      $masterAddressee = $dao->master_addressee;
       $copyAddressee = $dao->copy_addressee;
 
-      if (!$sharedAddress) {
-        if (!isset($this->contactGreetingFields[$dao->master_contact_id])) {
-          $this->contactGreetingFields[$dao->master_contact_id] = $this->replaceMergeTokens($dao->master_contact_id);
-        }
-        $masterPostalGreeting = CRM_Utils_Array::value('postal_greeting',
-          $this->contactGreetingFields[$dao->master_contact_id], $dao->master_postal_greeting
-        );
-        $masterAddressee = CRM_Utils_Array::value('addressee',
-          $this->contactGreetingFields[$dao->master_contact_id], $dao->master_addressee
-        );
-
-        if (!isset($contactGreetingTokens[$dao->copy_contact_id])) {
-          $this->contactGreetingFields[$dao->copy_contact_id] = $this->replaceMergeTokens($dao->copy_contact_id);
-        }
-        $copyPostalGreeting = CRM_Utils_Array::value('postal_greeting',
-          $this->contactGreetingFields[$dao->copy_contact_id], $dao->copy_postal_greeting
-        );
-        $copyAddressee = CRM_Utils_Array::value('addressee',
-          $this->contactGreetingFields[$dao->copy_contact_id], $dao->copy_addressee
-        );
-      }
+      $this->cacheContactGreetings((int) $dao->master_contact_id);
+      $this->cacheContactGreetings((int) $dao->copy_contact_id);
+      $masterPostalGreeting = $this->getContactGreeting((int) $dao->master_contact_id, 'postal_greeting', $dao->master_postal_greeting);
+      $masterAddressee = $this->getContactGreeting((int) $dao->master_contact_id, 'addressee', $dao->master_addressee);
+      $copyPostalGreeting = $this->getContactGreeting((int) $dao->copy_contact_id, 'postal_greeting', $dao->copy_postal_greeting);
+      $copyAddressee  = $this->getContactGreeting((int) $dao->copy_contact_id, 'addressee', $dao->copy_addressee);
 
       if (!isset($merge[$masterID])) {
         // check if this is an intermediate child
@@ -2414,4 +2397,33 @@ protected function writeRows(array $headerRows, array $componentDetails) {
     );
   }
 
+  /**
+   * Cache the greeting fields for the given contact.
+   *
+   * @param int $contactID
+   */
+  protected function cacheContactGreetings(int $contactID) {
+    if (!isset($this->contactGreetingFields[$contactID])) {
+      $this->contactGreetingFields[$contactID] = $this->replaceMergeTokens($contactID);
+    }
+  }
+
+  /**
+   * Get the greeting value for the given contact.
+   *
+   * The values have already been cached so we are grabbing the value at this point.
+   *
+   * @param int $contactID
+   * @param string $type
+   *   postal_greeting|addressee|email_greeting
+   * @param string $default
+   *
+   * @return string
+   */
+  protected function getContactGreeting(int $contactID, string $type, string $default) {
+    return CRM_Utils_Array::value($type,
+      $this->contactGreetingFields[$contactID], $default
+    );
+  }
+
 }