Required IEF forms for existing entities don't work

Let's trigger the test.

Assigning for review.

Thanks @idimopoulos,

Works like a charm!

Found a couple of nits:

1. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -566,12 +568,69 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +      else if ($widget_state['form'] == 'ief_add_existing') {
   

   According to the coding standards, elseif should be written as one word. It is also a good practice to use === instead of == when doing string comparisons.

2. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -566,12 +568,69 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +          $options = array(
   

   Let's use the shorthand array syntax [].

   This can be applied throughout the whole patch.

3. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -566,12 +568,69 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +          /** @var /Drupal\Core\Entity\EntityReferenceSelection\SelectionInterface $handler */
   

   The first forward slash should be a backslash.

4. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -566,12 +568,69 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +          $handler = \Drupal::service('plugin.manager.entity_reference_selection')
   

   Let's inject this service rather than sideloading it.

Adjusted code. Replaced loading the service statically with dependency injection. Please review it.

#11 is doing the job.
Many thanks alunyov!!

+++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
@@ -566,12 +580,68 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
+              elseif (count($entities) > 5) {
...
+              elseif (count($entities) > 1) {

As per #10.1 and Drupal coding standards, these need to be two words: else if () {.

Otherwise the patch works well, and should be RTBC when that last nit is fixed. Please remember to include an interdiff.

@Darvanen,
I'm sorry, but I didn't get your point:

As per #10.1 and Drupal coding standards, these need to be two words: else if () {

According to codding standards it should be elseif (like it is in code now). Even if you check link you are referencing Control Structures it says:

(Note: Don't use "else if" -- always use elseif.)

Getting the ticket back to RTBC.

Wow, I totally had that backwards, sorry.

It wasn't RTBC when I commented but I agree it is now.

I think the patch needs to be rerolled:

patching file README
patching file src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
Hunk #2 FAILED at 40.
Hunk #5 succeeded at 605 (offset 32 lines).
1 out of 5 hunks FAILED -- saving rejects to file src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php.rej
patching file src/Tests/ComplexWidgetWebTest.php
Hunk #1 succeeded at 430 (offset -2 lines).

Queued for re-testing against 8.6

Just needed a minor modification to get it working. Please review!

Thanks @kathay, can we have an interdiff next time please?

Looks like it's failing, but at least it's applying!

Looks like a test function was replaced between #11 and #20. Updated the patch with interdiff against #1.

Sorry @kaythay the interdiff we need is against #11 - the most recently reviewed patch. I'm struggling to find the difference between that and your patches except for the line numbers.

Sorry about that. #11 was actually not a full patch and only applied against #2 so I had only interdiffed against #2. Here is the interdiff against #11.

I think perhaps that hunk didn't apply when you went to create the interdiff because both patches contain that section.

What I'm taking from this is that your reroll of the patch didn't change any code, just the line numbers in the patches (which was necessary, obviously because #11 wasn't applying properly.

Is that correct? If you didn't change any code, this is RTBC.

The only change besides line number was changing $this->setAllowExisting(TRUE); to $this->updateSetting('allow_existing', TRUE); in #23 for tests to pass.

Great! Then I call #23 RTBC based on previous reviews.

Hello,
The #23 patch applied correctly and working as expected.
+1 for RTBC.
Tested on dev branch.

+1 to RTBC

Thanks, everyone.

Hello,

Just updated from rc1 to rc2 and our inline_entity_form for existing entities started not working. Reverting back to rc1 got everything working again.

In rc2 when trying to select an existing image I would get the following error…

Notice: Undefined index: entity_id in Drupal\inline_entity_form\Plugin\Field\FieldWidget\InlineEntityFormComplex->extractFormValues() (line 621 of /modules/contrib/inline_entity_form/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php).

Please let me know if you need more information.

Thanks,

•• matt