Problem/Motivation

Drupal asked for dependency injection on classes where possible.

Proposed resolution

Inject services:

  • SMTPMailSystem.php
  • ConnectionTester.php
  • SMTPConfigForm.php
CommentFileSizeAuthor
#16 interdiff-11-16.txt2.49 KBthalles
#16 smtp-Use_Dependency_injection_in_classes-3007711-16-D8.patch8.28 KBthalles
#11 smtp-Use_Dependency_injection_in_classes-3007711-11-D8.patch5.68 KBvivek panicker
#8 smtp-Dependency_injection_on_SMTPConfigForm-3007711-8-D8.patch7.94 KBthalles
#5 smtp-Dependency_injection_on_SMTPConfigForm-3007711-5-D8.patch6.16 KBthalles
#3 smtp-Dependency_injection_on_SMTPConfigForm-3007711-3-D8.patch4.5 KBthalles

Comments

Thalles created an issue. See original summary.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review

Follow the patch with refactor code

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Title: Coding standards on SMTPConfigForm » Dependency injection on SMTPConfigForm
Issue summary: View changes
StatusFileSize
new smtp-Dependency_injection_on_SMTPConfigForm-3007711-3-D8.patch4.5 KB

Floow the patch!

Status: Needs review » Needs work

The last submitted patch, 3: smtp-Dependency_injection_on_SMTPConfigForm-3007711-3-D8.patch, failed testing. View results

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
StatusFileSize
new smtp-Dependency_injection_on_SMTPConfigForm-3007711-5-D8.patch6.16 KB

Follow the patch!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: smtp-Dependency_injection_on_SMTPConfigForm-3007711-5-D8.patch, failed testing. View results

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs work » Needs review
StatusFileSize
new smtp-Dependency_injection_on_SMTPConfigForm-3007711-8-D8.patch7.94 KB

Status: Needs review » Needs work

The last submitted patch, 8: smtp-Dependency_injection_on_SMTPConfigForm-3007711-8-D8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

vivek panicker’s picture

vivek panicker commented
Title: Dependency injection on SMTPConfigForm » Use Dependency injection in classes
Assigned: Unassigned » vivek panicker
Issue summary: View changes
Issue tags: +DIACWMay2020
vivek panicker’s picture

vivek panicker commented
StatusFileSize
new smtp-Use_Dependency_injection_in_classes-3007711-11-D8.patch5.68 KB

Path #8 not included as I can see that Dependency Injection has already been done for the Config Form.

vivek panicker’s picture

vivek panicker commented
Assigned: vivek panicker » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: smtp-Use_Dependency_injection_in_classes-3007711-11-D8.patch, failed testing. View results

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Hello @Vivek Panicker!
You need to inject this services by test class SMTPMailSystemTest

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Assigned: Unassigned » thalles
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Assigned: thalles » Unassigned
Status: Needs work » Needs review
StatusFileSize
new smtp-Use_Dependency_injection_in_classes-3007711-16-D8.patch8.28 KB
new interdiff-11-16.txt2.49 KB
vivek panicker’s picture

vivek panicker commented

Great! Thank you @thalles.
The test cases too seem to pass!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

@Vivek Panicker, can you make review for us?

vivek panicker’s picture

vivek panicker commented

Hi thalles,
Applied patch from inside smtp folder using git apply smtp-Use_Dependency_injection_in_classes-3007711-16-D8.patch.
Tested on local on MailHog server.
Generated Test email from SMTP config page. Also generated reset password mail. Mails delivered fine and no errors reported on DBLog page.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Thankful @Vivek Panicker!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Status: Needs review » Reviewed & tested by the community
japerry’s picture

japerry
Location KOMK
commented
Status: Reviewed & tested by the community » Postponed

Will probably get closed because that work is already going on here: #3135595: SmtpConnect() error but email sends successfully

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

But looks me different things

japerry’s picture

japerry
Location KOMK
commented
Status: Postponed » Fixed

You're right! so many connection classes ;) I'm not currently touching this one.

Fixed.

  • japerry committed 52b3849 on 8.x-1.x authored by thalles 
    Issue #3007711 by thalles, Vivek Panicker: Use Dependency injection in...
thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Thanks @all!

vivek panicker’s picture

vivek panicker commented

Happy to contribute to such a useful module! :)

Status: Fixed » Closed (fixed)

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