Fix entity property info of product / customer profile / line item reference fields [#1356006]

The entity property info product reference fields contains some incorrect 'property info', which is not necessary for referenced entities. For referenced entities the regular entity property info is taken into account.

This popped at #1058856-29: Entity tokens not created for multi-value fields.
Also, the patch keeps the query callback in place which should work just fine in case of a "plain" entity reference.

Comment	File	Size	Author
#9	1356006.commerce_reference_field_property_info.patch	2.96 KB	rszrama
#9
	commerce_properties.patch	1.2 KB	fago

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

rszrama CreditAttribution: rszrama commented 29 November 2011 at 17:19

Ahh, was this a recent change or something we've had wrong all along? I'm guessing we'll have the same bug in Customer / Line Item for the customer profile reference and line item reference fields.

Comment #2

fago

German

Vienna

CreditAttribution: fago commented 30 November 2011 at 09:14

No, there were no change - it just did not pop up as the 'property info' key is usually totally ignored for entity references. The linked patch though used it to detect 'struct' properties for which the 'property info' key is for.

I shortly looked at the definition of the Customer / Line Item references - they look good.

Comment #3

rszrama CreditAttribution: rszrama commented 30 November 2011 at 15:22

Ok, thanks for the extra info. I'm a little confused then why we're not using the query callback or setter / getter callback for customer profile reference fields:

/**
 * Callback to alter the property info of the reference field.
 *
 * @see commerce_customer_field_info().
 */
function commerce_customer_profile_property_info_callback(&$info, $entity_type, $field, $instance, $field_type) {
  $property = &$info[$entity_type]['bundles'][$instance['bundle']]['properties'][$field['field_name']];
  $property['options list'] = 'entity_metadata_field_options_list';
  unset($property['query callback']);
}

The same code is used for the line item reference field.

(On a side note, that function is not helpfully named; think I'll update it to make it explicit it's for the reference field.)

Comment #4

pcambra

he/him

Spanish

Asturies

CreditAttribution: pcambra commented 30 November 2011 at 16:13

'query callback' is supposed to be deprecated #1345882: Undeprecate 'query callback' when used with a 'computed' property.
But I'm wondering why are we unsetting it in each field property callback.

Comment #5

fago

German

Vienna

CreditAttribution: fago commented 1 December 2011 at 10:02

'query callback' is supposed to be deprecated #1345882: Undeprecate 'query callback' when used with a 'computed' property.

Yep, still it's used by the field property info generation code what won't change for backward compatibility.

I'm a little confused then why we're not using the query callback or setter / getter callback for customer profile reference fields:

Good point. Fields unset the callback if it doesn't work - usually as they are using another data structure we cannot query for. This was the case for entity references too, as entity_property_query() did not translate entity objects to ids. It does now so though (already for quite some time). Thus, if the field is a plain entity reference (without any struct) it's working just fine now.

I realized we have the same fault in the entity module's term reference field support, so I've fixed it over at #1357902: allow querying for referenced terms and added tests for it. Thus, you can keep the query callback for your entity references and so enable querying for those properties.

Comment #6

pcambra

he/him

Spanish

Asturies

CreditAttribution: pcambra commented 1 December 2011 at 10:31

Marking this one #1306106: Can't load nodes via a product reference in a rule as duplicated, as this change would fix the issue there.

Comment #7

lukus

he/him

CreditAttribution: lukus commented 5 December 2011 at 16:05

I had a problem with 7.x-1.1 where I couldn't reference product replacement tokens via my display node type.

I was using the tokens for display of custom breadcrumbs and auto urls.

After chatting with pcambra, I was advised to apply this patch - I did so, and now I have replacement tokens.

Thanks all.

Comment #8

Summit CreditAttribution: Summit commented 15 December 2011 at 23:24

Hi, Could this patch also solve the problem that term references to products are not shown as real terms, and term references to product-displays are shown as real terms?
greetings, Martijn

Comment #9

rszrama CreditAttribution: rszrama commented 20 December 2011 at 05:19

File	Size
1356006.commerce_reference_field_property_info.patch	2.96 KB

Let's see how this patch goes, then. It removes everything from our reference field property info definition except the options list callback. If that isn't necessary, let's remove it, too. Any tips on that, fago?

Comment #10

pcambra

he/him

Spanish

Asturies

CreditAttribution: pcambra commented 20 December 2011 at 07:20

Actually there's already an issue to remove the line item reference query callback #1358760: Rules "Fetch entity by property" action with line item reference field, marking that one as duplicate.

Comment #11

fago

German

Vienna

CreditAttribution: fago commented 20 December 2011 at 08:55

The options list callback has to be added manually as of now. I guess it should be possible to auto-detect it somehow though.

Anyway, that's not there yet so #9 looks good to me.

Comment #12

rszrama CreditAttribution: rszrama commented 20 December 2011 at 14:28

Title:	fix entity property info of product reference fields	» Fix entity property info of product / customer profile / line item reference fields
Component:	Product reference	» Developer experience
Status:	Needs review	» Fixed