Problem/Motivation

FILE: ...www/html/drupal_cms/drupal9/web/modules/contrib/symfony_mailer/tests/modules/symfony_mailer_legacy_test/src/Form/LegacyTestEmailForm.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 58 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/drupal_cms/drupal9/web/modules/contrib/symfony_mailer/src/Form/PolicyAddForm.php
----------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------
 20 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------


FILE: /var/www/html/drupal_cms/drupal9/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/WrapAndConvertEmailAdjuster.php
----------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------
 143 | WARNING | Unused variable $titles.
----------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/drupal_cms/drupal9/web/modules/contrib/symfony_mailer/src/Plugin/EmailAdjuster/AddressAdjusterBase.php
--------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------
 94 | WARNING | Unused variable $display.
--------------------------------------------------------------------------------------------------------------------------

Time: 1.62 secs; Memory: 14MB
CommentFileSizeAuthor
#12 interdiff_10-12.txt1.93 KBlucassc
#12 3318143-12.patch4.29 KBlucassc
#10 dependency-injection-3318143-10.patch4.22 KBrohit rana
#4 dependency-injection-3318143-4.patch5.91 KBrohit rana
#2 dependency-injection-3318143-2.patch5.27 KBrohit rana

Comments

Rohit Rana created an issue. See original summary.

rohit rana’s picture

rohit rana commented
StatusFileSize
new dependency-injection-3318143-2.patch5.27 KB
rohit rana’s picture

rohit rana commented
Issue summary: View changes
rohit rana’s picture

rohit rana commented
StatusFileSize
new dependency-injection-3318143-4.patch5.91 KB
rohit rana’s picture

rohit rana commented
Assigned: rohit rana » Unassigned
Status: Active » Needs review
WagnerMelo’s picture

WagnerMelo commented
Assigned: Unassigned » WagnerMelo

Hi, i'll review it. XD

WagnerMelo’s picture

WagnerMelo commented
Assigned: WagnerMelo » Unassigned
Status: Needs review » Reviewed & tested by the community

Hi, i reviewed this issue, and test the patch sended by @Rohit Rana on #4, and all dependency injections was solved, and the module keep work correctly after the changes in the code.
So i'll move this issue to RTBC! XD

adamps’s picture

adamps commented
Status: Reviewed & tested by the community » Needs work

Great thanks.

1. Please sort use statements alphabetically.

use Drupal\Core\Form\FormStateInterface;
 use Drupal\symfony_mailer\Entity\MailerPolicy;
 use Drupal\Core\Url;
+use Drupal\symfony_mailer\Processor\EmailBuilderManager;
+use Symfony\Component\DependencyInjection\ContainerInterface;

and

use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Mail\MailManagerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\Theme\ThemeManagerInterface;

2. Please remove the changes that aren't part of this issue and anyway seem incorrect.

     return new static(
-      $configuration,
-      $plugin_id,
-      $plugin_definition,
-      $container->get('renderer'),
-      $container->get('module_handler'),
+     $configuration,
+     $plugin_id,
+     $plugin_definition,
+     $container->get('renderer'),
+     $container->get('module_handler'),

and

-      $display = $item['display'];

and

-    $titles = [
-      'plain' => $this->t('Plain text'),
-      'swiftmailer' => $this->t('Emulate swiftmailer'),
-    ];

I agree this last bit of code is not currently used, but probably it should be used, so let's not just delete it.

rohit rana’s picture

rohit rana commented
Assigned: Unassigned » rohit rana
rohit rana’s picture

rohit rana commented
Assigned: rohit rana » Unassigned
Status: Needs work » Needs review
StatusFileSize
new dependency-injection-3318143-10.patch4.22 KB

Hi @AdamPS ,
I made changes as per your suggestion. Please Review.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Great thanks. Sorry, I spotted one more thing:

The patch contains:

+    $this->emailBuilder = $email_builder;

The existing code contains either of:

$this->builderManager = $email_builder_manager;
$this->emailBuilderManager = $email_builder_manager;

Please can you copy one of the existing ones?

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
StatusFileSize
new 3318143-12.patch4.29 KB
new interdiff_10-12.txt1.93 KB

Hi! I replaced $this->emailBuilder = $email_builder with $this->builderManager = $email_builder_manager.

Please, review.

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
Status: Needs work » Needs review

  • AdamPS committed 5d3d2461 on 1.x 
    Issue #3318143 by Rohit Rana, lucassc: Use dependency injection
adamps’s picture

adamps commented
Status: Needs review » Fixed

Great thanks. I changed two comments a tiny bit for consistency.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.