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.
Part of #2924538: [META] Remove all usages of drupal_set_message and drupal_get_messages
grep -inr -e "drupal_set_message" -e "drupal_get_message" core/modules/[a-f]
Comment | File | Size | Author |
---|---|---|---|
#46 | interdiff-43-46.txt | 640 bytes | jofitz |
#46 | 2935256-46.patch | 311.35 KB | jofitz |
#43 | interdiff-40-42.txt | 4.13 KB | jofitz |
#43 | 2935256-42.patch | 311.35 KB | jofitz |
#40 | interdiff-38-40.txt | 1.42 KB | jofitz |
Comments
Comment #2
kim.pepperInitial patch split from parent issue. Blocked on messenger trait in #2935255: Remove all usages of drupal_set_message and drupal_get_messages in core lib
Comment #4
kim.pepperUnblocked as messenger trait was added in #2937945: Add messenger to ControllerBase and FormBase
Comment #5
kim.pepperReroll
Comment #7
kim.pepperAdds missing messenger.
Comment #9
kim.pepperFixing test fails.
Comment #10
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedPatch applies cleanly. The requirements needed are satisfied.
Comment #11
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedPatch applies cleanly. The requirements needed are satisfied.
Comment #14
shubhangi1995Comment #15
kim.pepperPostponed on #2942068: FileTestSaveUploadFromForm incorrectly counts messages
Comment #16
jibranComment #17
jibranComment #18
shubhangi1995Comment #19
voleger#2942068: FileTestSaveUploadFromForm incorrectly counts messages already in core
Comment #20
volegerRerolled regarding last changes.
core/modules/file/tests/file_test/src/Form/FileTestSaveUploadFromForm.php
- calls already replaced. Removed changes from the patch.core/modules/content_moderation/src/EntityTypeInfo.php
- content moderation changes had removed single usage of DSM. So, removed changes from the patch for this file.core/modules/field/tests/modules/field_test/field_test.module
- rerolled regarding #2940201: hook_field_widget_form_alter() can no longer affect the whole widget for multi-value fieldsComment #21
kim.pepperThanks @voleger. I ran
grep -inr -e "drupal_set_message" -e "drupal_get_message" core/modules/[a-f]*
and found a few missing converstions and comments:
Comment #22
volegerThank's @kim.pepper for review.
Added missing replacements.
Comment #23
kim.pepperAddresses all feedback to RTBC
Comment #24
catchPlease combine the patches in #2935258: Remove all usages of drupal_get_message and drupal_set_message in modules M-R and #2935259: Remove all usages of drupal_get_message and drupal_set_message in modules S-Z into this one so all the module changes can be reviewed together. I think it's fine to do core/lib separately.
Comment #25
catchComment #26
yogeshmpawar@catch - i will combine all three patches
Comment #27
yogeshmpawar@catch - I have combined all three patches & removed the core/lib separately as mentioned in #24 but i think few usages #2935257: [PP-1] Remove all usages of drupal_get_message and drupal_set_message in modules G-L are still there.
Comment #28
yogeshmpawarAs discussed with @catch, here is the latest updated patch to remove all usage of drupal_get_message & drupal_set_message in modules, only two occurrences left when we check with
grep -inr -e "drupal_set_message" -e "drupal_get_message" core/modules/*
as mentioned in #16.Hopefully it pass all the test cases.
Comment #31
ibustosComment #32
ibustosOops.
Comment #33
volegerComment #34
volegerCleanup after reroll in #33.
Comment #37
Girish-jerk CreditAttribution: Girish-jerk at Valuebound commentedComment #38
jofitz CreditAttribution: jofitz at ComputerMinds commented@Girish-jerk hasn't touched this for 3 weeks so I'll have a go.
Corrected the failing test (after a minor re-roll).
Comment #39
kim.pepperThanks for picking this up again @Jo Fitzgerald
It's not great having to review a 306K patch, but here we are!
I've reviewed this a few times in the sub-issues that were merged into this one, so scanning through, I could only find one obvious issue.
This method and usages are not necessary if we use messenger directly.
Comment #40
jofitz CreditAttribution: jofitz at ComputerMinds commentedResolved the issue that @kim.pepper raised in #39.
A quick search suggests there are plenty of instances of
drupal_set_message()
remaining in the codebase (e.g. Drupal\Core\Form\EventSubscriber\FormAjaxSubscriber::drupalSetMessage) - do any of these have reason to remain? I'm fast going off this ticket!Comment #42
volegerI guess this method should not be removed for now. Maybe better mark that method internal or deprecated?
Comment #43
jofitz CreditAttribution: jofitz at ComputerMinds commentedUpdate tests to reflect removal of drupalSetMessage().
Comment #44
kim.pepperLooking great.
I ran a check for instances of drupal_set_message and drupal_get_message in the core/modules directory.
Those are all valid instances and can remain.
Nit: Should really be ::addMessage().
Other than that, I think this is ready.
Comment #45
voleger#44: +1
Comment #46
jofitz CreditAttribution: jofitz at ComputerMinds commentedCorrected the nit from #44.
Let's push this one across the line and forget this huge patch forever!
Comment #47
kim.pepperComment #48
voleger+1 for RTBC
Comment #50
kim.pepperDrupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTest failed. Retesting.
Comment #51
kim.pepperRandom bot fail. Setting back to RTBC
Comment #52
alexpottCrediting @jibran and @catch for reviewing this and the other patches that have been merged into this one.
Comment #53
alexpottCommitted b84b827 and pushed to 8.6.x. Thanks!
Comment #56
alexpottThis broke PHP 5.5 testing. I hotfixed because it is the 8.6.x branch and committing this patch once is enough disruption.