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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

In 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)

andypost’s picture

In general this change is great except a documentation.

+++ b/core/includes/entity.incundefined
@@ -484,12 +485,16 @@ function entity_form_submit(EntityInterface $entity, $operation = 'default', &$f
+ * @param array $form_state
+ *   (optional) An associative array containing the current state of the form.
  *
  * @return
  *   The processed form for the given entity and operation.
  */
-function entity_get_form(EntityInterface $entity, $operation = 'default', $langcode = NULL) {
-  $form_state = entity_form_state_defaults($entity, $operation, $langcode);
+function entity_get_form(EntityInterface $entity, $operation = 'default', array $form_state = array()) {
+  $form_state += entity_form_state_defaults($entity, $operation);
+  // Add in any other arguments, removing $entity, $operation, and $form_state.
+  $form_state['build_info']['args'] += array_slice(func_get_args(), 3, NULL, TRUE);

Suppose doc-block should be extended by mentioning about ability to get a set of additional parameters

swentel’s picture

Awesome, 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

tim.plunkett’s picture

FileSize
2.31 KB
15.73 KB

Thanks @andypost, I cleaned up the docs.

swentel’s picture

Did 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!

fago’s picture

hm, 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?

tim.plunkett’s picture

The changes are all in entity.inc, the entire rest of the patch is the use case.

tim.plunkett’s picture

Title: Allow arguments to be passed to entity_get_form() » Allow custom form state to be passed to entity_get_form()
FileSize
3.28 KB
13.82 KB

Okay, @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.

Status: Needs review » Needs work

The last submitted patch, form-1913742-8.patch, failed testing.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Status: Needs work » Needs review
FileSize
797 bytes
13.88 KB

Whoops! Forgot that while before it had a default argument in the method, now I need to actually check first.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Damn this is good stuff!

fago’s picture

Status: Reviewed & tested by the community » Needs work

Patch looks good, just one thing:

+++ b/core/includes/entity.inc
@@ -484,12 +483,14 @@ function entity_form_submit(EntityInterface $entity, $operation = 'default', &$f
+ * @param array $form_state
+ *   (optional) An associative array containing the current state of the form.
  *
  * @return
  *   The processed form for the given entity and operation.
  */
-function entity_get_form(EntityInterface $entity, $operation = 'default', $langcode = NULL) {
-  $form_state = entity_form_state_defaults($entity, $operation, $langcode);
+function entity_get_form(EntityInterface $entity, $operation = 'default', array $form_state = array()) {
+  $form_state += entity_form_state_defaults($entity, $operation);

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.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
738 bytes
14.03 KB

Well the $langcode was never documented in the first place, I definitely didn't remove it, so not a regression.
But, I added some anyway.

fago’s picture

Status: Needs review » Reviewed & tested by the community

yes, it was not nice before either, but at least their was an obvious parameter. Docs improvements are good, back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, form-1913742-13.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
14.09 KB

Argh! I had a stale branch locally, that was the patch from #8. Sorry.

catch’s picture

Title: Allow custom form state to be passed to entity_get_form() » Change notice: Allow custom form state to be passed to entity_get_form()
Status: Reviewed & tested by the community » Active

Tidy. Committed/pushed to 8.x, could use a change notice.

jibran’s picture

Issue tags: +Needs change record

Adding tag.

cyborg_572’s picture

Assigned: Unassigned » cyborg_572

Working on a change notification

cyborg_572’s picture

Status: Active » Needs review

Here's my draft for the notice:

Summary

  • Allows entity_get_form() to pass along additional information, to preserve compatibility with use cases covered by drupal_get_form().
  • Cleans up the arguments to entity_get_form() and entity_form_state_defaults().
  • The $langcode argument for entity_get_form() can now be passed as a value in the $form_state array.

Before

<?php
  function entity_form_state_defaults(EntityInterface $entity, $operation = 'default', $langcode = NULL) { }
  function entity_get_form(EntityInterface $entity, $operation = 'default', $langcode = NULL) { }
?>

After

<?php
  function entity_form_state_defaults(EntityInterface $entity, $operation = 'default', array $form_state = array()) { }
  function entity_get_form(EntityInterface $entity, $operation = 'default') { }
?>

impacts: Module Developers

star-szr’s picture

Assigned: cyborg_572 » Unassigned
Priority: Normal » Critical

Bumping to critical for the change notice - can we please get a review on the proposed change notice in #20?

tim.plunkett’s picture

Title: Change notice: Allow custom form state to be passed to entity_get_form() » Allow custom form state to be passed to entity_get_form()
Priority: Critical » Normal
Status: Needs review » Fixed
Issue tags: -Needs change record

Added https://drupal.org/node/2026029. Next time please just add the change notice and mark the issue needs review.

star-szr’s picture

Issue tags: +Needs change record

@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 :

Change notifications are now tweeted and posted to RSS automatically, so we need to check that they are correct before they are submitted. Therefore, ask contributors to post the text in the issue for review.

star-szr’s picture

Zombie tag.

Automatically closed -- issue fixed for 2 weeks with no activity.

xjm’s picture

Issue summary: View changes
Issue tags: -Needs change record