drupal_set_message() will be deprecated soon.

I want to throw this out there, to see what others think. I've been reviewing how other contributors have been addressing this issue, considering we do not want to release this until at least 8.5.0 is released.

My idea at the moment, is to temporarily place the following in the main info.yml file:

dependencies:
  - drupal:system (< 8.5)

Postponing this patch, and as soon as 8.5 is released, begin work on the drupal_set_messages patch, that will include:

dependencies:
  - drupal:system (>= 8.5.0)

If possible, it would be preferable to not have to support multiple branches for legacy core systems, especially since this is a new module.

Any thoughts? We have a few months until crunch time on this, anyway

Patch created to ensure initial safeguard for core version and deprecated drupal_set_message().

In retrospect, I am actually going to make a start on this patch. Since 8.5 is already out (so messenger service is already in core) and we should be ready for full release very soon, it makes sense that we 8.5.0 should be our starting point. This relieves pressure from having separate releases for < 8.4.

@banhu, I know you're keen and want to learn about this. So reviewing the patch may be a good introduction to it.

Patch created, can someone please review.

reviewed and tested by self. patch applies, all tests pass.

accidentally committed this with the incorrect message, re. the previous commit (forgot to add the tests for marketing_cloud_example)

Re-opening this issue, found a couple of drupal_set_message in MarketingCloudExample->sdubmit_form().

Patch created, please review.

Also added more explanation comments to the response handling.

I swear I had uploaded the patch. Oh well, here it is...

The main steps to take here, are validate that the patch installs, that unit tests still work, are there any unit tests that we can add that will validate the changes in this patch (I don't think there are), and code quality validation - is there a more elegant way rather than using MessengerTrait (I don't think there is unless we consider overriding the constructor, which seems a little overkill for this).

Hi @mahtab_alam, many thanks for your patch. Unfortunately, procedural code like yours (and my code that I'm replacing) is being discouraged in D8. So I can't use this (even though your code is good, terse and easy to read). See:

• https://www.drupal.org/project/drupal/issues/2935153
• https://api.drupal.org/api/drupal/core%21lib%21Drupal.php/function/Drupal%3A%3Amessenger/8.5.x
• https://www.drupal.org/node/2774931

From www.drupal.org/node/2774931:

In rare occasions, you may need to use the Messenger service in procedural functions (hooks, alters, callbacks/static method callbacks, etc.) where the dependency injected Messenger service is not accessible (e.g. no $this->messenger). In these instances, the \Drupal::messenger() static helper method is available for you to use. This helper method should only be used in places where the service cannot be injected and should never be used as a way to create a new service that requires this service.

Many thanks for your input!

Hi @mahtab_alam,

Patch failed QA. It is unable to the class extending WebformHandlerBase, which has a large constructor, and then does not call the parent constructor:

The website encountered an unexpected error. Please try again later.

In addition, the constructor is sent the large number of injected dependencies that WebformHandlerBase expects, but only MessengerInterface is defined in the contract for MarketingCloudExample:

TypeError: Argument 1 passed to Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample::__construct() must implement interface Drupal\Core\Messenger\MessengerInterface, array given, called in /var/www/sites/mc_dev.local/modules/contrib/webform/src/Plugin/WebformHandlerBase.php on line 144 in Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample->__construct() (line 39 of /var/www/sites/mc_dev.local/modules/custom/marketing_cloud/modules/marketing_cloud_example/src/Plugin/WebformHandler/MarketingCloudExample.php) #0 /var/www/sites/mc_dev.local/modules/contrib/webform/src/Plugin/WebformHandlerBase.php(144): Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample->__construct(Array, 'marketing_cloud...', Array, Object(Drupal\Core\Logger\LoggerChannelFactory), Object(Drupal\Core\Config\ConfigFactory), ...

I had a play with this, and got as far as:

<?php

namespace Drupal\marketing_cloud_example\Plugin\WebformHandler;

use Drupal\webform\WebformSubmissionInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\webform\Plugin\WebformHandlerBase;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\webform\WebformSubmissionConditionsValidatorInterface;

...

class MarketingCloudExample extends WebformHandlerBase {

  protected $messenger;

  public function __construct(array $configuration, $plugin_id, $plugin_definition, LoggerChannelFactoryInterface $logger_factory, ConfigFactoryInterface $config_factory, EntityTypeManagerInterface $entity_type_manager, WebformSubmissionConditionsValidatorInterface $conditions_validator, MessengerInterface $messenger) {
    parent::__construct($configuration, $plugin_id, $plugin_definition, $logger_factory, $config_factory, $entity_type_manager, $conditions_validator);
    $this->messenger = $messenger;
  }

...

Which gave me:

TypeError: Argument 8 passed to Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample::__construct() must implement interface Drupal\Core\Messenger\MessengerInterface, none given, called in /var/www/sites/mc_dev.local/modules/contrib/webform/src/Plugin/WebformHandlerBase.php on line 144 in Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample->__construct() (line 46 of /var/www/sites/mc_dev.local/modules/custom/marketing_cloud/modules/marketing_cloud_example/src/Plugin/WebformHandler/MarketingCloudExample.php) #0 /var/www/sites/mc_dev.local/modules/contrib/webform/src/Plugin/WebformHandlerBase.php(144): Drupal\marketing_cloud_example\Plugin\WebformHandler\MarketingCloudExample->__construct(Array, 'marketing_cloud...'...

At that point, I stopped. Looking into other webform handler extensions such EmailWebformHandler, things get quite confusing and will likely require a complete refactoring of the class and defining new parent classes and/or interfaces.

The simplest method may be to stick to patch #17 that just uses a trait to inject the messenger service.

Thoughts, anyone?

Hi @mahtab_alam,

Yes I had read that somewhere too, quite a good decision tree!

If you look at WebformHandlerBase, they already have a logger injected, which was why I was able to use $this->logger. However they have not injected the messenger service, and are still using drupal_set_message() in the extra handlers like the email webform handler that they provide out of the box.

So using $this->logger fixes the same trap I had fallen into, using \Drupal::logger.

IMO, since this is just an example module to show how to use the services, it should be as simple as possible so that devs can read the code and see exactly what they have to do without having to wade through convoluted code just so that we can inject the messenger service.

Many thanks!

So decided then, applying patch #17. Many thanks for you input and suggestions @mahtab_alam!