Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Files that need converting are:
- core/modules/search/search.api.php
- core/modules/search/search.module
- core/modules/search/search.pages.inc
- core/modules/search/tests/modules/search_extra_type/search_extra_type.module
Blocked by:
- #1998696: Use Symfony Request for core includes for the destination issue
Comment | File | Size | Author |
---|---|---|---|
#32 | 1999426-search-request-32.patch | 706 bytes | kim.pepper |
#25 | drupal8.base-system.1999426-25.patch | 2.1 KB | andypost |
#23 | 1999426-search-request-23.patch | 4.15 KB | kim.pepper |
#23 | interdiff.txt | 3.65 KB | kim.pepper |
#19 | 1999426-search-request-19.patch | 3.54 KB | kim.pepper |
Comments
Comment #1
zhgenti CreditAttribution: zhgenti commentedComment #2
zhgenti CreditAttribution: zhgenti commentedComment #4
zhgenti CreditAttribution: zhgenti commentedNamespace added
Comment #6
zhgenti CreditAttribution: zhgenti commented#4: use-symfony-request-for-search-module-1999426-3.patch queued for re-testing.
Comment #8
kim.pepperShould this just be 'querystring'?
$request->request is actually the $_POST params. If we aren't sure, we can just use $request->get() and it will search query, post and attributes.
We should create a $request variable and re-use here too.
Let's extract a $request variable here too.
I think you want $request->get() instead of $request->has()
Comment #9
zhgenti CreditAttribution: zhgenti commentedCorrected all issues posted in previous comment. But following part will fail testing:
In a similare issue #1999436: Use Symfony Request for taxonomy module unset($_GET['destination']) was left, do you guys think this is the way it should be done? Perhaps this could be separate issue to refactor all occurrences of destination unsetting?
Comment #11
zhgenti CreditAttribution: zhgenti commentedHey guys, testing failed... Please see my previous comment and suggest how to proceed.
Thanks
Comment #12
kim.pepperHi zhgenti.
Looks like this issue depends on the changes going on in common.inc. #1998696: Use Symfony Request for core includes If you can help out there, that will unblock a number of these conversions.
Thanks,
Kim
Comment #13
zhgenti CreditAttribution: zhgenti commented#9: use-symfony-request-for-search-module-1999426-8.patch queued for re-testing.
Comment #15
zhgenti CreditAttribution: zhgenti commentedComment #16
marcingy CreditAttribution: marcingy commented#15: use-symfony-request-for-search-module-1999426-14.patch queued for re-testing.
Comment #18
kim.pepperNeeds reroll
Comment #19
kim.pepperReroll.
Comment #20
marcingy CreditAttribution: marcingy commentedLooks good
Comment #21
kim.pepper#19: 1999426-search-request-19.patch queued for re-testing.
Comment #22
alexpottshould be query string not querystring - see other documentation in drupal for example the line above :)
The comment needs reflowing as well.
We are in procedural code here no need for the leading \
Comment needs reflowing
Comment #23
kim.pepperThanks Alex. Fixes for #22
Comment #24
star-szrTagging for reroll.
Comment #25
andypostSearch module is changed a lot, so only 2 hunks + new for 'destination'
Comment #26
kim.pepperLooks great!
Comment #27
David Hernández CreditAttribution: David Hernández commented#25: drupal8.base-system.1999426-25.patch queued for re-testing.
Comment #28
alexpottLater on in the same function we are assigning
$request = \Drupal::request();
we should move that above here and use it.Comment #29
kim.pepperLet's wait until #1987806: Convert search_view() to a new style controller goes in. It only leaves us the changes in search.module
Comment #29.0
kim.pepperAdded blocker
Comment #30
cosmicdreams CreditAttribution: cosmicdreams commentedShouldn't be blocked anymore since #1998696: Use Symfony Request for core includes is now resolved.
Comment #31
cosmicdreams CreditAttribution: cosmicdreams commentedI mean #1987806: Convert search_view() to a new style controller
Comment #32
kim.pepperComment #33
kim.pepperRe-rolled
Comment #35
kim.pepper32: 1999426-search-request-32.patch queued for re-testing.
Comment #36
cosmicdreams CreditAttribution: cosmicdreams commentedCouldn't be more simple. Also, I can confirm that these are the only $_GET's in the search module.
Comment #37
Xano32: 1999426-search-request-32.patch queued for re-testing.
Comment #38
Xano32: 1999426-search-request-32.patch queued for re-testing.
Comment #40
XanoThis was apparently fixed somewhere else already. There are no occurrences of $_GET in any of Search's files anymore.