Node update hook is triggered upon group content create

#45 works for me.

Re @minorOffense: Are your able to share more details about your setup? At the very least a backtrace of the error would be helpful as well as any contrib (or custom) modules that might be doing something on node save in order to be track down if the patch is causing an issue there.

Note that 🐛 Content Moderation fatals when a moderated entity is re-saved on hook_insert() Fixed was committed to core so anyone getting duplicate entry issues with this patch, try the latest 9.5 or 10.0 dev version to see if that fixes the issue.

I presume that was also your issue @minorOffense. Sorry, I had forgotten about the core bug report when I wrote the previous comment.

Hi!

Thanks for the original patch!
I has facing a similar issue, as i have in a custom module a hook_node_update that was being called couple of times, just to notice that group was triggering the function during some relationship updates. Unfortunately, the original patch only works with v1.x and now v2.x (and 3.x) are released, making the patch incompatible.

Here is (what i think it should be fully compatible) the patch for the v2.x of the module (sorry, i have not tested the test).

I am not sure if i should change the version of the metadata to v2.x or to any other version, so I leave in 1.x for now

Hi!
Fixing a small error in the test, that was causing a fatal error.

Sorry, not familiar with tests (I hope now is fixed once for all)

The approach taken here is not sufficient for a few reasons. It seems to work around the code found in workspaces' EntityOperations class:

    /** @var \Drupal\Core\Entity\ContentEntityInterface|\Drupal\Core\Entity\EntityPublishedInterface $entity */
    if (!$entity->isNew() && !$entity->isSyncing()) {
      // Force a new revision if the entity is not replicating.
      $entity->setNewRevision(TRUE);

The issue is that this code, as the interface instructs, checks for synchronizing content, i.e.: a migration or default content when a module is first installed. We should not be abusing this by setting isSyncing() to TRUE ourselves.

Furthermore, the current patch will still break on entities that are revisionable but do not implement the synchronizable interface. An edge case, because all entity base classes implement the interface, but an important thing to note nonetheless.

Having said that, the current patch works. So I would understand if people used it until we got something more robust in place. I just can't commit it in the current state as it uses an interface for the wrong reasons.

See https://www.drupal.org/node/3052114: →

Note: the concept of a syncing content entity is still being explored and defined in core. You should carefully evaluate if your use-case fits the definition of a syncing content entity before using this API. This is being discussed in more detail here: 📌 [PP-2] Better describe how SynchronizableInterface should be used for content entities Needs work

//This means we may need to update access records,
// flush some caches containing the entity or perform other operations we
// cannot possibly know about.

if we go back to this comment, may i suggest to add new method to `Drupal\group\Plugin\Group\Relation\GroupRelationInterface`
which takes care of invoking exactly the kind of hooks required for each entity instead of calling $entity->save() ?

Hi!

I have also noted in 3.3.x the issue is still active.

When doing a basic code such as:

  $new_location = \Drupal::service('entity_type.manager')->getStorage('node')->create([my_fields]);
  $new_location->save();
  $new_event->addRelationship($new_location, 'plugin');

if the node entity contains any postSave() function, such as:

  /**
   * {@inheritdoc}
   */
  public function postSave(EntityStorageInterface $storage, $update = TRUE): void {

    // PostSave as expected.
    parent::postSave($storage, $update);

    // Do this only in an update; the insert should be done via hook too.
    if ($update) {
      // Let's do here some redirection here for example
    }
  }

}

that one is triggered one more time during the addRelationship() function as the following function is triggered (in GroupRelationship.php):

  /**
   * {@inheritdoc}
   */
  public function postSave(EntityStorageInterface $storage, $update = TRUE) {
    parent::postSave($storage, $update);

    // For memberships, we generally need to rebuild the group role cache for
    // the member's user account in the target group.
    $rebuild_group_role_cache = $this->getPluginId() == 'group_membership';

    if ($update === FALSE) {
      // We want to make sure that the entity we just added to the group behaves
      // as a grouped entity. This means we may need to update access records,
      // flush some caches containing the entity or perform other operations we
      // cannot possibly know about. Lucky for us, all of that behavior usually
      // happens when saving an entity so let's re-save the added entity.
      $this->getEntity()->save();
    }
  ........