Webhook fires multiple times
MattBred opened this issue · 5 comments
Using Mautic 2.15.2
I setup a webhook and it works, but when I send an email (segment email) the webhook fires twice.
I put some logging in place and found that the event is being kicked twice, here:
https://github.com/gugacavalieri/MauticEmailSentNotifierBundle/blob/master/Helper/EmailSentHelper.php#L67
Not sure if it's a Mautic bug or a bug in the implementation of this plugin.
Update, seems like someone else found this out too: mautic/mautic#7950
Is it possible to bandaid this somehow, or failing that, find the root cause?
I believe that I found the root cause:
In SendEmailToContact.php
Mautic calls sendStandardEmail()
This function dispatches the EMAIL_ON_SEND
event (which mostly triggers listeners in charge of filling in tokens).
Then, two lines later, the function calls the queue
method but it incorrectly sets the $dispatchSendEvent
argument to true
when it's supposed to be false
.
Here is the verbatim code (notice the comments)
protected function sendStandardEmail()
{
// Dispatch the event to generate the tokens
$this->mailer->dispatchSendEvent();
// Create the stat to ensure it is availble for emails sent
$this->createContactStatEntry($this->contact['email']);
// Now send but don't redispatch the event
return $this->mailer->queue(true, MailHelper::QUEUE_RETURN_ERRORS);
}
Here is the signature of the mailer->queue()
function ( Mautic\EmailBundle\HelperMailHelper::queue
)
/**
* If batching is supported and enabled, the message will be queued and will on be sent upon flushQueue().
* Otherwise, the message will be sent to the transport immediately.
*
* @param bool $dispatchSendEvent
* @param string $returnMode What should happen post send/queue to $this->message after the email send is attempted.
*
* @return bool|array
*/
public function queue($dispatchSendEvent = false, $returnMode = self::QUEUE_RESET_TO)
And this gets passed along to the MailHelper::send
function which ultimately calls the dispatchSendEvent()
function again.
So, there is clearly a flaw here because the developer says "don't redispatch the event", but the true flag specifically redispatches it.
IMO, a PR should be created to fix this flag (but I don't know if there would be other side effects
Just tested your suggestion by editing "true" to "false". Definitely fixes the problem, havent found any side effects!
awesome @MajesteitBart ! Could you open the PR please?
Will do!