Nodes and users created on EntityContext, must be stored as array

[juanjol]

EntityContext must keep the same structure as the parent class for the nodes and users arrays, storing the object with nid, type, etc., not only the id. This throw a warning:

Warning: Attempt to read property "nid" on string in vendor/drupal/drupal-driver/src/Drupal/Driver/Cores/Drupal8.php line 114

This PR fixes the structure of the arrays, removing this warning. In the future, it is advisable to review whether EntityContext.php really needs to keep track of nodes and users, as the parent class already takes care of those entity types.

[rsanzante]

There is a problem with the fix.

The patch changes the type of the data stores in Metadrop\Behat\Context\EntityContext's $users and $nodes properties. I don't see $nodes or $users in parent classes. But the mai problem is Metadrop\Behat\Context\EntityContext itself.

Let's see the method getGivenEntitiesMap:

  protected function getGivenEntitiesMap(): array {
    $map = [];

    foreach ($this->entities as $item) {
      $map[$item['entity_type']][] = $item['entity_id'];
    };

    $map['user'] = $this->users;
    $map['node'] = $this->nodes;

    return $map;
  }

This function returns entity ids. It's clear when you see where is used, in purgeEntities:

  $given_entities = $this->getGivenEntitiesMap();
[...]
      if (!empty($given_entities[$entity_type])) {
        $entities_ids = array_diff($entities_ids, $given_entities[$entity_type]);
      };

I agree we should reviewed if this is required or not, but I thjink the patch should address also the getGivenEntitiesMap methods, may it's just to update it so it returns ids.

[omarlopesino]

There is a problem with the fix.

The patch changes the type of the data stores in Metadrop\Behat\Context\EntityContext's $users and $nodes properties. I don't see $nodes or $users in parent classes. But the mai problem is Metadrop\Behat\Context\EntityContext itself.

Let's see the method getGivenEntitiesMap:

  protected function getGivenEntitiesMap(): array {
    $map = [];

    foreach ($this->entities as $item) {
      $map[$item['entity_type']][] = $item['entity_id'];
    };

    $map['user'] = $this->users;
    $map['node'] = $this->nodes;

    return $map;
  }

This function returns entity ids. It's clear when you see where is used, in purgeEntities:

  $given_entities = $this->getGivenEntitiesMap();
[...]
      if (!empty($given_entities[$entity_type])) {
        $entities_ids = array_diff($entities_ids, $given_entities[$entity_type]);
      };

I agree we should reviewed if this is required or not, but I thjink the patch should address also the getGivenEntitiesMap methods, may it's just to update it so it returns ids.

@albeortev @juanjol at this lines of EntityContext:

      if (!empty($given_entities[$entity_type])) {
        $entities_ids = array_diff($entities_ids, $given_entities[$entity_type]);
      };

$given_entities[$entity_type] must be an array of entity IDS, but with the last change it is a list of entities. Can you adapt it?

Suggestion:

      if (!empty($given_entities[$entity_type])) {
        $entities_ids = array_diff($entities_ids, array_map(function (EntityInterface $entity) {
           if (isset($entity->nid)) {
              return $entity->nid;
           }
           elseif (isset($entity->uid)) {
             $entity->uid;
          }
           else {
             return null;
           }
        }, $given_entities[$entity_type]));
      };

[juanjol]

Updated the PR, now we use the nodes and users IDs, but this is redundant as clearing of nodes is done on the parent class.

[omarlopesino]

The EntityContext clears entities created during scenarios that are not managed by contexts with @purgeEntities tag. For example, a node created by submitting the node/add form.

That lines of code that you have adapted prevents deleting the entities that have been created by contexts. So it is still necessary adapting it.

[omarlopesino]

THanks for the fix @juanjol , after @albeortev review this we can merge it.