From a341edf6b566849e1aba14ac38d2f43f09b12238 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Fri, 3 Sep 2021 18:37:34 +1200
Subject: [PATCH] [REF] Follow on on code move, make not static

---
 CRM/Member/Form/Task/PDFLetter.php            | 25 +++++++++++--------
 .../CRM/Member/Form/Task/LabelTest.php        |  2 +-
 .../Member/Form/Task/PDFLetterCommonTest.php  |  7 +++---
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/CRM/Member/Form/Task/PDFLetter.php b/CRM/Member/Form/Task/PDFLetter.php
index d6cb9df570..02d581159f 100644
--- a/CRM/Member/Form/Task/PDFLetter.php
+++ b/CRM/Member/Form/Task/PDFLetter.php
@@ -70,28 +70,29 @@ class CRM_Member_Form_Task_PDFLetter extends CRM_Member_Form_Task {
     $this->setContactIDs();
     $skipOnHold = $this->skipOnHold ?? FALSE;
     $skipDeceased = $this->skipDeceased ?? TRUE;
-    self::postProcessMembers(
-      $this, $this->_memberIds, $skipOnHold, $skipDeceased, $this->_contactIds
-    );
+    $this->postProcessMembers($this->_memberIds, $skipOnHold, $skipDeceased, $this->_contactIds);
   }
 
   /**
    * Process the form after the input has been submitted and validated.
-   * @todo this is horrible copy & paste code because there is so much risk of breakage
-   * in fixing the existing pdfLetter classes to be suitably generic
    *
-   * @param CRM_Core_Form $form
    * @param $membershipIDs
    * @param $skipOnHold
    * @param $skipDeceased
    * @param $contactIDs
+   *
+   * @throws \CRM_Core_Exception
+   * @todo this is horrible copy & paste code because there is so much risk of breakage
+   * in fixing the existing pdfLetter classes to be suitably generic
+   *
    */
-  public static function postProcessMembers(&$form, $membershipIDs, $skipOnHold, $skipDeceased, $contactIDs) {
+  public function postProcessMembers($membershipIDs, $skipOnHold, $skipDeceased, $contactIDs) {
+    $form = $this;
     $formValues = $form->controller->exportValues($form->getName());
-    list($formValues, $categories, $html_message, $messageToken, $returnProperties) = CRM_Contact_Form_Task_PDFLetterCommon::processMessageTemplate($formValues);
+    [$formValues, $categories, $html_message, $messageToken, $returnProperties] = CRM_Contact_Form_Task_PDFLetterCommon::processMessageTemplate($formValues);
 
     $html
-      = self::generateHTML(
+      = $this->generateHTML(
       $membershipIDs,
       $returnProperties,
       $skipOnHold,
@@ -120,6 +121,8 @@ class CRM_Member_Form_Task_PDFLetter extends CRM_Member_Form_Task {
   /**
    * Generate html for pdf letters.
    *
+   * @internal
+   *
    * @param array $membershipIDs
    * @param array $returnProperties
    * @param bool $skipOnHold
@@ -130,7 +133,7 @@ class CRM_Member_Form_Task_PDFLetter extends CRM_Member_Form_Task {
    *
    * @return array
    */
-  public static function generateHTML($membershipIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $html_message, $categories) {
+  public function generateHTML($membershipIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $html_message, $categories) {
     $memberships = CRM_Utils_Token::getMembershipTokenDetails($membershipIDs);
     $html = [];
 
@@ -141,7 +144,7 @@ class CRM_Member_Form_Task_PDFLetter extends CRM_Member_Form_Task {
       $params = ['contact_id' => $contactId];
       //getTokenDetails is much like calling the api contact.get function - but - with some minor
       // special handlings. It precedes the existence of the api
-      list($contacts) = CRM_Utils_Token::getTokenDetails(
+      [$contacts] = CRM_Utils_Token::getTokenDetails(
         $params,
         $returnProperties,
         $skipOnHold,
diff --git a/tests/phpunit/CRM/Member/Form/Task/LabelTest.php b/tests/phpunit/CRM/Member/Form/Task/LabelTest.php
index 2860d6e657..642f65cd3f 100644
--- a/tests/phpunit/CRM/Member/Form/Task/LabelTest.php
+++ b/tests/phpunit/CRM/Member/Form/Task/LabelTest.php
@@ -22,7 +22,7 @@ class CRM_Member_Form_Task_LabelTest extends CiviUnitTestCase {
    * This is a 'no error' test to support refactoring. It ensures no fatal is hit & lays
    * the basis for more tests later.
    */
-  public function testMembershipTokenReplacementInPDF() {
+  public function testMembershipTokenReplacementInPDF(): void {
     // First check tasks are there is some weird static caching that could mess us up.
     $tasks = CRM_Member_Task::tasks();
     $this->assertArrayHasKey(201, $tasks, print_r($tasks, TRUE));
diff --git a/tests/phpunit/CRM/Member/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Member/Form/Task/PDFLetterCommonTest.php
index a30f6df286..856f2cab27 100644
--- a/tests/phpunit/CRM/Member/Form/Task/PDFLetterCommonTest.php
+++ b/tests/phpunit/CRM/Member/Form/Task/PDFLetterCommonTest.php
@@ -27,9 +27,9 @@ class CRM_Member_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase {
   /**
    * Test token replacement for Print/Merge Task
    */
-  public function testMembershipTokenReplacementInPDF() {
+  public function testMembershipTokenReplacementInPDF(): void {
     $membershipIds = $returnProperties = $categories = $expected = [];
-    list($tokens, $htmlMessage) = self::getSampleHTML();
+    [$tokens, $htmlMessage] = self::getSampleHTML();
 
     $membershipDates = [
       date('Y-m-d'),
@@ -71,7 +71,8 @@ class CRM_Member_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase {
       }
     }
     $messageToken = CRM_Utils_Token::getTokens($htmlMessage);
-    $testHTML = CRM_Member_Form_Task_PDFLetter::generateHTML($membershipIds,
+    $form = new CRM_Member_Form_Task_PDFLetter();
+    $testHTML = $form->generateHTML($membershipIds,
       $returnProperties,
       NULL,
       NULL,
-- 
2.25.1