Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Part of #1971384: [META] Convert page callbacks to controllers
Convert below menu items:
batch_test_chained_form
batch_test_multistep_form
batch_test_simple_form
For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.
Comment | File | Size | Author |
---|---|---|---|
#16 | batch-2132477-16.patch | 16.98 KB | tim.plunkett |
Comments
Comment #1
shameemkm CreditAttribution: shameemkm commentedComment #2
tkuldeep17 CreditAttribution: tkuldeep17 commentedI have converted batch_test_chained_form, batch_test_multistep_form,
batch_test_simple_form into Controller. For this I changed batch_test.routing .yml file. used _form parameter instead of _content in router.
Comment #3
tkuldeep17 CreditAttribution: tkuldeep17 commentedI am attaching patch which contains controller of these menu items..
Comment #4
dawehnerWe could use 'redirect_route' here instead
Comment #5
tim.plunkettThanks for the patch!
Next patch, can you mark the issue as "needs review"?
These should be redirect_route still
Our convention has been to put buildForm before submitForm
This is missing a '
These are indented wrong, but also the functions should be converted to methods on this class.
Comment #6
tkuldeep17 CreditAttribution: tkuldeep17 commentedThanks dawehner, tim.plunkett for reviewing patch..
@tim.plunkett When I tried to add these submit handlers into Class, drupal was not recognizing methods. So could you please help me how to convert these function into methods.
Comment #7
tkuldeep17 CreditAttribution: tkuldeep17 commentedSubmitting updated patch..
Comment #9
tkuldeep17 CreditAttribution: tkuldeep17 commentedSorry, First I had to test it locally, but now I have tested it locally, all test cases are passing. So again I am submitting updated patch..
Comment #10
tkuldeep17 CreditAttribution: tkuldeep17 commentedSubmitting new patch. This patch is now locally tested, all test cases are passing..
Comment #11
tkuldeep17 CreditAttribution: tkuldeep17 commentedUpdated patch.. Now I have converted
into
Comment #12
tkuldeep17 CreditAttribution: tkuldeep17 commentedFor creating Form object implemented
FormInterface
instead ofextending
FormBase
,As here we only need very simple form, not required dependencyInjection. it was just creating Form with additional functionality which was not required.
And also converted
batch_test_mock_form
into Form ObjectDrupal\batch_test\Form\BatchTestMockForm
.as it was not listed in issue, but I thought this must also converted :-)
Comment #14
tkuldeep17 CreditAttribution: tkuldeep17 commentedSorry, I was wrong, We have to create form using extending
Drupal\Core\Form\FormBase
. and also when I convertingbatch_test_mock_form
into Form ObjectDrupal\batch_test\Form\BatchTestMockForm
. Test cases are failing, why it is happening not able to find our reason.. If any one, then please share here..So please consider my previous patch https://drupal.org/comment/8222485#comment-8222485
Comment #15
RoSk0Comment #16
tim.plunkettWe should use FormBase.
Comment #17
tim.plunkettComment #18
jibranUnable to find anything objectionable so RTBC.
Comment #20
tim.plunkettEntityCrudHookTest random fail.
Comment #21
alexpottCommitted 37e61b1 and pushed to 8.x. Thanks!