Contrib.social

EmailWebformHandler::sendMessage() does not return the mailing result

Open on Drupal.org →
Created on 31 January 2023, almost 2 years ago

Problem/Motivation

\Drupal\webform\Plugin\WebformHandler\EmailWebformHandler::sendMessage() performs the following to get the result of the mailing:

$result = $this->mailManager->mail('webform', $key, $to, $current_langcode, $message, $from);

\Drupal\Core\Mail\MailManagerInterface::mail() declares that if the email is sent, the result element of the return array will contain the success indicator of the email, failure being already written to the watchdog.

\Drupal\Core\Mail\MailManager::doMail() method which actually sends the email, stores the result of the sender in result element:

$message['result'] = $system->mail($message);

\Drupal\webform\Plugin\WebformHandler\EmailWebformHandler::sendMessage() gets the result of \Drupal\Core\Mail\MailManagerInterface::mail() and returns the send element of the result:

return $result['send'];

The return of EmailWebformHandler::sendMessage() does not represent the actual result of the mailing but rather the initial intention to send as send element is usually set to TRUE.

Steps to reproduce

1. Install drupal/smtp module and set it as system default mailer.
2. Send the email using a SMTP module settings which would produce an error (non-existing SMTP hostname or wrong credentials).
3. See that EmailWebformHandler::sendMessage() will return TRUE even if message was not sent.

Proposed resolution

Add a method getSendMessageResult() which on behalf of ::sendMessage() returns the result from the mailer. This will allow overriding classes to return their own understanding of what constitutes a failed mailing:

/**
   * Returns the result from the mailer.
   *
   * @param array $result
   *   Mailing result array.
   *
   * @return bool
   *   The result of the email action.
   */
  protected function getSendMessageResult(array $result) {
    return $result['send'];
  }

EmailWebformHandler::sendMessage() would return the following:

return $this->getSendMessageResult($result);

This allows overriding implementations to return $result['result'] instead of $result['send'] while maining the current logic of ::sendMessage() return.

Remaining tasks

No tasks are remaining.

User interface changes

No changes.

API changes

No changes.

Data model changes

No changes.

🐛 Bug report
Status

Active

Version

6.1

Component

Code

Created by

🇱🇻Latvia maijs

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @maijs
  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇱🇻Latvia maijs
  • Status changed to Postponed: needs info almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States jrockowitz Brooklyn, NY

    I am not following why we need this change or how it helps.

    Hopefully, someone can review the ticket and patch and provide more context.

  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇱🇻Latvia maijs

    Sorry for not providing enough context for why this is important. In a project there's a requirement to lock the webform when the mailing of the submission values resulted with a failure. After the webform administrator makes sure that the webform is properly sent/resent via email to the relevant parties (according to the email mailer), the webform is manually unlocked, and the webform is then processed according to purging policies.

    I have updated the text of the issue for clarity.

    As I mentioned before, the \Drupal\Core\Mail\MailManagerInterface::mail() PhpDoc states that it's the result element that contains the result of the mailing. The send element that the sendMessage() method returns is the intent to send or not to send the email, and not the result itself.

    Interestingly enough the \Drupal\webform\Plugin\WebformHandlerMessageInterface::sendMessage() PhpDoc doesn't state any return value, yet it returns the $result['send'] (intent) value.

    My suggestion is to make the return of the sendMessage() method more meaningful, so that overriding classes could act on what happens within the sendMessage() method without copying and pasting the whole method's code just to return the success or the failure of the mailing.

  • Comment almost 2 years ago →
    🇺🇸United States jrockowitz Brooklyn, NY

    I feel like we are dealing with an edge case that may not require this change. Of course, you can use the patch AS-IS.

  • Status changed to Closed: won't fix over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States jrockowitz Brooklyn, NY
  • Comment 15 days ago →
    🇮🇳India sethu_sk

    We are sending the attachment in email, once after the email is sent, we need to delete the file stored in Drupal. So this reason, we need to know the status of email.

  • Comment 14 days ago →
    🇮🇳India sethu_sk
  • Comment 14 days ago →
    🇮🇳India sethu_sk
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024