gugacavalieri/MauticEmailSentNotifierBundle

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!