Simplify EntityDisplayEditFormBase ajax rebuild flow to work only with $this->entity

Unfinished sentence in the IS

As a preliminary, there's some code we can simplify now that EVDEditForm and EFDEditForm are entity forms : the Display entity gets rebuilt with ajax submitted values, so no need to store and merge them separately in $form_state :-)

More manual merge removal

The above doesn't work with the "cancel" button on formatter settings subform, updated values get saved even if you hit cancel (looks like we're missing some test coverage there)

I tried to fix this and clarify when exaclty we merge incoming settings form values, but this now seems to never actually update the settings in the Display entity, even with the"update" button. I must be missing something in the flow of ajax submit & buildEntity().

Posting what I have for now, but it's broken :-)

Yeah, actually in order to do what the issue title says, we first need EntityViewDisplayEditForm::$entity to reflect the current state of the form - meaning, that all the code in the form actually uses the (awesome) promise from #2448357: Config entity forms need to have access to an updated entity object at all times : "we get to work with an entity object at all times instead of checking submitted values from the form state".

Sorry, I should have made that clearer, but I also thought I'd be faster in making those preliminary cleanups :-)

So I'm having a hard time figuring out what exactly our ajax submit should be doing (storing something in form state for the next rebuild, or updating the entity direclty). I guess that is a larger question of "how do we work in Ajax with entity forms", coz I'm not sure at this point :-)

My goal / assumption is : multistep EntityForm::form() implementations should ideally just do their stuff based on the current $this->entity, without fetching anything else from $form_state or whatnot.

I did a step-by-step debug on an ajax submit of the "Update" button in a formatter settings subform, and checked where our Form methods are called :

- FormAjaxController::content() fetches the form from cache, and calls formBuilder->processForm() to process the submit
- formBuilder->processForm() first calls formBuilder->doBuildForm() to finish preparing the form taken from cache, and this runs #after_build
--> 1: our copyFormValuesToEntity() is called, but we don't want to update the entity with submitted values at this point, since our submit handler hasn't run, and it's the one who knows what should happen depending on which button was submitted (update, cancel...)
- formBuilder->processForm() then moves on to processing the actual submit
--> 2: our multistepSubmit() button submit handler runs, it can prepare "stuff" for the next rebuild according to what button was submitted
- formBuilder->processForm() then rebuilds the form
--> 3: our form() / buildFieldRow() methods are run, and should ideally just do their stuff based on the current $this->entity EVD
- #after_build runs again at the end of the rebuild
--> 4: our copyFormValuesToEntity() is called, but it's too late to update $this->entity with submitted values now, the new form content has already been generated

So it looks like it's the button submit handler in 2: that has the only opportunity to update the Form::$entity with incoming values so that it's ready for the next rebuild in 3: ?

But it turns out that just modifying $this->entity in the button submit handler doesn't persist up to the next rebuild (you can do $this->entity->foo = 'bar' in there, the form() method on the next rebuild gets a $this->entity that has no ->foo).

That's bacause (surprisingly ?) within the submit handler, $this !== $form_state->getFormObject() - they are two distinct instances of the same class (EntityViewDisplayEditForm), and its the latter that gets persisted for the next rebuild, not $this.

So in order to modify the entity for the next rebuild, the button submit handler needs to do something

$this->entity->foo = 'bar':
$form_state->getFormObject()->setEntity($this->entity)
$form_state->setRebuild();

Which... feels wrong/weird ? Or is that the right way ?

If the end of #7 is the correct way, then the attached patch seems to work for me.

- A new updateFieldComponentSettings() helper, called by submit handlers explicitly when applicable, is in charge of updating formatter settings in $this->entity
- copyFormValuesToEntity() is only in charge of the rest (weights, formatter plugin id), which is not part of the multistep flow.

(still haven't got to actually doing what the issue title says, but it should be fairly easy at this point)

Minor - reverted a couple useless hunks, and adjusted some comments.

And then actually doing what we originally came here for.

Updated the title and IS with the actual content of the patch :-)

#9 looks like a testbot hiccup, #8 and #10 are green :-)

OK, very weird, I was about to open a separate issue for "$this !== $form_state->getFormObject()" thing, but I can't seem to reproduce it anymore, and $this->entity->foo = 'bar' in a button submit does now persist to the next rebuild. Dunno if I was on crack at the end of #7, or if I was the one breaking it by doing weird stuff :-/

So, no need for $form_state->getFormObject()->setEntity($this->entity), and then here's an even simpler version, removing 10 more lines : keeps all the "form values --> entity" logic back into copyFormValuesToEntity(), the button submit just places a flag notifying whether values from the settings subform should be merged.

OK, got it : "$this !== $form_state->getFormObject()" happens when the submit callback is specified with
'#submit' => array(array($this, 'callback'))
rather than
'#submit' => array('::callback')

That's actually described in the IS of #2309323: Allow #submit and #validate to be specified as methods of the form object that introduced that '::callback' syntax a year ago :

super-weird behavior because *the object the method will be called on is not the same as $form_state['build_info']['callback_object']* during form submission. This is especially bad for entity forms, which maintain $this->entity as the primary reference for the entity being built

@Berdir be blessed :-)
(Looks like I even commented on that issue back then - although my comment shows that I didn't really get the point :-p)

I only incidentally added that syntax at some point in the patch thinking "oh, cool", but it's actually fairly important. Thus, using it in another place where multistepSubmit() is used, and using it for multistepAjax() as well, which suffers from the same.

Also, reverting one line that doesn't actually need to be moved.

Testbot, you're drunk, that failing patch was 10 posts ago, the last one is green :-)