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 |
---|---|---|---|
#21 | common_test-1987644-21.patch | 3.09 KB | dawehner |
#18 | drupal8.system-module.1987644-18.patch | 3.12 KB | mparker17 |
#14 | drupal8.system-module.1987644-14.patch | 3 KB | mparker17 |
#14 | interdiff.txt | 817 bytes | mparker17 |
#11 | drupal8.system-module.1987644-11.patch | 3 KB | mparker17 |
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
mparker17I'll help!
Comment #4
mparker17Try this...
Comment #5
dawehnerLet's also put a @return statement in there.
check_plain can be replaced by String::checkPlain()
Comment #6
dawehnerLet's also put a @return statement in there.
check_plain can be replaced by String::checkPlain()
Comment #7
mparker17Try this...
Comment #8
dawehnerSorry, but pattern should start with "/"
... these parameters should be absolute, so \Symfony\Component\HttpFoundation\Response ...
Comment #9
mparker17K
Comment #10
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 #11
mparker17Try this...
Comment #12
dawehnerMissing trailing "\" in the @return statement.
The rest looks perfect!
Comment #13
mparker17Trailing slash or leading slash? :)
Comment #14
mparker17Try this instead...
Comment #16
mparker17#14: drupal8.system-module.1987644-14.patch queued for re-testing.
Comment #18
mparker17Straight re-roll of the patch...
Comment #19
dawehnerThat is fine ... even we should rather convert drupal_get_destination() to something else, but yeah this is a follow up.
Comment #20
webchickSorry, no longer applies.
Comment #21
dawehnerLet's move the function into the existing controller.
Comment #22
mparker17Hey @dawehner,
I had put this into a different controller based on a conversation with @Crell at the Midwest Drupal Developer Summit, where we decided it would be helpful to have separate controller objects for different return types.
However, I don't think it's worth holding up the patch for that... I really want to see the old routing system gone!
The code looks good.
The tests pass.
Manual testing works.
Looks RTBC! Thanks for all your help!
Comment #23
webchickCommitted and pushed to 8.x. Thanks!