Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated callback.phps #2305

Closed
wants to merge 1 commit into from
Closed

Updated callback.phps #2305

wants to merge 1 commit into from

Conversation

nicolomonili
Copy link

Updated example

Update example
@nicolomonili nicolomonili changed the title Update callback.phps Updated callback.phps Apr 8, 2021
@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

That doesn't look right - the $to param is an array, so I would expect this change to result in array-to-string conversion errors. That said, the example uses $address[0], which will work, but also $address[1], which doesn't exist, and should contain the to name. That's an omission in the callback code. I think that this line:

            $callbacks[] = ['issent' => $isSent, 'to' => $to[0]];

should become

            $callbacks[] = ['issent' => $isSent, 'to' => $to];

so that it contains all info about the recipient.

@nicolomonili
Copy link
Author

Ohh yes, that was the problem!

@nicolomonili
Copy link
Author

It should be so yes

$callbacks[] = ['issent' => $isSent, 'to' => $to];

The example would only be correct if both information were present (array with address and name).

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

It's actually a bit more involved – the use of doCallback is inconsistent. Sometimes it will contain (as in this case):

['address@example.com`]

but in other places the whole of $mail->to is passed in, which may contain structures like:

[['address@example.com`, 'User Name'], ['address2@example.com`, 'User2 Name']]

The first case should be amended to match the second pattern.

@nicolomonili
Copy link
Author

nicolomonili commented Apr 8, 2021

There is also another problem

$callbacks = [];
//Attempt to send to all recipients
foreach ([$this->to, $this->cc, $this->bcc] as $togroup) {
    foreach ($togroup as $to) {
        if (!$this->smtp->recipient($to[0], $this->dsn)) {
            $error = $this->smtp->getError();
            $bad_rcpt[] = ['to' => $to[0], 'error' => $error['detail']];
            $isSent = false;
        } else {
            $isSent = true;
        }

        $callbacks[] = ['issent' => $isSent, 'to' => $to[0]];
    }
}

By doing this, the parameters cc and bcc aren't updated in the $callbacks array. Only the to parameter is updated.

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

I have pushed a change in master that should make calls to doCallback consistent; addresses will now always have the second format, as documented, and expected by this example. Please give it a try.

@nicolomonili
Copy link
Author

There still seems to be the problem. Maybe the change should be made here too?

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

No, because the to property is already in the right format. What problem are you seeing?

@nicolomonili
Copy link
Author

nicolomonili commented Apr 8, 2021

Ok found the problem.
Here, to is not an array.

Synchro added a commit that referenced this pull request Apr 8, 2021
@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

Thanks, I missed that one, fix pushed

@nicolomonili
Copy link
Author

Tested, now it works!
Now only this problem remains to be solved.

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

What problem? The original callback example should now work correctly?

@nicolomonili
Copy link
Author

The original callback example now it works, now to it's an array of arrays.
But

the parameters cc and bcc aren't updated in the $callbacks array. Only the to parameter is updated.

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

By doing this, the parameters cc and bcc aren't updated in the $callbacks array. Only the to parameter is updated.

I don't think that really applies here. At the SMTP level there is no distinction between to, cc and bcc – they are all just RCPT TO commands.

@nicolomonili
Copy link
Author

Ok, so in the callback i will have all the addresses in the parameter to? Even if they were added as CC or BCC?

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

No, with SMTP you will get a separate callback for each message recipient. I don't think it's important whether the address was originally in to, cc or bcc, because you were the one that set them!

The mail() transport behaves differently because you don't get that opportunity, and you will get all of to, cc, and bcc in a single hit, broken out into those params, and you can't tell which address failed. If it acted the same way for SMTP, you would not be able to get the separate error status for each recipient that you were originally looking for.

@nicolomonili
Copy link
Author

Maybe i explained myself wrong, sorry.

With SMTP a separate callback for each message recipient it's perfect.

But in this case :

...
$mail->addAddress("email1@gmail.com");
$mail->addBCC("email2@gmail.com");
$mail->addCC("email3@gmail.com");
...

in the callback function the parameters to, bcc and cc will they be like this?

$to = [["email1@gmail.com", ""],["email2@gmail.com", ""],["email3@gmail.com", ""]]
$bcc = []
$cc = []

and not so?

$to = [["email1@gmail.com", ""]] 
$bcc = [["email2@gmail.com", ""]]
$cc = [["email3@gmail.com", ""]]

@Synchro
Copy link
Member

Synchro commented Apr 8, 2021

No, you will get three separate callbacks, each time with a different $to value:

$to = [["email1@gmail.com", ""]]

$to = [["email2@gmail.com", ""]]

$to = [["email3@gmail.com", ""]]

Whether an address is to, cc, or bcc has no bearing on the sending status – you're never going to have an address that fails in cc but works in to, because at the point that happens, the receiving server can't tell them apart.

With the mail() transport you will get your second example, because it only gets called once, with all recipient addresses.

@nicolomonili
Copy link
Author

I have explained myself badly once again, perfect 🤣
However now i understand that in any case the address will be in the to parameter, thanks

@Synchro Synchro closed this Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants