Support entity revision references

Amended the patch to apply this the same "protection" to non-revisioned IEF, hope that is ok. Interdiff attached.

I consider this (syncing revisions between parents and children) to be out of scope for the already too-big-almost-unmaintainable codebase.
Especially since the entityreference part hasn't even landed (and probably won't).

Instead of figuring out how to do this ourselves, could we just add widget support to Entity Reference Revisions? That way someone can use the built-in Entity Reference field or a Entity Reference Revisions if they want to save the revision as well.

The only thing that might be needed is a middle man (which would be another module) that updates the revision when the parent's revision is updated. That or we could add that as an option. Regardless, I imagine the code would be somewhat trivial.

Hi,
I came up with a solution to this with entity_reference_revisions. I was working on getting diffs working with IEF's so they could see deep changes within the referenced entities, so this is all detailed in this issue on the diff queue along with my forked change of IEF based on current 8.x head: https://www.drupal.org/node/2649138

I agree that integrating with entity_reference_revisions is the way to go in D8.

I've tested @vilepickle fork from github (https://github.com/vilepickle/inline_entity_form) against current 8.x-1.x and it effectively allows using IEF with entity_reference_revisions. Attached is the same fix that's in his repo in patch format (for testbot convenience).
Hope this makes it into this module!

Updating the patch from @vilepickle against current 8.x-1.x.

Previous patch failed when IEF was for a non-node entity. Updated patch attached will fix that by checking the interface implemented by the entity.

1. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -617,6 +618,7 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +          $is_revisionable = \Drupal::entityManager()->getStorage($entity->getEntityTypeId())->getEntityType()->isRevisionable();
   

   We know the entity type upfront. This could be a property on the widget itself.

2. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
   @@ -624,6 +626,19 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
   +              if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {
   

   $entity instanceof should work as well.

+++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
@@ -624,6 +626,19 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
+            if ($is_revisionable) {
+              $entity->setNewRevision();
+              // If a new revision is created, save the current user as revision author.
+              $entity->setRevisionCreationTime(REQUEST_TIME);
+
+              if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {
+                $entity->setRevisionAuthorId(\Drupal::currentUser()->id());
+              }
+              elseif (in_array('Drupal\entity\Revision\EntityRevisionLogInterface', class_implements($entity))) {
+                $entity->setRevisionUser(\Drupal::currentUser());
+              }
+            }

Maybe it should be alterable by modules?

+++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
@@ -624,11 +627,23 @@ class InlineEntityFormComplex extends InlineEntityFormBase implements ContainerF
+            if ($entity instanceof RevisionableInterface) {
+              $entity->setNewRevision();

Thinking more about it, we shouldn't set a new revision unless the current field type is entity_reference_revisions.

setRevisionUser() accepts a UserInterface not an AccountInterface.

Tested latest #26 patch and it works correctly (removes a fatal error that appeared previously regarding a wrong interface).

Great work, thanks everyone.

+            if ($entity instanceof RevisionableInterface) {
+              $entity->setNewRevision();
+
+              // If a new revision is created, save the current user as revision author.
+              if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {
+                $entity->setRevisionAuthorId(\Drupal::currentUser()->id());
+                $entity->setRevisionCreationTime(REQUEST_TIME);
+              }
+              elseif (in_array('Drupal\entity\Revision\EntityRevisionLogInterface', class_implements($entity))) {
+                $entity->setRevisionUserId(\Drupal::currentUser()->id());
+                $entity->setRevisionCreationTime(REQUEST_TIME);
+              }
+            }
             $entity->save();

It would make sense to (re)introduce a save() method on the inline form, and move the logic there (EntityInlineForm having the generic version, NodeInlineForm having its own). Also allows entity types that we didn't think of to do their thing.

Also, we should test this somehow.

This for now is just a reroll without addressing #28
I moved the logic to EntityInlineForm, which seems to be still sort of a good place.

.

Rerolled the patch as it did not apply clean.

+++ b/src/Form/EntityInlineForm.php
@@ -239,6 +240,26 @@ class EntityInlineForm implements InlineFormInterface {
+      if (!empty($item['needs_save'])) {

$item doesn't exists in this context.

Hi,

I have created a test for this.

Sorry, it had a bit to much changes. Attached the correct ones.

$item still doesn't exist. I'm wondering whether this interdiff, which I experimented yesterday gives us some form of starting point.

@harings_rob
Are you sure the test is correct? As long you don't use the entity_reference_revisions field here, it would still point to the same node, so its target ID would be the same and this is at least why its failing at the moment. I'll adapt the test coverage to use ER_revisions.

@dawehner:

Hmm, I might have missed something then. I tried it from the ui and could not find any difference. But let me take another look at the tests and adapt them.

Regards,

Step one: #2693975: NotNull validation duplicates the field level validation was required here.

Here is an updated patch, doesn't include any test changes yet. This is up for tomorrow ...

Here are tests finally ...

I fear we need #2695791: Add entity_reference_revisions as test dependency first

It won't work without #2693975: NotNull validation duplicates the field level validation I believe, but let's try.

Small iteration ...

Re-rolled #49.

Rerolls with invalid PHP syntax are brutal

Fixed some breaking typos in the test method, and removed config dependencies that prevented the reference field form display config from being installed.

This is working locally for me - I'm not sure why testbot is still having problems with it. Here's a version that should prevent the testbot fatal.

Well, that didn't help. Here's #55, but with files from config/optional moved to config/install.

Removed an unnecessary menu_ui dependency from the node type config.

Whoops, moved configs back to config/optional.

Passes! Here's the interdiff between @dawehner's last patch at #53 and #64.

There will need to be a follow-up to this patch to properly handle nested inline entity forms. IEF currently saves nested entities from the outside in, but should save entities from the inside out in order for new revision IDs to bubble up to the entities that are referencing them.

Opened a postponed issue at #2721349: Nested inline entities must be saved in "inside-out" order.

The attached patch results in the target_revision_id property being populated, and IEF being available on the entity_reference_revisions field. That's great.

Then there's this part:

+          if ($entity instanceof RevisionableInterface) {
+            $entity->setNewRevision();
+            if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {
+              $entity->setRevisionCreationTime(REQUEST_TIME);
+              // If a new revision is created, save the current user as revision author.
+              $entity->setRevisionAuthorId(\Drupal::currentUser()->id());
+            }
+            elseif (in_array('Drupal\entity\Revision\EntityRevisionLogInterface', class_implements($entity))) {
+              // If a new revision is created, save the current user as revision author.
+              $entity->setRevisionUserId(\Drupal::currentUser()->id());
+              $entity->setRevisionCreationTime(REQUEST_TIME);
+            }
+          }

This means that if the child entity is revisionable, we always create a new revision of it, even if we're not creating a new revision of the parent entity.
We need to fix that (posted the paragraphs logic in the next comment), and ensure it only triggers when the field used is of type entity_reference_revisions.

EDIT: Changed the comment several times as my understanding of the issue evolved.

Paragraphs logic:

    $new_revision = FALSE;
    if ($entity instanceof RevisionableInterface) {
      if ($entity->isNewRevision()) {
        $new_revision = TRUE;
      }
      // Most of the time we don't know yet if the host entity is going to be
      // saved as a new revision using RevisionableInterface::isNewRevision().
      // Most entity types (at least nodes) however use a boolean property named
      // "revision" to indicate whether a new revision should be saved. Use that
      // property.
      elseif ($entity->getEntityType()->hasKey('revision') && $form_state->getValue('revision')) {
        $new_revision = TRUE;
      }
    }

+1 for taking into account this logic!

Does not apply anymore :/. I tried to re-roll the patch but could not figure out what happened to InlineEntityFormComplex::massageFormValues. Any hints? Thanks!

It's not needed at all. The field item knows what to do when it gets the 'entity' key (take the appropriate id, and in case of entity_reference_revisions, revision id).

Ok, lets see.

I'll wrap this up.

+++ b/src/WidgetSubmit.php
@@ -46,6 +47,19 @@ class WidgetSubmit {
           $handler = InlineEntityForm::getInlineFormHandler($entity->getEntityTypeId());
+          if ($entity instanceof RevisionableInterface) {
+            $entity->setNewRevision();

This if statement is certainly borked. You need $entity->getEntityType()->isRevisionable() instead.

bojanz commented about 5 hours ago
Something is going wrong here ...

One thing I'm also wondering is whether we should just check whether the parent entity got a new revision and just in that case create a new revision of the inline entity itself.

Just providing a patch for #74

Looks good from a read through, just found this:

+++ b/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php
@@ -7,6 +7,7 @@ use Drupal\Component\Utility\SortArray;
+use Drupal\Core\Entity\RevisionableInterface;

This isn't actually used anywhere in this class.

Also setting back to Needs Review to see if the tests will pass.

Found a couple other things with the patch.

+++ b/src/WidgetSubmit.php
@@ -46,6 +47,19 @@ class WidgetSubmit {
+            if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {

Could we keep this consistent with the outer conditional and do $entity instanceof NodeInterface?

+++ b/src/WidgetSubmit.php
@@ -46,6 +47,19 @@ class WidgetSubmit {
+            elseif (in_array('Drupal\entity\Revision\EntityRevisionLogInterface', class_implements($entity))) {

Same here. Also I don't see an interface named Drupal\entity\Revision\EntityRevisionLogInterface anywhere in Drupal core. Should we use $entity instanceof \Drupal\Core\Entity\RevisionLogInterface instead? That seems to have the methods we want to call here.

Here is a patch for the changes I suggested.

This patch adds a test for using Complex widget with entity_reference_revisions field types. I took out the content types that were added and added three new ones that were easier to keep straight in my head. I've got setup 3 content types, Level 1, Level 2, Level 3. Level 1 has a entity reference revisions field that points to Level 2's. Level 2 has an entity reference revisions field that points to Level 3's. Level 3 is just the title. The test creates a Level 1 with a nested Level 2 and Level 3 using the complex widget. It then edits that node and changes the referenced nodes titles using the complex IEF widget.

As illustrated, the 3rd level node doesn't get properly referenced on edit. But the first level works. This is what's meant by #2721349: Nested inline entities must be saved in "inside-out" order, which @Les Lim brought up in #66.

Wondering if this ticket should go in without getting that fixed.. Or at least before support for entity_reference_revisions makes a cut release, this issue and #2721349: Nested inline entities must be saved in "inside-out" order should make the cut.

+++ b/src/WidgetSubmit.php
@@ -49,12 +51,12 @@ class WidgetSubmit {
-            if (in_array('Drupal\node\NodeInterface', class_implements($entity))) {
+            if ($entity instanceof NodeInterface) {
...
-            elseif (in_array('Drupal\entity\Revision\EntityRevisionLogInterface', class_implements($entity))) {
+            elseif ($entity instanceof RevisionLogInterface) {

Isn't that adding a dependency on node/entity module ? Well, this actually works, see https://3v4l.org/JsUkC

Neat, I hadn't thought of that, but if it works, great, a little cleaner.

@m4olivei

Wondering if this ticket should go in without getting that fixed.. Or at least before support for entity_reference_revisions makes a cut release, this issue and #2721349: Nested inline entities must be saved in "inside-out" order should make the cut.

Well, I'm wondering whether its related with revisions or not :) If it is an already existing issue, we better open up a follow up, given how many people kind of need this particular patch at this point in time.

• Applied the logic from the paragraphs module
• Add method on the inline form handler to deal with that instead of putting everything onto the WidgetSubmit class.

This is starting to look good.

Here's a review:
- The last patch lost the ComplexWidgetRevisionsWebTest. We need to add it back, comment out the failing third level, point to #2721349: Nested inline entities must be saved in "inside-out" order.

+  /**
+   * Determines wheter we should save a new revision.
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity.
+   * @param \Drupal\Core\Form\FormStateInterface $form_state
+   *   The form state.
+   *
+   * @return bool
+   */
+  protected function hasNewRevision(EntityInterface $entity, FormStateInterface $form_state) {

Sounds like we should call it shouldSaveNewRevision() then? Also, typo in the docblock ("whether").

+    if ($entity instanceof RevisionableInterface && $entity->getEntityType()->isRevisionable()) {
+      if ($entity->isNewRevision()) {
+        $new_revision = TRUE;
+      }
+
+      // Most of the time we don't know yet if the host entity is going to be
+      // saved as a new revision using RevisionableInterface::isNewRevision().
+      // Most entity types (at least nodes) however use a boolean property named
+      // "revision" to indicate whether a new revision should be saved. Use that
+      // property.
+      elseif ($entity->getEntityType()->hasKey('revision') && $form_state->getValue('revision')) {
+        $new_revision = TRUE;
+      }

$entity->getEntityType()->hasKey('revision') is the internal logic of $entity->getEntityType()->isRevisionable() so we don't need to repeat it.

+    if ($this->hasNewRevision($entity, $form_state) && $entity instanceof RevisionableInterface) {

No need for the second check, it's already done by the called method.

+      if ($entity instanceof NodeInterface) {

We're in the Node handler now, this condition will always be true.

+  /**
+   * Submits the widget.
+   *
+   * This method is executed before ::save().
+   *
+   * @param \Drupal\Core\Entity\EntityInterface $entity
+   *   The entity.
+   * @param array $form
+   *   The entity form.
+   * @param \Drupal\Core\Form\FormStateInterface $form_state
+   *   The form state of the parent form.
+   */
+  public function widgetSubmit($entity, $form, $form_state);

My initial thought was that we should rename this method to preSave().
But thinking about it more, why don't we just pass $form and $form_state to save() and move the logic there?

To which IEF dev version can I apply this patch? I tried it couple of times with different versions via composer (adding to extra / patches to composer.json file), but it's keep saying "can't apply patches, skipping" which means I don't have that exact version to which I could apply!

Just attaching a patch based on @bojanz feedback. I think I addressed all of his points but let me know if missed anything. ^_^

Also attaching an interdiff. Hopefully the tests pass ^_^

Updated patch with fixes to $inline_form_handler->save().

sorry, didn't mean to post to this issue

revert

using entity reference revisions, inline entity form and patched eck to support revisions with err. I get duplicate revisions saved by $this->save() in

-  public function save(EntityInterface $entity) {
+  public function save(EntityInterface $entity, FormStateInterface $form_state) {
+    if ($this->shouldSaveNewRevision($entity, $form_state)) {
+      $entity->setNewRevision();
+      if ($entity instanceof RevisionLogInterface) {
+        // If a new revision is created, save the current user as revision author.
+        $entity->setRevisionUserId(\Drupal::currentUser()->id());
+        $entity->setRevisionCreationTime(REQUEST_TIME);
+      }
+    }
     <strong>$entity->save();</strong>
   }

ERR does checking if revision should be saved:
if (!$host->isNew() && $host->isNewRevision() && $this->entity && $this->entity->getEntityType()->get('entity_revision_parent_id_field'))

for a temporary quick fix, I commented out IEF save method but maybe there is a proper way to fix it?

@audriusb may be you should add something like


/**
 * Implements hook_entity_type_build().
 */
function my_custom_module_entity_type_build(array &$entity_types) {
  /** @var \Drupal\Core\Entity\EntityTypeInterface[] $entity_types */
  if (isset($entity_types['my_eck_entity_type_id']) && !$entity_types['my_eck_entity_type_id']->getHandlerClass('inline_form')) {
    $entity_types['my_eck_entity_type_id']->setHandlerClass('inline_form', '\Drupal\my_custom_module\Form\MyEckInlineForm');
  }
}


for your entity then extends Drupal\inline_entity_form\Form\EntityInlineForm see Drupal\inline_entity_form\Form for implementation.

Otherwise for me #92 patch did the trick for a custom entity type using 8.x-1.0-beta1, entity_reference_revisions 8.x-1.2 and drupal core 8.2.7.

The patch in #92 works for me on a Entity Reference Revisions field in a Paragraph bundle referencing a Custom Block.

Drupal 8.3.2
Paragraphs 8.x-1.x-dev
Entity Reference Revisions 8.x-1.2
Inline Entity Form 8.x-1.x-dev

Tested patch 92 working with Drupal 8.2.8, paragraphs 1.1.0, and entity_reference_revisions 1.2.0 .

Changing status to RTBC. Raising priority because of the ubiquity of paragraphs module. Anyone who tries to use inline forms with an entity that has a paragraphs field is losing data until this patch makes it in.

+++ b/src/InlineFormInterface.php
@@ -137,18 +141,4 @@ interface InlineFormInterface extends EntityHandlerInterface {
-  /**
-   * Submits the widget.
-   *
-   * This method is executed before ::save().
-   *
-   * @param \Drupal\Core\Entity\EntityInterface $entity
-   *   The entity.
-   * @param array $form
-   *   The entity form.
-   * @param \Drupal\Core\Form\FormStateInterface $form_state
-   *   The form state of the parent form.
-   */
-  public function widgetSubmit($entity, $form, $form_state);

+++ b/src/WidgetSubmit.php
@@ -49,9 +49,7 @@ class WidgetSubmit {
-          $handler->widgetSubmit($entity, $form, $form_state);

Removing these methods could break existing subclasses, as they maybe expect the call. Should this maybe be cleaned up later to minimize the possible risks?

Per #99

I just tested #92 and I'm seeing behavior that I think might be unexpected.

I have a content type "Provider", which has an Entity Reference / Entity Reference Revisions field (I tried both) with the IEF complex widget that references nodes of content type "Strategy". The Strategy content type has the "Create new revision" box checked.

I navigate to the node edit page for a Provider node and update a Strategy that the ER/ERR field already references, then I save the IEF.

I expect that this will result in a new revision being created of the Strategy node, but no new revision is created.

Any ideas?

any news about this? It's a very important requirement.

would be nice if anybody can give a feedback

This will definitely need tests.

I"m using patch #92 and is working well for me.

I'm not using paragraphs, just custom revisionable entities in IEF.

I am using:

Drupal 8.3.7
entity_reference_revisions 8.x-1.3
inline_entity_form: 8.x-1.0-beta1

hi
please what will be the solution for Drupal 7

@delacosta456: for Drupal 7 just use the Paragraphs module, it includes the underlying revision-safe entity reference field.

hi thanks @DamienMcKenna

May be ... but the real situation i am trying to achieve is

-- with Workbench + Workbench Moderation

---a node type A referenced (and embed with inline Entity Form) a node type B
--- A is under Moderation and revision is created anytime published version is created.
--- B should ONLY be edited through Awhich means
****** to modify any field of form B embed in A with IEF , A must be opened and so after saving until new revision of A is not published B also shoudn't be updated

@delacosta456: Your use case strongly suggests that you should be using Paragraphs for the B object instead of a content type.

That said, this is off-topic for this issue.

Paragraphs aren't reusable. So using paragraphs instead of another entity type is not a workable solution in cases where inline entities need to be reusable.

@oknate: That is true, though the new Library functionality in the D8 version can expand the possibilities. Also, in delacosta456's use case, it isn't clear what the intended use cases are for content type B, which leaves lots of possibilities.

Questions about the UX:

• Should it automatically create a revision or just provide the option to create a new revision (i.e. not hide those entity options)?
• If it is going to automatically create a new revision, shouldn't this only happen for entities which were actually modified?
• Should all of this be optional in the widget settings?

I'm working on this and have rerolled the patch. Instead of changing the signature of the presave method (which violates BC) this one sets the newRevision() on form build. I did some manual tests with this patch and it worksforme like a charm.

#113: Yes a new revision is created when the bundle is configured to, and only if there is a change. This is a reasonable default and if we want options for this, we can do that in a followup.

NR for the bot but did not fix tests.

By testing manually the patch #114, I found that revisions are not being created on change, but it changes the 1st revision of the referenced entity. And if you revert the host entity, the referenced entity does not revert. However the patch #92 works fine from this regard.

@joevagyok #115:
Thanks for testing this and reporting back.

> but it changes the 1st revision of the referenced entity

I do not understand this. Do you mean the current revision?

Also, please go to admin/structure/types/manage/[referenced-node-type] (for both node types) and look into the Publishing Options at the bottom. Is Create new revision checked?

Yes, I meant the current revision.
After taking a look at the patch #114 I found that your IF condition is the issue.
In my case I have an entity type that does not have bundles and it is not a bundle-able entity.
Do you have any specific reason in order to restrict newRevision() to getBundleEntityType() only?
In my opinion this should work for any revisionable content entity type regardless of bundle capability.

Thanks for the feedback. Alas, can you refer me to a filename and line? Or even better, get dreditor and extract the patch lines you refer to?

+++ b/src/Element/InlineEntityForm.php
@@ -132,6 +134,23 @@ class InlineEntityForm extends RenderElement {
+      && ($bundle_entity_type = $entity_form['#entity']->getEntityType()->getBundleEntityType())

This is the if statement I am referring to. This line here prevents my non bundle-able entity to be saved in a new revision, even though it is configured to do so.

As you see in the code, i went along the lines of ContentEntityForm::addRevisionableFormFields to see if we should create a new revision.

> to be saved in a new revision, even though it is configured to do so.

If no bundles, can you elaborate how we should know if a new revision should be created? i don't know a generic API for that.

(Maybe we should instead add a "new revision" setting to IEF?)

+    // Set revision property if the bundle is configured to.
+    // @see \Drupal\Core\Entity\ContentEntityForm::addRevisionableFormFields
+    if (
+      $entity_type->isRevisionable()
+      && ($bundle_entity_type = $entity_form['#entity']->getEntityType()->getBundleEntityType())
+      && ($bundle_entity = \Drupal::entityTypeManager()->getStorage($bundle_entity_type)->load($entity_form['#entity']->bundle()))
+      && $bundle_entity instanceof RevisionableEntityBundleInterface
+    ) {
+      $new_revision = $bundle_entity->shouldCreateNewRevision();
+      $entity_form['#entity']->setNewRevision($new_revision);
+      // @see \Drupal\Core\Entity\ContentEntityForm::buildEntity
+      if ($new_revision && $entity_form['#entity'] instanceof RevisionLogInterface) {
+        $entity_form['#entity']->setRevisionUserId(\Drupal::currentUser()->id());
+        $entity_form['#entity']->setRevisionCreationTime(\Drupal::time()->getRequestTime());
+      }
+    }
+

Right, I understand your intentions and your question is correct. You can't determine what should happen with revisions when the entity does not have the bundle configuration.
As first look, I believe in such case it would make sense to follow the host entity behavior, so if the referenced entity is revisionable but not bundleable, then do what the host entity will do with the new revision. Otherwise we look into the bundle config as you did in your patch.

It would be nice indeed to implement such config for IEF that could control this behavior on the form widget as a setting.

I will try to reroll the patch with these aspects in the following days.

I came to make a setting of it (untested) - can u review and test?

I took the #122 patch and I wrote tests and I have took a look at the series of failures in other tests.

I found couple of issues regarding the entity change check with DiffArray, that returns false always since in that submit the values are already in updated state so even after buildEntity() it is the same. Changes were never been detected and doing this way it is not reliable enough. I would postpone this until we get the proper EntityChangedAPI in from core. Currently if you save a node without changing the fields it will create a new revision. So practically we follow standard drupal behavior. I see no problem with having new revision once the IEF are opened and the user hits Save.

I also found an issue with not using the ['#save_entity']. Basically, the save does not work without that when revisioning is involved. By making sure that this variable is set when we are creating new revision everything is back to normal. On the other hand it is true that it does not seem to be used, but I did not intend to change this completely from what the creators implemented as it would be out of scope.

To wrap it up, this patch brings the widget configuration for "Create new revision". I have extended the tests from the patch and I have added tests for "Entities without bundle" (entity_test module) case just in case.

Fixed some php code sniffer issues that I have introduced in my previous patch #124.

I accidentally included the patch in the patch in #125, sorry.
It should be correct now.

Kudos for the extensive tests!

Alas, this looks like some #122 changes got accidentally reverted.

InlineEntityForm:

+    // Handle revisioning if the entity supports it.
+    if ($entity_type->isRevisionable()) {
+      if ($entity_form['#revision']) {
+        $entity_form['#entity']->setNewRevision($entity_form['#revision']);
+        // If we make a new revision make sure that it gets saved.
+        $entity_form['#save_entity'] = TRUE;
+
+        // @see \Drupal\Core\Entity\ContentEntityForm::buildEntity
+        if ($entity_form['#entity'] instanceof RevisionLogInterface) {
+          $entity_form['#entity']->setRevisionUserId(\Drupal::currentUser()->id());
+          $entity_form['#entity']->setRevisionCreationTime(\Drupal::time()->getRequestTime());
+        }
+      }
+    }
+

@geek.merlin aka axel.rutz Please read my #124 comment where I explain my intentions on why that and others were changed. But if you take a closer look, this is not far at all what was in #122 just added some additional logic for improvement on deciding over revisioning.
Please, test and review the patch before marking back to Needs work.

Ah, OK thanks. TBH i did not read that thoroughly enough :-/.
(I know how tedious this is but interdiffs would help. And let's look forward to the d.o gitlab thing that will simplify that...)

Now this totally makes sense to me code-wise. I did not yet test this manually, but given that i use #114 in production, the logic itself did not change since then (only the revisionable configuration), and the extensive tests you made are green, i' give this RTBC.

Really appreciate the work you put into this!

I have created a new patch with an interdiff based on the #126 patch. I have simplified the code. I also removed an unnecessary line. Recreated this patch in #131.

In #130, my final patch was not correct so I had to redo it. I have created a new patch with an interdiff based on the #126 patch. I have simplified the code. I also removed an unnecessary line of code. Thank you for the review you made @geek.merlin

So ['#save_entity'] being needed was a misunderstanding?
Apart from that, the interdiff is quite simple and looks good to me.

@geek.merlin, right. After revisiting the code I realised that using it would save the entity right away in the complex widget when we click "Create node" and close the inline entity form by the ajax operation. This might be an unwanted change that I did not intend to introduce.

After using this patch actively, we have encountered the fact that the "Revision log message" field is automatically added to the form since we are referencing revisions and we can create new revisions with IEF as well. Since this patch introduces the support for revisions I think it would well fit the scope to provide an additional widget setting to "Hide revision log message" input field from the form. I will proceed to provide this additional functionality to the patch.

> that the "Revision log message" field is automatically added to the form

See related.

Great! Thanks Axel :)

Now that #2974544: Convert tests from Simpletest to FunctionalJavascript is in, re-triggering test.

Also unassigning @bojanz.

OK, now that tests are modernized, #131 does not apply anymore and the tests need to be updated too.

@joevagyok can you look into this so we can bring this in soon? (I'm now comaintainer.)

I have ported the revision web tests to Functional JavaScript tests.

Try removing interdiff.txt from displaying on top, Dreditor went mad.

I did testing of patch. In general looks good except failed test: \Drupal\Tests\inline_entity_form\FunctionalJavascript\ComplexWidgetTest::testReferencingExistingEntities
Looks we should have here 12 referencies in 'All bundles' field.

Thanks @Chewie. Yes I see that the test there assigns node from all existing bundles and since I added 3 new bundles to the test module it sees 12 and not 9 results anymore. I will fix the assertion and re-roll the patch.

I fixed the error in the ComplexWidgetTest::testReferencingExistingEntities test that originated from the fact that in the test module the patch adds 3 new content bundles and this particular test uses all existing bundles and the total number now is 12 instead of 9.

I did a testing of last #143 patch. Patch looks good for me. Thanks, @joevagyok! I think it makes sense to put RTBC.
Great work was done!

🐳Wooooooooot!🐳 This is gorgeous. Thanks everyone who worked on this!