Problem/Motivation

As per Drupal best practises. "\Drupal calls should be avoided in classes, use dependency injection instead"

Steps to reproduce

Proposed resolution

Use dependency injection for following files.
Sms.php
UserSettingsForm.php

CommentFileSizeAuthor
#13 Usedependencyinjection-3358358-13.patch7.01 KBnishant
#4 3358358-3.patch4.78 KBkaustab_roy
#3 3358358-3.patch3.82 KBkaustab_roy

Issue fork twilio-3358358

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

shashank5563 created an issue. See original summary.

shiv_sharma’s picture

shiv_sharma commented
Assigned: Unassigned » shiv_sharma

I am checking this issue

kaustab_roy’s picture

kaustab_roy commented
StatusFileSize
new 3358358-3.patch3.82 KB

I have a created a patch, review once with it.
Affected files:
Sms.php
UserSettingsForm.php

kaustab_roy’s picture

kaustab_roy commented
StatusFileSize
new 3358358-3.patch4.78 KB

Added fixed for the controller issue on send test message sms issue in the patch.
sorry i didn't saw it was already assigned to someone.
Kindly review it as well.

Nishant made their first commit to this issue’s fork.

nishant’s picture

nishant
Primary language English
commented
Assigned: shiv_sharma » nishant

Nishant opened merge request !10

nishant’s picture

nishant
Primary language English
commented

Created MR #10, Please review.

shiv_sharma’s picture

shiv_sharma commented
Version: 8.x-2.0-alpha9 » 8.x-3.0-alpha7
Status: Needs work » Reviewed & tested by the community

Reviewed the PR #06 working fine on drupal.
Drupal 10.0.9
PHP 8.1.16

shashank5563’s picture

shashank5563 commented

@nishant, can you remove the unused class from UserSettingsForm.php file.

Also, add the Class Constructor description in the file. 

  /**
   * Class Constructor.
   *
   */
nishant’s picture

nishant
Primary language English
commented

Will do

shashank5563’s picture

shashank5563 commented
Status: Reviewed & tested by the community » Needs work
nishant’s picture

nishant
Primary language English
commented
StatusFileSize
new Usedependencyinjection-3358358-13.patch7.01 KB

Created Patch. Please review.

nishant’s picture

nishant
Primary language English
commented
Status: Needs work » Needs review

shashank5563’s picture

shashank5563 commented
Version: 8.x-3.0-alpha7 » 8.x-3.0-alpha9
Status: Needs review » Fixed
shashank5563’s picture

shashank5563 commented
Status: Fixed » Closed (fixed)