Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
drupal_get_message() and drupal_set_message() replaced by Messenger service, see https://www.drupal.org/node/2774931
drupal_set_message() was partially replaced in #2968718: drupal_set_message function declaration in VerboseMessengerTest prevents the function from being flagged as @deprecated in PHPStorm. This issue will replace the remaining calls.
Proposed resolution
Replace existing calls to the drupal_set_message() function with calls to the Messenger service
Remaining tasks
- Write a patch
- Review
- Commit
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#32 | interdiff_30_32.txt | 438 bytes | subson |
#32 | remove_deprecated_drupal_message-2971259-32.patch | 8.34 KB | subson |
| |||
#30 | remove_deprecated_drupal_message-2971259-29.patch | 8.48 KB | subson |
| |||
#28 | remove_deprecated_drupal_message-2971259-28.patch | 8.48 KB | subson |
#24 | add_dependency_injection_remove_deprecated_drupal_message-2971259-24.patch | 65.79 KB | alesbencina |
|
Comments
Comment #2
nkoporecI'm on it.
Comment #3
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedReassigned.
Comment #4
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedAdded patch to remove deprecated function.
Comment #5
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedReplaced Drupal core message service with pathauto.verbose_messanger.
Comment #6
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedFixed failing tests.
Comment #7
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedCorrected code so assertText does not fail when using Messenger service.
Comment #8
nkoporecTested the latest patch and it looks good. I think we can mark it RTBC.
Comment #9
idebr CreditAttribution: idebr at ezCompany commented@nkoporec / alesbencina Thanks for working on this issue. I recommend installing in your code editor, so you get notified when code violates the Drupal coding standards. You can find instructions at https://www.drupal.org/node/1419988
Doc comment short description must end in a full stop.
This parameter comment is required by the Drupal coding standards
Missing parameter comment
Let's call this property "messenger" in line with Drupal core
A comma should follow the last multiline array item
A comma should follow the last multiline array item
A comma should follow the last multiline array item
A comma should follow the last multiline array item.
Missing parameter comment.
Let's call this property "messenger" in line with Drupal core
Use $this->t instead of t() when possible
A comma should follow the last multiline array item.
Missing short description in doc comment.
Missing parameter comment
Let's call this property "messenger" in line with Drupal core.
Missing short description in doc comment
Missing parameter comment
Visibility must be declared on method "__construct"
Let's call this property "messenger" in line with Drupal core.
Expected one blank line after function, found 0
Use dependency injection in classes when possible.
- Use dependency injection in classes when possible
- Expected one space after comma, found 0.
- A comma should follow the last multiline array item.
Comment #10
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedThank you for reviews. I will fix coding standards.
Comment #11
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedFixed coding standards.
Comment #12
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedFix coding standards..
Comment #13
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedChanged translation class (t to $this->t).
Comment #14
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedComment #15
vakulrai CreditAttribution: vakulrai as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedAdding a patch to fix code errors and dependency injection.
Comment #17
vakulrai CreditAttribution: vakulrai as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedFixed lint errors.
Comment #19
vakulrai CreditAttribution: vakulrai as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedComment #21
vakulrai CreditAttribution: vakulrai as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedComment #23
nkoporecTested #13 patch and the major part of issue mentioned in #12 is fixed.I just found one minor issue:
I think we should inject service \Drupal::translation() too so we can fix one more issue along the way.
@vakulrai I don't understand why are you posting new patches? Did you test and review #13? Also before posting a patch, it's better to run tests locally to check if they pass.
Comment #24
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedInjected string translation manager, where was possible.
@vakulrai please use module "Testing" and run tests locally.
Comment #25
alesbencina CreditAttribution: alesbencina at Agiledrop - Your Trusted Drupal Teammates commentedComment #26
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented@alesbencina Thanks for your hard work on this issue and continues effort of improving the patch. This patch is doing more than the scope of this issue like fixing the coding standard of the code which is not under the scope of this issue and moving the method within the class.
This is called scope creep. The scope of this issue is just to replace drupal_set_message with Messagener service. I'll suggest two things:
MessengerTrait
instead of injecting Messenger service inside constructor. Inside the constructor, you can call$this->setMessenger()
and pass it messenger service. This will reduce the amount of code you need to write. See https://www.drupal.org/node/2774931Comment #27
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commentedComment #28
subson CreditAttribution: subson as a volunteer commentedCreating a patch just to replace drupal_set_message() to use MessengerTrait wherever possible.
Comment #30
subson CreditAttribution: subson as a volunteer commentedFixing failures, need to call messenger() function in PatternEnableForm.php
Comment #31
subson CreditAttribution: subson as a volunteer commentedComment #32
subson CreditAttribution: subson as a volunteer commentedremoving extra line.
Comment #33
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commentedI can confirm the patch #32 applies cleanly and remove all the
drupal_set_message()
with an appropriate injection of Messanger service wherever possible Or useDrupal::service('messanger')
in static method.grep -rn 'drupal_set_message' .
running this command from inside this module directory after applying this patch return no result that means all thedrupal_set_message()
call has been replaced. Moving this to RTBCComment #34
BerdirThis should not be necessary as the parent already has the trait (it also extends from FormBase in the end).
Adding this could cause problems in some PHP versions.
Fixed on commit.