In submit use the entity built during the validation instead of building it again - for performance and consistency ensuring we are saving the entity that has been validated

I've implemented the proposed solution from the issue summary.

@vprocessor sorry ;)

So the problem was that the user registration form was relying on building the entity twice and has done stuff with the form values which has to be done as part of ::buildEntity instead of ::submitForm.

@vprocessor when you are posting a patch could you please provide interdiffs and a little explanation what you are doing? Is there anything you do not agree with regarding the last patch I've posted?

Re-uploading the patch from #11 as the currently working active patch to be reviewed.

Oups.. there it is...

end of that sentence doesn't make sense, a not missing?

Yes :). Will update in the next patch version :).

Any change that requires changing something else means that it is an API change and breaks BC, unless it is in areas where we say that's ok, which is I think not true for this.

Yes I know that this is a change, but it is there for two reasons - one is performance and the second is to ensure that exactly the entity we have validated is saved and not any other one. Take a look at the paragraphs module which actually is a famous one and there is an inconsistency that when the langcode of the language widget is altered and the form is submitted the entities built during the validation phase will have the language code they had during initially building the form but then in the submit the generated entities will differ than the ones generated during the validation and will have the new language code. This leads into saving entities which are not validated and under circumstances might even lead to some constraints not validating what is being saved. Consider e.g. a validation constraint on the paragraph entity that the language "test_language" is not a valid translation, but currently they would pass the validation with the previous langcode and be saved with the new one...

That is why I think that this API change is an important and needed one as everybody makes mistakes and this would prevent such ones - I guess we all want that when we start using a contrib or also a custom module in our system that this module is not saving not-validated entities.

Addressing #17.1 and moving all knowledge of the intermediate entity to the content entity form only.

Just a small improvement for a shorter statement.

The part where I had to adjust the RegisterForm is actually an issue on itself - #2834030: Currently we are saving an user entity with some not validated fields on the registration form as because of the current implementation not all user entity fields are correctly validated. #2834030: Currently we are saving an user entity with some not validated fields on the registration form just proves that it is so easy to oversee such things and how important this issue is in order to prevent saving invalid entities - in core, custom and contrib.

Re-roll + instead of unsetting the intermediateEntity property in __sleep it is better to set it to NULL as in some issues we've seen problems by doing so on PHP 5.6.

You are right .. I've addressed this and also found a little mistake in buildEntity having logic still running twice, so I've covered that too.

Would it be better/simpler if instead of having these two new properties on the content entity form just to put the validated entity as a temporary value in the form state and then retrieve it in the submit?
In this case we don't have to worry about the serialization of the form, form object and the form state as temporary values are being thrown away and we don't serialize between validation and submit.

What I mean is:
$form_state->setTemporaryValue('validated_entity', $entity);

Adding getter and setter to ContentEntityFormInterface for the intermediate entity.

1. +++ b/core/modules/layout_builder/src/Plugin/Field/FieldWidget/LayoutBuilderWidget.php
   @@ -39,12 +39,6 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   -    // @todo This isn't resilient to being set twice, during validation and
   -    //   save https://www.drupal.org/project/drupal/issues/2833682.
   -    if (!$form_state->isValidationComplete()) {
   -      return;
   -    }
   

   Fixed layout builder by removing condition which was meant to be removed with this patch.

   This is just amazing. The current approach has been just validated as needed for core!

2. +++ b/core/lib/Drupal/Core/Entity/ContentEntityForm.php
   @@ -146,7 +166,13 @@ public function form(array $form, FormStateInterface $form_state) {
   +    // Let ::buildEntity return the entity generated in ::validateForm instead
   

   When referring to methods in a comment we should append braces.

   Also #33 still has to be addressed.