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 #1971384: [META] Convert page callbacks to controllers
For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.
Comment | File | Size | Author |
---|---|---|---|
#22 | drupal8.system-module.1987718-22.patch | 5.83 KB | brianV |
#20 | drupal8.system-module.1987718-20.patch | 5.79 KB | brianV |
#17 | drupal8.system-module.1987718-17.patch | 5.1 KB | brianV |
#15 | drupal8.system-module.1987718-15.patch | 5.16 KB | brianV |
#13 | drupal8.system-module.1987718-13.patch | 5.11 KB | brianV |
Comments
Comment #1
vijaycs85Need to rewrite the whole module to make test sync with current test implementation. For more details, please refer: #1988802: [META] Rewrite test modules in system to provide better unit testing.
Comment #2
ayelet_Cr CreditAttribution: ayelet_Cr commentedComment #3
kgoel CreditAttribution: kgoel commentedGoing to work on this.
Comment #4
xjmThanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.
Comment #5
brianV CreditAttribution: brianV commentedFirst attempt at a patch. Thanks @larowlan for walking me through it so far.
Comment #6
kgoel CreditAttribution: kgoel commentedComment #7
brianV CreditAttribution: brianV commentedAnd... this time without an empty patch file. Sorry all!
Comment #8
brianV CreditAttribution: brianV commentedcross posted
Comment #9
kgoel CreditAttribution: kgoel commentedComment #10
kgoel CreditAttribution: kgoel commentedI think we made edit at the same time.
Comment #11
brianV CreditAttribution: brianV commentedLast cross post of the night ;)
Comment #12
larowlanGreat job, talk about jumping in at the deep end!
chuck a blank line between the namespace and use statements as per http://drupal.org/node/1354
This empty method can go thanks to ControllerBase having all the stuff we need.
Extra blank line can go
missing 'public'
Needs a blank line before last }
Comment #13
brianV CreditAttribution: brianV commentedSee attached, should resolve the above.
Comment #14
larowlannitpick:trailing whitespace
nitpick:can we loose one of these extra lines
Other than that RTBC in my opinion, assuming bot agrees
Comment #15
brianV CreditAttribution: brianV commentedWhat's a nitpick between friends?
Hopefully final revision attached.
Comment #16
larowlansorry I've confused you :( there were lines with whitespace, but the ones at the end of the file had to stay
Comment #17
brianV CreditAttribution: brianV commentedGood. I thought you had suggested otherwise ;)
re-roll!
Comment #18
larowlanUnless bot disagrees.
Comment #20
brianV CreditAttribution: brianV commentedNew version with the two changes:
1. I never removed the original form_test_two_instances() callback. That's done now.
2. EntityManager::getStorageController('node')->create(); doesn't seem to create the node form with the form display settings set by entity_get_form_display()->setComponent()->save() into account. Therefore, the test failed in the above patch because the AJAX field created specifically for that test wasn't being added to the node form.
I asked about #2 in #drupal-contribute a few times, but got no responses, so I've reverted back to entity_create() in the controller to see if that fixes it.
Comment #22
brianV CreditAttribution: brianV commentedThis should work, passes all tests locally.
Turns out the issue was this the permissions for the router item that prevented the test user from seeing the page.
Reverted back to EntityManager::getStorageController('node')->create(); to create the node objects for the node forms.
Comment #23
brianV CreditAttribution: brianV commentedbumping back to NR
Comment #24
larowlanLegend, thanks for coming back and working through it
Comment #25
webchickGreat work! I found a couple of small indentation problems on the comments, but fixed those on commit.
Committed and pushed to 8.x. Thanks!