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.
Drupal 8.5.x has introduced a new messenger service.
recently drupal_set_message() has been deprecated
#2760167: Add \Drupal\Core\Messenger\Messenger - and continual maintenance patches are better than a single monster patch.
Comment | File | Size | Author |
---|---|---|---|
#35 | interdiff-2931217-34-35.txt | 454 bytes | martin107 |
#35 | 2931217-35.patch | 14.85 KB | martin107 |
| |||
#19 | 2931217-19.patch | 19.39 KB | martin107 |
|
Comments
Comment #2
martin107 CreditAttribution: martin107 as a volunteer commentedone less hidden dependancy.
Comment #4
martin107 CreditAttribution: martin107 as a volunteer commentedupdated this a little.
Updating the issue summary to highlight that this patch valid on Drupal 8.5.x but that this will break on Drupal 8.4.x
Comment #6
martin107 CreditAttribution: martin107 as a volunteer commentedfixing another minor err - while I see it.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman commentedI'm not sure its prudent to commit this now, as providing bug fixes for 8.4 will be more complicated (requires backports). Also, this patch should require 8.5 in devel.info.
Comment #9
willzyx CreditAttribution: willzyx commented@martin107 thanks for the patch! as stated in #8 we need to postpone this issue for now
Comment #10
martin107 CreditAttribution: martin107 as a volunteer commentedAs well as being postponed .. I think the patch needs to be discarded and a alternative approach taken.
When this patch lands ...
#2935255: Remove all usages of drupal_set_message and drupal_get_messages in core lib
all the cases in the patch extend FormBase or ControllerBase so we pick up the MessengerTrait
- drupal_set_message("Sample text");
+ $this->messenger()->addStatus("Sample text");
Comment #11
martin107 CreditAttribution: martin107 as a volunteer commentedSo my intent here is to work on this again after 3 months ....
Get it into a converted working state and then postpone again while we talk about the correct time to commit this....
Today core ( 8.6.x ) has committed conversions of a bunch of these.
Specifically in relation to this issue I see that
#2935256: Remove all usages of drupal_get_message and drupal_set_message in modules
has given ConfigFormBase the MessengerTrait - which resolves the issues I was talking about in #10
Comment #12
martin107 CreditAttribution: martin107 as a volunteer commenteda) From #8
As appropriate, I have modified three info files.
b) As per the change record, the messenger service should probably be directly injected into any custom service rather than using the trait functionality.
c) I have simplified the constructor's of classes that extend base classes.
d) Something new to this issue ... I had to refactor devel_generate/devel_generate.batch.inc
e) See DevelGenerateBase there was a minor maintenance hazard - $type defaulted to 'sting' --- if MessengerInterface ever did anything crazy like updated the value of TYPE_STATUS .. we would be out of sync with that craziness. So I made this change.
So now we have to discuss Moshe Weitzman's "prudent" comment from #8
PS The last patch need a reroll, I said I was going to start from scratch in any event so no interdiff.
Comment #13
martin107 CreditAttribution: martin107 as a volunteer commentedGo testbot go.
Comment #15
martin107 CreditAttribution: martin107 as a volunteer commentedMy bad,
$this->messenger is wrong
$this->messenger() is good
I cut and paste that from the change record... so I am about to update the change record...sorry for the distraction.
Comment #17
martin107 CreditAttribution: martin107 as a volunteer commentedLocally DevelQueryDebugTest now passes.
Comment #18
msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commentedcore key should always specify major version of drpual like 7.x, 8.x not the specific release of core.
https://www.drupal.org/docs/8/theming-drupal-8/defining-a-theme-with-an-...
There are few more places where
drupal_set_message
is left.Comment #19
martin107 CreditAttribution: martin107 as a volunteer commentedThanks for the quick review... I hope I have corrected everything.
Comment #20
Prashant.c@martin107
Great work.
I tested it for couple of scenarios and it worked fine.
I have a question should we consider MessengerTrait for the same purpose ?
Comment #21
martin107 CreditAttribution: martin107 as a volunteer commentedA decision tree for when to work with traits
1) If it is a service you are creating then avoid the trait and directly inject into the constructor
2) If it is a small controller method, and it is appropriate to extend ControllerBase, then look for the trait in the base class
3) If what you are writing can be considered application code, then use traits
4) Testing can become complicated. so if you need 100% coverage then consider converting a trait into DI logic.
I hope this helps.
Comment #22
Prashant.cThanks @martin107
This clarifies and definitely very helpful.
Comment #23
moshe weitzman CreditAttribution: moshe weitzman commentedComment #24
martin107 CreditAttribution: martin107 as a volunteer commentedWith a DrupalCon starting... I should say I am working on this.
Comment #25
martin107 CreditAttribution: martin107 as a volunteer commentedComment #26
martin107 CreditAttribution: martin107 as a volunteer commentedI had to undo do dependency injection ( in the constructor ) because head had already converted many classes to use the trait. No biggie.
Comment #27
thallesLooks good!
Comment #28
thallesI took the opportunity to replace the syntax of the matrices in the files involved.
Comment #29
thallesComment #30
moshe weitzman CreditAttribution: moshe weitzman commentedFails to apply due to today's changes. sorry.
Comment #31
martin107 CreditAttribution: martin107 as a volunteer commentedFirst step, reroll.
No conflicts just auto merging.
Comment #32
martin107 CreditAttribution: martin107 as a volunteer commentedComment #33
martin107 CreditAttribution: martin107 as a volunteer commentedNote to self -- when there a DrupalCon going I must pull everyday.
Comment #34
martin107 CreditAttribution: martin107 as a volunteer commentedA little fix.
Comment #35
martin107 CreditAttribution: martin107 as a volunteer commentedAnother fix.
Comment #36
thallesLooks good!
Comment #37
willzyx CreditAttribution: willzyx commentedRetitling
Comment #38
willzyx CreditAttribution: willzyx commentedComment #39
amarphule CreditAttribution: amarphule at DevsAdda for DevsAdda commented#35 applied clean and works for me on Drupal-8.6.15 and devel 8.2-2.x
Comment #41
moshe weitzman CreditAttribution: moshe weitzman commented