Follow up for #1971384: [META] Convert page callbacks to controllers and #1913618: Convert EntityFormControllerInterface to extend FormInterface
Whilst we have _entity_form for our defaults in routing, it can't be expected to deal with entity add callbacks, as these need to create a psuedo entity first and the keys they need to set vary (eg menu items may need a plid) or are dynamic (eg node/add/type and block/add/custom_block_type).
This leaves the controller methods for callbacks like block/add/custom_block_type #1978166: Convert block/add/{%custom_block_type} to Controller needing to call entity_get_form from their controllers which breaks our OO goal.
This issue tracks moving entity_get_form to Drupal\Core\Entity\EntityManager::getForm()
Related
Comment | File | Size | Author |
---|---|---|---|
#42 | entity-get-form-1982980-38.patch | 21.86 KB | ParisLiakos |
#42 | interdiff.txt | 2.33 KB | ParisLiakos |
#36 | entity-get-form-1982980-36.patch | 21.98 KB | ParisLiakos |
#36 | interdiff.txt | 2.56 KB | ParisLiakos |
#34 | entity-get-form-1982980-43.patch | 22.54 KB | ParisLiakos |
Comments
Comment #1
larowlanHow goes it bot?
Comment #3
tim.plunkettNeeds to be \Drupal (that's the two failures)
Comment #4
larowlanyeah, sorry picked that up too
one of those days.
Comment #5
ParisLiakos CreditAttribution: ParisLiakos commentedthis is awesome:)
+1000
Comment #6
andypostLet's get #1982984: Create Drupal::entityManager for improved DX first, +1 to commit
Comment #7
dawehnerUpdated the patch
Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedwhy dont we remove this entirely?
Docs are messed up:) some of documentation for $operation is in $form_state
Comment #9
tim.plunkettWe cannot remove it until all of the
'page callback' => 'entity_get_form',
routes are converted.Comment #10
dawehnerThere are currently 7 different menu entries which still use entity_get_form.
Here is just a simple rerole + fix of the good point in #8.
Comment #11
ParisLiakos CreditAttribution: ParisLiakos commentedright i forgot those page callbacks..
this needs a reroll and while at it:
lets fix the double "instead"
Comment #12
larowlanRe-roll and fixes #11 as well as a reference to Drupal::service('plugin.manager.entity') that snuck back in.
Comment #13
dawehnerGreat!
Comment #14
alexpottNo longer applies
Comment #15
ParisLiakos CreditAttribution: ParisLiakos commentedreroll, also replaced one more instance is user.admin.inc, probably just sneaked in
Comment #16
ParisLiakos CreditAttribution: ParisLiakos commentednot needed prefix
Comment #17
ParisLiakos CreditAttribution: ParisLiakos commentedrerolled, replacing more calls that sneaked in the meanwhile
Comment #18
aspilicious CreditAttribution: aspilicious commentedShouldn't we move "entity_form_state_defaults" to the controller too? Or is that for another issue?
Comment #19
dawehnerGood idea in general, though we don't replace entity_form_submit so far, so let's go with this.
Comment #20
ParisLiakos CreditAttribution: ParisLiakos commentedno longer applies
Comment #22
tim.plunkett#20: entity-get-form-00-1982980.20.patch queued for re-testing.
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedComment #24
ParisLiakos CreditAttribution: ParisLiakos commentedchasing HEAD
Comment #25
alexpottUnrelated change.
And the patch no longer applies
Comment #26
ParisLiakos CreditAttribution: ParisLiakos commentedremoved unrelated change and reroll
Comment #27
ParisLiakos CreditAttribution: ParisLiakos commentedback to rtbc
Comment #28
ParisLiakos CreditAttribution: ParisLiakos commentedone more reroll after admin/people view went in
Comment #30
tim.plunkettRerolled after #2015377: Move Controllers in 'Routing' to 'Controller' namespace
Comment #31
ParisLiakos CreditAttribution: ParisLiakos commentedwe are close to 10 rerolls here:P
Comment #32
alexpottHo hum... here we go again...
Comment #33
alexpottComment #34
ParisLiakos CreditAttribution: ParisLiakos commentedconflicted with #1978166: Convert block/add/{%custom_block_type} to Controller
i injected the plugin manager instead and put the request in the controller method since i was touching it
Comment #35
tim.plunkettIf we need the entity manager in addition, so be it. But this is not the right direction to go in for the storage controllers
Comment #36
ParisLiakos CreditAttribution: ParisLiakos commentedyeap, i stand corrected
Comment #38
ParisLiakos CreditAttribution: ParisLiakos commented#36: entity-get-form-1982980-36.patch queued for re-testing.
Comment #39
larowlanshouldn't we be injecting these too?
Comment #40
tim.plunkettGetting rid of the Request is correct, but not the other two
Comment #42
ParisLiakos CreditAttribution: ParisLiakos commentedk lets just swap request with entity manager
Comment #44
tim.plunkett#42: entity-get-form-1982980-38.patch queued for re-testing.
Comment #46
ParisLiakos CreditAttribution: ParisLiakos commented#42: entity-get-form-1982980-38.patch queued for re-testing.
Comment #47
tim.plunkettSixth time's the charm?
Comment #48
alexpottCommitted c4b6658 and pushed to 8.x. Thanks!
I don't think this needs a change notice as we haven't removed
entity_get_form()
yet...Comment #49.0
(not verified) CreditAttribution: commentedUpdated issue summary.