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.
Unlike drupal_get_form(), entity_get_form() does not pass along any other arguments given to it.
That has led to the Views UI attaching arbitrary data to the entity before passing it along, which is not ideal.
Contrib will very likely need to do this as well.
In order to implement this, I also had to clean up the arguments passed to entity_get_form() and entity_form_state_defaults(). entity_get_form() now mirrors entity_submit_form(), and entity_form_state_defaults() is less tailored to one use case.
Comment | File | Size | Author |
---|---|---|---|
#16 | form-1913742-16.patch | 14.09 KB | tim.plunkett |
#13 | form-1913742-13.patch | 14.03 KB | tim.plunkett |
#13 | interdiff.txt | 738 bytes | tim.plunkett |
#10 | form-1913742-10.patch | 13.88 KB | tim.plunkett |
#10 | interdiff.txt | 797 bytes | tim.plunkett |
Comments
Comment #1
tim.plunkettIn addition to fixing Views UI code, removing a WTF, and cleaning up code, it will also help unblock #1913618: Convert EntityFormControllerInterface to extend FormInterface (since entity_form_id() is now only called in entity.inc)
Comment #2
andypostIn general this change is great except a documentation.
Suppose doc-block should be extended by mentioning about ability to get a set of additional parameters
Comment #3
swentel CreditAttribution: swentel commentedAwesome, this might/will pave the way to get #1510544: Allow to preview content in an actual live environment forward - even if it doesn't land in core, I'd be tempted to create a contrib for it. And yes, I can imagine other contrib modules needing this, so +10 from me
Comment #4
tim.plunkettThanks @andypost, I cleaned up the docs.
Comment #5
swentel CreditAttribution: swentel commentedDid a quick test with the patch mentioned in #3 and this works nice, so I'm all for it, so this a ok for me!
Comment #6
fagohm, I'm a bit worried on that as it's impossible to generally embed/use an entity form then, you'd have to know about the arguments.
I'd be curious about the use-case here? Is that really needed?
Comment #7
tim.plunkettThe changes are all in entity.inc, the entire rest of the patch is the use case.
Comment #8
tim.plunkettOkay, @fago talked me out of actually passing stuff through. It makes EntityFormControllerInterface::build() messy, since an entity form shouldn't rely on anything other than the $entity and the form_state.
But, the $form_state changes enable us to accomplish it anyway.
interdiff is against the OP, since the docs changes in #4 were removed wholesale.
Comment #10
tim.plunkettWhoops! Forgot that while before it had a default argument in the method, now I need to actually check first.
Comment #11
dawehnerDamn this is good stuff!
Comment #12
fagoPatch looks good, just one thing:
Here is a regression: No (documented) way to pass in the desired language.
Also, when would I pass the form state and when not? This misses docs.
Comment #13
tim.plunkettWell the $langcode was never documented in the first place, I definitely didn't remove it, so not a regression.
But, I added some anyway.
Comment #14
fagoyes, it was not nice before either, but at least their was an obvious parameter. Docs improvements are good, back to RTBC.
Comment #16
tim.plunkettArgh! I had a stale branch locally, that was the patch from #8. Sorry.
Comment #17
catchTidy. Committed/pushed to 8.x, could use a change notice.
Comment #18
jibranAdding tag.
Comment #19
cyborg_572 CreditAttribution: cyborg_572 commentedWorking on a change notification
Comment #20
cyborg_572 CreditAttribution: cyborg_572 commentedHere's my draft for the notice:
Summary
entity_get_form()
to pass along additional information, to preserve compatibility with use cases covered bydrupal_get_form()
.entity_get_form()
andentity_form_state_defaults()
.entity_get_form()
can now be passed as a value in the$form_state
array.Before
After
impacts: Module Developers
Comment #21
star-szrBumping to critical for the change notice - can we please get a review on the proposed change notice in #20?
Comment #22
tim.plunkettAdded https://drupal.org/node/2026029. Next time please just add the change notice and mark the issue needs review.
Comment #23
star-szr@tim.plunkett - I thought so too (the change notice is from a local sprint I mentored) but see this revision note from https://drupal.org/contributor-tasks/draft-change-notification :
Comment #24
star-szrZombie tag.
Comment #26
xjm