Problem/Motivation

If I create a relation type like this one:

  $relation_type = new stdClass();
  $relation_type->disabled = FALSE; /* Edit this to true to make a default relation_type disabled initially */
  $relation_type->api_version = 1;
  $relation_type->relation_type = 'foo_bar';
  $relation_type->label = 'Foo Bar';
  $relation_type->reverse_label = 'Bar Foo';
  $relation_type->directional = 1;
  $relation_type->transitive = 0;
  $relation_type->r_unique = 1;
  $relation_type->min_arity = 2;
  $relation_type->max_arity = 2;
  $relation_type->source_bundles = array(
    0 => 'node:foo',
  );
  $relation_type->target_bundles = array(
    0 => 'node:bar',
  );
  $export['foo_bar'] = $relation_type;

Two entity properties are provided on nodes: relation_event_foo_bar_node_reverse and relation_foo_bar_node.

If I create two nodes, one foo and one bar related to each others trough a foo_bar relation, using these two properties on an entity metadata wrapper for the foo node always return a single element list containing the node itself. The relation_event_foo_bar_node_reverse should return and empty list since the wrapped foo node is not the target of any foo_bar relation. And relation_event_foo_bar_node should return the bar node since it is the target of a foo_bar relation whose source is the wrapped foo node.

Digging into the code, there is no r_index or source-target check in the getter functions for the properties (ie. relation_rules_get_related_entities). Instead, for each relation whose source or target is the wrapped entity, the function return the endpoint matching the type of the property.

CommentFileSizeAuthor
#11 relation-related-entities-of-a-node-include-the-node-itself-1811400-11.patch603 bytesb-prod
#5 1811400-5.patch588 bytesbenjy

Comments

adamjw’s picture

adamjw commented

Hi - is there any chance of addressing this issue?

I've fixed it for my purpose by inserting an r-index check into the relation_rules_get_related_entities function, so that it only returns entities that are the target of the relationship and excludes those that are the source:

  foreach (relation_load_multiple($rids) as $relation) {
    foreach ($relation->endpoints[LANGUAGE_NONE] as $endpoint) {
      if ($endpoint['entity_type'] == $info['target_type']) {
             if ($endpoint['r_index'] <> 0) {
                $entities_ids[] = $endpoint['entity_id'];
             }
      }
    }
  }

I'm using this in the context of the Search API module, where returning both the source and target entities was a problem - particularly in the context of creating an Index hierarchy. I'm not yet sure if my 'fix' causes problems elsewhere.

Thanks to all those behind the Relation module - really useful!

Jorrit’s picture

Jorrit commented

I had a similar problem when using the Relation Select module, I also resorted to filtering on the r_index. To make your solution more complete, only filter on r_index <> 0 when the relation is directional.

naught101’s picture

naught101 commented

Stick it in a patch, and I'll commit it if it doesn't break anything. It'd be good to see some tests for this too, I think, since I'm not entirely sure I follow what's going on here.

mikran’s picture

mikran commented
Priority: Major » Normal
benjy’s picture

benjy commented
Version: 7.x-1.0-rc3 » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new 1811400-5.patch588 bytes

Lets try this.

mikran’s picture

mikran commented
Status: Needs review » Needs work

Steps to repeat the problem are a bit unclear. Is this same as #2477661: Rules integration endpoint fetch should return a list instead of array keyed by entity ID?

benjy’s picture

benjy commented

The problem is that $entity_ids should be returned with the $entity that is passed into relation_rules_get_related_entities.

mikran’s picture

mikran commented

So testRelationLoadRelatedRules test is wrong?

benjy’s picture

benjy commented

I had a quick look at that test in the code browser, it doesn't directly call relation_rules_get_related_entities so I'm not sure if it tests relation_rules_get_related_entities() indirectly? But, if it does, and we have a bug here i'd say there is a problem with that test, yes.

b-prod’s picture

b-prod commented

Actually the problem is that this function returns the source entity as well as the related entity, when both source and target entity types are the same.

For example, lest's say I have a relation between two types of nodes (same bundle or not, this doesn't matter):

  • Node A with ID 1
  • Node B with ID 2

If I call this function passing the node A, I expect to get the node B only. Bur in fact I get the two nodes.

This issue may cause big troubles when using different types of nodes for the relation and indexing through Search API.

How to reproduce :

  • Create a node type "type_1"
  • Create a node type "type_2"
  • Create a field "field_test" for "type_2" nodes, that does not exist for "node_1" type.
  • Create a relation "are_together" between those two types of node
  • Create a search index for nodes of type "type_1", with a nested field: relation_are_together_node:field_test
  • After creating some dummy nodes and relations, try to index the data

Here we get an error in the search_api_extract_fields() function, when reaching the line foreach ($wrapper as $w). Indeed, the foreach loop is also applied on the node of type "type_1", as it is returned by the function relation_rules_get_related_entities().

So this need to be fixed.

Another point that could be addressed in another issue is the properties definition, as the "reverse" property is exactly the same as the other one, except that it defines a different 'target_type' that is not used in the query... But this is not the main purpose of the current issue so I don't go further.

b-prod’s picture

b-prod commented
Status: Needs work » Needs review
StatusFileSize
new relation-related-entities-of-a-node-include-the-node-itself-1811400-11.patch603 bytes

After further investigation, it appears that the issue with Search API is caused by another bug in the Entity module, which may be solved by the patch here: #2090007: EntityDrupalWrapper::getIterator() throws PHP error: EntityMetadataWrapperIterator::__construct() must be an array

Solving the fact that the source entity is returned in the results seems trivial, but we need to be sure that this is the expected behaviour. Maybe the module maintainers wanted this function to return even the node passed in the arguments. If so, a comment in the function and the token descriptions would be great, otherwise the patch below fixes the issue.

Status: Needs review » Needs work

The last submitted patch, 11: relation-related-entities-of-a-node-include-the-node-itself-1811400-11.patch, failed testing.

b-prod’s picture

b-prod commented

The error has non sense :

Fatal error: Class 'view' not found in /var/www/html/sites/all/modules/relation/tests/relation.views.test on line 289

The view class is correctly found in the other methods, and this failure is not related to the changes in the patch...

The last submitted patch, 11: relation-related-entities-of-a-node-include-the-node-itself-1811400-11.patch, failed testing.