If you use the multiple widget and click "edit" on more than one form to open, using the submit handler on that form also closes all siblings, throwing away any work there. This is due to logic introduced to handle nested forms -- inline_entity_form_close_child_forms calls inline_entity_form_close_all_forms, which includes ALL elements in the IEF hierarchy. It should actually just close all child forms instead.

Patch attached -- tested on the multiple form with and without nested forms. Saves when you save, cancels when you cancel.

CommentFileSizeAuthor
#24 interdiff.txt644 bytesm4olivei
#24 2343689-cancel_only_the_chosen_form.patch8.62 KBm4olivei
#21 interdiff.txt916 bytesm4olivei
#21 2343689-cancel_only_the_chosen_form.patch8.15 KBm4olivei
#18 interdiff.txt5.02 KBm4olivei
#18 2343689-cancel_only_the_chosen_form.patch7.26 KBm4olivei
#15 interdiff.txt4.39 KBm4olivei
#15 2343689-cancel_only_the_chosen_form.patch6.61 KBm4olivei
#12 interdiff.txt5.31 KBm4olivei
#12 2343689-cancel_only_the_chosen_form.patch4.95 KBm4olivei
#10 2343689-cancel_only_the_chosen_form.patch1.87 KBm4olivei
#5 2343689-cancel_only_the_chosen_form.patch1.87 KBm4olivei
#4 2343689-ief-nested-close-all-4.patch1.22 KBfearlsgroove
ief-multiple-forms-closing.patch632 bytesfearlsgroove

Comments

bojanz’s picture

bojanz commented
Status: Needs review » Fixed

Committed, thanks.

  • bojanz committed 93efa62 on 7.x-1.x authored by fearlsgroove 
    Issue #2343689 by fearlsgroove: Fixed Multiple forms submit/cancel...

Status: Fixed » Closed (fixed)

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

fearlsgroove’s picture

fearlsgroove commented
Status: Closed (fixed) » Needs review
StatusFileSize
new 2343689-ief-nested-close-all-4.patch1.22 KB

I'm not sure why this worked before, but it's definitely broken now. Attached patch ensures that only child forms of the delta that's being submitted are closed. Lightly tested with all the scenarios I could think of -- 2 levels of nesting.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Version: 7.x-1.x-dev » 8.x-1.x-dev
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch1.87 KB

Here is a patch for Drupal 8. So far working for me. Anyone out there want to check this out?

Status: Needs review » Needs work

The last submitted patch, 5: 2343689-cancel_only_the_chosen_form.patch, failed testing.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs work » Needs review

The last submitted patch, ief-multiple-forms-closing.patch, failed testing.

The last submitted patch, 4: 2343689-ief-nested-close-all-4.patch, failed testing.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch1.87 KB

Oops, re-uploading patch and setting to test against 8.x. Apologies.

bojanz’s picture

bojanz commented
Issue tags: +Needs tests

If there's a bug, then we first need tests to prove it.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Issue tags: -Needs tests
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch4.95 KB
new interdiff.txt5.31 KB

Sure, sorry it took so long for me to get to it. Will this suffice? I added a test case to ComplexSimpleWidgetTest. It tests the Cancel button for two open edit entity inline forms and for an open entity edit and open entity create inline forms. I also adjusted the method by which the parent form element is chosen for closing itself and children so that if you have a lot of nesting happening, only from the right level down are things closed up.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs review » Needs work

Found a case where this still isn't working:

* Create an entity using an inline entity form.
* Open the IEF Create form
* Open the IEF edit form
* Click Cancel on the edit form.
* Both forms close.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Assigned: Unassigned » m4olivei
m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch6.61 KB
new interdiff.txt4.39 KB

This updates the test to be more clear what we are testing and the different test cases we'd like to check. This introduces an additional assertion to test the issue found in my last comment, so altogether we now test:

* Two open IEF edit forms, Cancel one, other should remain open.
* One open IEF edit form, open IEF create form. Cancel create form, edit form should remain open.
* One open IEF edit form, open IEF create form. Cancel edit form, create form should remain open.

At this state of the patch, expecting the last case to fail as per my last comment. Fix to follow soon.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: 2343689-cancel_only_the_chosen_form.patch, failed testing.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Assigned: m4olivei » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch7.26 KB
new interdiff.txt5.02 KB

Wow, this is a tricky one. So I've changed my approach with this latest patch. I realized that InlineEntityFormComplex::closeForm is applicable to the create form and InlineEntityFormComplex::submitCloseRow is applicable to the edit forms. Also that, InlineEntityFormComplex::closeChildForms in it's current form is too broad, it will always close from the top level of the widget down, closing everything when used (currently used on Create -> Cancel and Edit -> Cancel). This latest patch setups the correct callbacks for each scenario (Create -> Create, Create -> Cancel, Edit -> Update, Edit -> Cancel). It also revises InlineEntityFormComplex::closeChildForms to properly act on the nearest inilne_entity_form parent downwards when closing child forms.

Should pass the testing now.

Status: Needs review » Needs work

The last submitted patch, 18: 2343689-cancel_only_the_chosen_form.patch, failed testing.

The last submitted patch, 18: 2343689-cancel_only_the_chosen_form.patch, failed testing.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs work » Needs review
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch8.15 KB
new interdiff.txt916 bytes

Here's one that fixes the PHP warnings.

Status: Needs review » Needs work

The last submitted patch, 21: 2343689-cancel_only_the_chosen_form.patch, failed testing.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented

Hum, seems there is a problem with adding existing now. Boo. Will test in the AM.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
StatusFileSize
new 2343689-cancel_only_the_chosen_form.patch8.62 KB
new interdiff.txt644 bytes

Yep found the regression. Hopefully this fixes it.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Status: Needs work » Needs review
pryrios’s picture

pryrios commented

This is still happening for me on 7.x-1-x. If I have two same-level Entity Forms opened and cancel one of them, they both close.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs review » Postponed

Trying to reproduce the issue but I can't. I have nested forms and all forms close or submit separately, is this issue still active in the latest dev. branch?

edwardchiapet’s picture

edwardchiapet
Location New York, NY
commented

I am also unable to reproduce this issue on 8.x-1.x-dev.

joachim’s picture

joachim commented
Status: Postponed » Closed (outdated)
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Title: Multiple forms submit/cancel closes all child forms » D7: Multiple forms submit/cancel closes all child forms