Hence set the last parameter in get_branch_contacts() to false if you want multiple mailing and leave it at true for single email.

The sequence of customer and branch creation is as follows:
1. Sales => Add and Manage Customers => Added New Customer with email id.
2. Customer Branches => Select Customer => Edit Customer Branch => Contacts => Add 2 New Contacts For Branch
=> Invoice and General Type with email id
=> Invoice and Orders Type with email id
3. Sales => Direct Invoice => Make an invoice for the customer branch.
4. Sales => Customer and Sales Reports => Print Invoices =. Choose to email customers and select the newly created invoice and Display Print Invoice (after setting rep107.php last parameter in get_branch_contacts() to false).

Works as expected - emails to 3 different recipients but separate emails.

@joe: Your logic is sound. Tested it out as well and it corroborates well - the NewPage is the one that actually gives effect to  the Tmpl setting and it's sequence anywhere prior to it is immaterial.

Hello again.

The member function Header2() is called in the member function NewPage on line 944 in pdf_report.inc.

Therefore it doesn't matter if the SetHeaderType member function is placed Before or after the SetCommonData member function.

/Joe

In this instance in rep107.php (Invoicing), $contacts is populated with data from the get_branch_contacts() function and is passed on as an argument into the $rep->SetCommonData() method which in turn populates the $this->formData and $this->contactData properties which in turn is used to get the contacts to email the reports to. In fact the SetCommonData() method makes even a single email id into an array, ie., $this->contactData.

Will investigate further....

Why are we moving the SetHeaderTmpl to be after SetCommonData? As I can see in the code there is no call to Header2() before a NewPage is initiated. The SetHeaderTmpl is only setting the HeaderTmpl.

The NewPage call is after the SetCommonData.

So in my Point of view this is not necessary. I guess this was not necessary in 2.4 either, although it does not harm anything.

/Joe

                $contactData = array();
                if ($this->contactData)
                    foreach($this->contactData as $contact)
                        if (!empty($contact['email']))
                            $contactData[] = $contact;

                if(!count($contactData)) {
                    $this->SetLang(user_language());
                    display_warning(sprintf(_("You have no email contact defined for this type of document for '%s'."), $this->formData['recipient_name']));
                } else {
                    $sent = $try = 0;

Only if there was no email available would you get the error stated

Therefore in the incoming $this->contactData itself, there would be no such record with email contact details and that narrows it down to the output of the get_branch_contacts() function. A check on with and without the last parameter being false will reveal that when it is true, the defaults have kicked in and not the appropriate contact for the specified action type. Try adding more than one contact to the branch where you choose it to be of invoice type (rep107.php) and then see what happens on making the last parameter false.

Please note that you will need tobackportsome report fixes from FA 2.4 since the SetCommonData() method needs to precede the Header2() call in reports that call it. This is necessary because the property $this->contactData is assigned from the $contact in line 409 and the latter comes from the said method SetCommonData() which in turn gets it from the get_branch_contacts() function call in rep107.php a couple of lines earlier.

@joe: the backports of the repXXX.php files done on 2015-01-21 is hence necessary.

Message creation (in reporting/includes/pdf_report.inc) needs to be moved out of the foreach loop, with the approriate headers ("to" and/or "cc") being added as the script loops over the contacts array.

Hence currently, no code change is required except for setting the last parameter as false in reports that need to be emailed to multiple entities!

Either I am misunderstanding what this means, or this is not the case.

I think this means that in rep107.php if I change this line:

$contacts = get_branch_contacts($branch['branch_code'], 'invoice', $branch['debtor_no'], true);

to this:

$contacts = get_branch_contacts($branch['branch_code'], 'invoice', $branch['debtor_no'], false);

the mail should send to both contacts.  I had actually tried this before, based on the content of one of the posts you pointed me at earlier.  If I put the sales/includes/db/branches_db.inc file back to how it was originally and change true to false as described above, it throws this error on the screen when I send an email:

You have no email contact defined for this type of document for 'Computerisms'.

If I am misunderstanding, can you please clarify?

Currently, the function get_branch_contacts is used in:

sales/includes/ui/sales_order_ui.inc
repXXX.php
where XXX in 103 (commented out), 107 (Invoice), 108 (commented out), 109  through to 113.

In the extensions, rep_email_customers calls it in modules/reporting/rep_email_customers/includes/customer_emailer.inc with the last parameter as false (no defaults). Also, when the second parameter ($action) is set to null as in this file, all contacts accrue.

In sales/includes/ui/sales_order_ui.inc the following construct in lines 112-114:

    $contact = get_branch_contacts($branch_id, 'order', $customer_id);
    $order->set_branch($branch_id, $myrow["tax_group_id"],
    $myrow["tax_group_name"], @$contact["phone"], @$contact["email"]);

expect to have defaults (missing last parameter assumed $default as true) and restricted to the 'orders' type alone and is used as a single contact for populating the branch details alone.

The standard repXXX.php listed above use the following constructs:

rep107.php:
$contacts = get_branch_contacts($branch['branch_code'], 'invoice', $branch['debtor_no'], true);

rep109.php:
$contacts = get_branch_contacts($branch['branch_code'], 'order', $branch['debtor_no'], true);

rep110.php:
$contacts = get_branch_contacts($branch['branch_code'], 'delivery', $branch['debtor_no'], true);

rep111.php:
$contacts = get_branch_contacts($branch['branch_code'], 'order', $branch['debtor_no'], true);

rep112.php:
$contacts = get_branch_contacts($myrow['branch_code'], 'invoice', $myrow['debtor_no']);

rep113.php:
$contacts = get_branch_contacts($branch['branch_code'], 'invoice', $branch['debtor_no'], true);

which makes last parameter $default to be true but allows for (but currently does not obtain) multiple contacts in subsequent emailing usage alone.

Either we determine it to be a single contact (count elements in array), or choose only the first one for the sales/includes/ui/sales_order_ui.inc and all other places we need single contacts or make a new function called get_branch_contact (singular) with the current code in it and make new content for the existing function get_branch_contacts (plural) for usage where multiple contacts are needed.

As far as emailing is concerned, we can make the first contact as the "To" recipient and all others as the "Cc" recipients if they exist!

Way forward is to be decided - all your thoughts on this is welcome before a patch is forwarded to @joe.

Hence currently, no code change is required except for setting the last parameter as false in reports that need to be emailed to multiple entities!

Fortunately, in a small business it should be obvious to those concerned why they got the message. And it's quite clear who else got it, so they can always work it out between themselves if necessary.

I would suggest reorganising that bit to first create the message and then loop over the recipients while adding the "To" headers.

(CC is probably best avoided here, as it implies "FYI" rather than "Here's a message for you. Please do something with it". So, who is the "real" recipient, and who gets CC'ed?)

While multiple To headers would work just fine, CC would actually be more appropriate in most of my cases for the exact reason you state.  Most of my customers are small businesses, so I send the invoice to the accounting person/department, who need to do something about it, but need to send a copy of the invoice to the manager/owner so they are informed.

Maybe having a CC check box in the contacts section or something would be cool.

But really, To vs CC is minutia when compared to getting every thing bundled into a single mail...

When I looked at the code, it seemed that the mail was generated and sent to each recipient (which was of course of little consequence, as there was never more than one).

I would suggest reorganising that bit to first create the message and then loop over the recipients while adding the "To" headers.

(CC is probably best avoided here, as it implies "FYI" rather than "Here's a message for you. Please do something with it". So, who is the "real" recipient, and who gets CC'ed?)

I am quite busy at present. Will you send me a patch to fix this in release 2.3. Release 2.4 will then be updated when the final merge is done.

Thanks in advance.

/Joe

I have two follow-up questions:

1.  I have tried commenting each part(s) of the stanza in the get_branch_contacts function but seems I have to comment the whole thing for email to work.  Can I make a change there to fix this properly, or am I to understand from your reply that the problem is more involved than that?

2.  There is a CC function in class.mail.inc, but I don't find any way to use it.  it seems when a mail is sent to multiple contacts, it make a separate mail for each individual, such that nobody can see if the other recipients got it.  Can this be adjusted?  I have one client who says I need to send by snailmail if it can't, and I am not sure if I remember how to use a stamp

//
//    Search $needle in $haystack or in $haystack[][$valuekey]
//    returns $needle found or null.
//
function array_search_value($needle, $haystack, $valuekey=null)
{
    foreach($haystack as $key => $value) {
        $val = isset($valuekey) ? @$value[$valuekey] : $value;
        if ($needle == $val){
            return $value;
        }
    }
    return null;
}

This function is used by function get_branch_contacts in sales/includes/db/branches_db.inc (this file has another similarly named function prefixed with an underscore that is neither used in the FA core nor in any extension).

At the very end of the function get_branch_contacts we find that only one element is returned because of a return within the loop:

    foreach($defs as $type) {

        if ($n = array_search_value($type, $results, 'ext_type'))

            return $n;
    
    }

If this value is captured in a separate array if it is not empty, then that array would have all the necessary contacts.

This is clearly a bug.

@joe: please update accordingly in FA 2.3 and into FA 2.4.

PS: The error in includes/ui/simple_crud_class.inc is tolerated because of the "@" prefixing the assignment. A check for the variable's existence before assignment would remedy it and it is needed in more than one place in the same function!