Active
Project:
Drupal core
Version:
main
Component:
entity system
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
14 Aug 2012 at 01:28 UTC
Updated:
11 Apr 2023 at 07:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
fagoand we need to make sure that we can embed multiple forms at the same time!
Comment #2
sunThis partially duplicates a couple of other issues:
#367006: [meta] Field attach API integration for entity forms is ungrokable
#766146: Support multiple forms with same $form_id on the same page
(...and more...)
Comment #3
effulgentsia commentedI'm interpreting this issue as wanting to put the $form returned by EntityFormControllerInterface::build() into some other $form such that the HTML output is a single
<form>tag. Is that correct, or is this about wanting multiple $form arrays sharing the same $form_id (e.g., several entities of the same type) to be each rendered as their own<form>tags, but to not have id / form processing collisions when all rendered on the same page?Comment #4
eclipsegc commentedfwiw, I'm interpreting this as meaning that I could put any (or many) entity forms into the same form and determine what sort of entity forms existed within this and hand off processing of that separately. Something like:
And then be able to iteratively pass the form state to entity_form_submit and have it process the various entity forms embedded in the greater form. Is this a correct interpretation?
Eclipse
Comment #5
plachI think @fago's plan is to support both scenarios in #3.
@fago:
Is this correct?
@EclipseGC:
I think your example is not correct because
entity_get_form()returns an already processed form ready to be rendered. As pointed out in #3 the correct way should be using the value returned byEntityFormControllerInterface::build().Comment #6
fagoI was thinking of what is described in #4. Avoinding form id collisions is something different we might want to fix generally elsewhere.
Comment #7
andypostSuppose menu_overview_form is nice example of 2 forms
Comment #8
yched commentedI missed that issue so far, interesting, it's precisely the kind of things I'm about to bump into in #1992138: Helper issue for "field types as TypedData Plugins".
Late in the D7 cycle, we had quite some work to make sure Field API form integration (include widgets, extract values, validate and assign errors back to form elements) works with forms containing several entities. field_test_entity_nested_form() is a simple example in core right now, and IIRC, field_collection relies on this mechanism.
Not sure how this can play with EntityFormControllers that include more and more entity logic. #1992138: Helper issue for "field types as TypedData Plugins", among other things, removes field_attach_form_validate(), and moves the equivalent code in the EFC - and right now I'm not sure how to make the tests around field_test_entity_nested_form() pass again.
Comment #9
berdirTagging.
Comment #10
xanoSee #2006248: Add an EmbeddableFormInterface so that FormInterface can delegate to multiple objects for a generic solution.
Comment #11
fagoGiven entity forms need to replace field_attach_form() we need to fix this for entity forms being a full replacement.
Comment #12
yched commentedDiscussion in #2090509: Optimize entity_get_render_display() for the case of "multiple entity view" derived on "Edit in place"'s EditFieldForm, and from here "inline entity form" - see #2090509-28: Optimize entity_get_render_display() for the case of "multiple entity view".
The current situation is :
- Base fields can now be rendered by widgets.
- The ones that aren't are rendered through custom code (either in EntityFormController:buildForm() or in form alters) that is only triggered by "the official entity form with the right form controller and form_id"
- It's difficult to abstract those out and decide whether they should also be applied to an inlined form or a "single field" form.
So:
Wouldn't it just be good enough to provide easy support for "include field widgets for an entity in your custom form/subform" (with validation and extraction of values) ? - i.e simply our current field_attach_form*() refreshed for OO ?
Does "inline entity form" do anything else than rendering the field widgets right now ?
Comment #13
yched commented#2095195: Remove deprecated field_attach_form_*() has an initial patch with those "generic widget attachers". The question is: where do we put them...
Comment #14
berdirI don't know what else is left to do here?
NestedEntityTestForm is an example and test coverage that we can embed/edit multiple entities in the same form.
I know that profile2 is successfully using a similar approach (based on that example), the display profile edit forms within the user registration form.
Comment #15
yched commentedTo clarify :
We made sure is still possible is to attach the (widgets for the) fields of various entities at various places within one form. This is what NestedEntityTestForm tests (should probably be renamed...). In D7 terms, you can call field_attach_form() for various entities in various places in your form.
Granted, in D8, that gives you much more than in D7, but that's still limited to "the generic field / widget behavior".
You can edit the fields of two entities in your form, but the "subforms" won't leverage any of the logic that is present in the EntityForm / ContentEntityForm / [MyEntityType]Form classes. And that logic is currently a very intricated mix of a) entity massaging stuff that is relevent whenever an entity is created / editied in a form and b) UI-like stuff that is really related to "this form being the top-level form" (redirects, dsm()s...)
As a side note, NestedEntityTestForm currently only works with two pre-existing entities. If it wanted to allow creation of new entities (which is not relevant to its testing scope), it would have to do some additional legwork of its own.
This being said :
I happened to bump into @bojanz (who maintains Inline Entity Form) recently, and talked with him about this, he said at worst he still planned to have EntityType-specific handler classes for integration with Inline Entity Form.
So basically, the situation is not worse that in D7. Entity forms are not nestable, integrating your entity type with Inline Entity Form requires you to provide some specific integration class. There will be a separate InlineEntityFormController / [MyEntityType]InlineFormController stack that duplicates some of what's done in EntityForm / ContentEntityForm / [MyEntityType]Form for "main forms". Not ideal, but not worse than D7, core just won't have allowed progress on that side, we'll live...
In short : yeah, probably a won't fix...
Comment #16
fagoGiven #2010930: [META] Apply formatters and widgets to rendered entity base fields (all besides node.title) the question is whether tacking this issue is still required / needed. Given most stuff can move into the entity display form which is already embeddable, it should be enough to have only entity display forms as embeddable entity forms.
Then, the full entity forms would be just he conrete incarnation of the entity form display, whereas subforms somewhere else would be different incarnations. Given that it makes sense to have different form alters, i.e. having the main entity form alter not in the sub-form. The right way for modules to add something would be widgets anyway.
Then, I guess the question remains whether we need to have a generic alter accross forms generated from entity form displays. Given we had hook_field_attach_form() it even seems to be a regression to not have it right now, or do we?
However, for #2010930: [META] Apply formatters and widgets to rendered entity base fields (all besides node.title) to work out nicely we have to solve it's problematic areas. Discussing it with Wimleers and berdir, it seems it boils down to mostly
- groups
- help texts
- other non-field stuff which needs a widget like revision logs
Thus, we'd need #1875974: Abstract 'component type' specific code out of EntityDisplay solved - what I'd assume fieldgroup needs anwayway.
Given that, I think it's better to focus on moving more stuff into display and have that as re-usable, embeddable entity form only. Does that seem feasible for inline entity form?
Comment #17
yched commentedDigging in #2095195: Remove deprecated field_attach_form_*() :
- @yched in #79:
- @fago in #79-2:
- @yched in #80:
(and the comment's interdiff patch removes the "@todo what about hook_field_attach_form()")
- @tim.plunkett in #108 (post commit)
So, according to #80, we concluded in an IRC meeting (probably on feb. 20th 2014 ?) that we would just ditch hook_field_attach_form(). IIRC, it was more like "no clear use case atm, let's remove it for now and revisit if needed".
@Berdir, @amateescu, rings a bell ?
Comment #18
yched commentedLooking through my logs (feb. 21st 2014, actually), that's exactly what happened - "no clear case, we'll see in #1728816: Ensure multiple entity forms can be embedded into one form" :-)
Attaching the relevant part of the discussion.
Comment #19
yched commentedOther than that, yes, the more processing gets handled at the widget / entity form display level, the less custom code to write in dedicated IEF form controllers...
Comment #20
yched commented@Wim Leers just wrote this in #2226493-145: Apply formatters and widgets to Node base fields :
:-)
Comment #21
fagoYeah, we figured #access handling is taken care of by entity field access, for which we've to do #2028027: [META] Missing access control for base fields anyway.
Comment #22
bojanz commentedThat's great progress, everyone!
I can confirm #19.
Thank you yched for fighting for my pet module :)
Comment #23
swentel commentedComment #24
swentel commentedComment #25
xjmThis issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.
This could potentially be implemented in a minor with BC, so moving to 8.2.x.
Comment #32
kenton.r commentedSo the idea of having "embeddable entity forms (subforms)" that can be added and managed in the form displays would be great #16. Has this issue lost any attention. Is there another way to have multiple entity forms within one form?
Comment #37
geek-merlinCurrently working in IEF ointo that direction.
Comment #41
hudri