dev/core#48 Fix PDF Letter only generates a single letter when multiple contact IDs are specified

[monishdeb]

Overview

Ref https://lab.civicrm.org/dev/core/issues/48
When multiple contact IDs are specified via print/merge task the PDFLetterCommon code overwrites them with a single contact ID of the logged in user. This means that only a single PDF letter is printed/generated.
This PR only uses the logged in contact ID if not contact IDs have been specified.

Before

Only a single PDF letter was being generated by print/merge document.

After

Multiple PDF letters are generated by print/merge document.

Comments

It is possible that this issue was introduced by changes in #11411 which introduced a CRM_Core_Form_Task class but looking at the code and the proposed changes here I'm not sure how Print/Merge ever printed multiple documents as the code I'm changing here hasn't changed for more than 5 years!

[monishdeb]

@mattwire @eileenmcnaughton here's the additional change:

1. Although this is a hack from old days, but still left a TODO comment to indicate what needs to be done to remove that hack - https://github.com/civicrm/civicrm-core/pull/11985/files#diff-5224650ca4795126c06ff6c16ad0b7afR75

2. Lets maintain the same order like in other 6 tasks i.e.

public function preProcess() {
+    CRM_Contact_Form_Task_PDFLetterCommon::preProcess($this);
     $this->skipOnHold = $this->skipDeceased = FALSE;
     parent::preProcess();
     $this->setContactIDs();
-    CRM_Contact_Form_Task_PDFLetterCommon::preProcess($this);
   }

[eileenmcnaughton]

@monishdeb on checking there is quite a bit of order-variance on the other classes - this seems like the one you are emulating

  /**
   * Build all the data structures needed to build the form.
   */
  public function preProcess() {
    CRM_Contact_Form_Task_PDFLetterCommon::preProcess($this);
    parent::preProcess();

    // we have all the participant ids, so now we get the contact ids
    parent::setContactIDs();

    $this->assign('single', $this->_single);
  }

I can't see anything in the parent::preProcess that will be hurt by moving it right up there before it.

Merging

[monishdeb]

Yes checked these classes and they obey that pattern except the Case

CRM/Activity/Form/Task/Email.php:60:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);
CRM/Contact/Form/Task/Email.php:129:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);
CRM/Contact/Form/Task/PDFLetterCommon.php:61:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($form);
CRM/Contribute/Form/Task/Email.php:60:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);
CRM/Contribute/Form/Task/Invoice.php:132:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);
CRM/Event/Form/Task/Email.php:68:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);
CRM/Member/Form/Task/Email.php:71:    CRM_Contact_Form_Task_EmailCommon::preProcessFromAddress($this);

[eileenmcnaughton]

unrelated fail

[colemanw]

Nice Elvis