Image upload broken (> alpha5)

I am looking at the same issue, i also found this one that seems closely related. https://www.drupal.org/node/1329856.

I am not able to pinpoint the source of the problem yet, but it seems that the field collection issue implies to have fixed the problem / found a fix.

At the moment i am testing the same setup with field collection 8.x-1.x-dev (8.x-2.x-dev does not really do anything as it seems) and inline entity form 8.x-1.0-alpha5 (gonna try dev now) to see if it persists.

Maybe this helps, i will at least report my findings.

Checked out the dev version of Inline Entity Form and reverted this commit http://cgit.drupalcode.org/inline_entity_form/commit/?h=8.x-1.x&id=30589... but no cigar.

Also tried uninstalling field_collection all together, but still nothing.

Just to be sure field_collection does not break anything in this particular case. I reinstalled alpha-5 and re-enabled field_collection and everything started working again.

Sounds like the same problem from #2690093: The simple widget can't be embedded inside a complex one. We're making progress on test coverage and a fix there.

I won't. We'll want to add test coverage here at least (our tests should have an image field, to prevent future regressions)

We need a test here before we can proceed. Unfortunately, I won't have IEF time this week.

I'm looking into it.

Uploading the breaking test for now.

  public static function validateEntityForm(&$entity_form, FormStateInterface $form_state) {
    $inline_form_handler = static::getInlineFormHandler($entity_form['#entity_type']);
    $inline_form_handler->entityFormValidate($entity_form, $form_state);
    $form_state->setValueForElement($entity_form, $entity_form['#entity']);
  }

After a lot of debugging we realized that the last line can't work. We need to remove it, then change extractFormValues() to use the $element['#entity'] property instead of the submitted values acquired via $form_state->getValue().

The setValueForElement() method doesn't need to be removed completely as bojanz mentioned (btw, thanks for pointing me to the right direction :) ), it just needs a condition, to check whether if it was a file upload or not, and set the values for the element only if it wasn't a file upload.

I've tested it with nodes, and with a custom bundle-less entity (applied the patch from https://www.drupal.org/node/2569193 for it) and it worked for both files and images.

I've attached a patch for it.

Whops, the condition was not quite right, fixed it.

I fixed a notice introduced in #17, but tests still fail with the patch from #17: https://travis-ci.org/bojanz/inline_entity_form/jobs/120569592

Problem is that even if we skip that line on file upload, the widget extractFormValues() methods run. And I don't think we can skip them completely.
So the best option is to keep exploring #15.

dawehner's test patch breaks the build completely even with the fix.

For some reason, testEntityCreation() starts failing as soon as the inline form has an image field.
The Some reference/John/Doe form doesn't get saved, we instead get an empty form back, but that form belongs to the all_bundles IEF element, not the multi IEF element (they look the same but have different form names).
Meanwhile, enabling the test module and using the browser works fine.
I think this is unrelated to the current regression, but it certainly confuses things.

Attaching the current fix. It fails the existing nested tests.

I'm going to try cleaning up our nested hacks in another issue, with hope that it will help here. triggeredByCurrent() and the "if ($values['form'] == 'add') {" part of extractFormValues() should not need to exist.

Updated patch, skipped a commit.

Committed the fix that passed tests.
Opened a followup for tests #2701397: Add tests for image fields, since I couldn't get dawehner's to pass (even though manual testing works).

Also cleaned up the mentioned triggeredByCurrent: #2700571: Remove EntityInlineForm::triggeredByCurrent.

EDIT: Manual testing show sthat an image field inside an IEF form element (not a widget) still fails, so it's possible we have additional bugs here.

Closed #2719035: Image upload on core 8.1 and entity_browser-dev fails as duplicate.