Closed (fixed)
Project:
Entity API
Version:
7.x-1.x-dev
Component:
Entity property wrapper
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
3 Jan 2011 at 14:58 UTC
Updated:
17 Mar 2014 at 13:40 UTC
Jump to comment: Most recent
Comments
Comment #1
fagohm, internally parent::value() should be an list of ids, so that patch is not the right fix for your problem.
For a single entity, the API is capable of dealing with objects too, however still the usual representation is its id - e.g. it passes the id to a setter callback of an entity reference property. So maybe you have a getter callback returning the list of values? Then it should return a list of ids.
Comment #2
rszrama commentedI'm at a loss here... I don't really understand comment #1, but I'm happy to make any changes necessary to get this working again. I can't really isolate what part of the wrapper process is failing (i.e. is it the initial wrapping, the use of count($wrapper->line_items) on the list in the wrapper, or the foreach($wrapper->line_items as ...) on the list in the wrapper).
All I've really been able to nail down is that with no changes to the Commerce Cart module itself, the changes to entity.wrapper.inc between 1.1.2.2 and 1.1.2.3 seem to be causing the error (reverting to 1.1.2.2 allowed it to function just fine).
Perhaps we do need to change something in our Entity integration now, but that property on the order wrapper is getting defined automatically as it's a line item reference field. The pertinent integration in the Line Item module seems to be:
Comment #3
fagoThe default field getter just gets the value of your first field column, being 'line_item_id'. So maybe this is sometimes not an id but the full entity, e.g. like that?
If that's the case, then it gets full entities instead of ids, what leads to that problems. I've no idea why it worked before, because it was never intended to work like that - although it certainly would be a nice addition. ;)
So we have to options:
* Improve the wrappers to allow a list of entities to be passed as full objects too. For that we would have to check for that in the lists value() method and update $this->data with the list of ids - as parent::value() would return a list of objects. Also, all other uses of parent::value() in the list wrapper need to be replaced by value(array('identifier' => TRUE)) then, such that a list of ids returned.
* Improve the list item property integration to make sure a list of ids is returned. E.g. overwrite the default getter callback 'entity_metadata_field_property_get' with your own that returns a numerically indexed array of entity ids.
Comment #4
rszrama commentedAhh, pcambra reminded me that we do in fact have our own getter for the line item reference field on Orders...
I believe we thought that in order to use the line items properly through Rules, we had to 1) explicitly add the field to the order entity property info like node does for the body field and 2) define a getter that returned the line item entity array (id => object). For some reason this worked before, but it looks like we need to revert this to just return an array of IDs (or rather, to not have a custom getter at all).
In fact, what's interesting to me is that in my code comments I said we should use a metadata wrapper to avoid having to hardcode a language constant, but I realized it would be a little redundant to try and use a wrapper when defining the info that makes the wrapper possible. ; )
So do we just need to nuke this getter / will Rules still work with the line item entities, or do we just need to rewrite this getter somehow and work around the language problem?
Comment #5
rszrama commentedOk, digging deeper... what I did was nuke the custom getter (I can't remember why we had it in the first place) and set the getter of the line items property to 'entity_metadata_field_property_get' as you suggested. This results in things working fine as far as I can tell, but I don't know if there's anything else we should be doing... or as I mentioned above, if we should even be explicitly adding the line items property to the order info array. For default / "locked" fields, I wonder if there'd be a way for Rules to automatically represent these fields in data selectors?
Patch is here: http://drupal.org/node/1012888#comment-3899922
Comment #6
fagohm, if you have a ususal field on the order, why do you manually define the property info at all? It looks like you have provided the property info integration for the line item field type, so it should automatically generate a property for the field. Any reason to not go with that?
>1) explicitly add the field to the order entity property info like node does for the body field
This is done for nodes such that one needs not check the node type in order to be able to use the body field. But do you have bundles for orders at all?
Comment #7
rszrama commentedWell, we do have bundles, but they'll always have a line item reference field on them. The problem is that I can't get the line items of an order to show up in the data selector if I don't explicitly add them in the commerce_order.info.inc.
For example, I have a global variable I've added to the site variables for the current user's shopping cart order. So, on a random Rule, in the actions I'm trying to add a loop and over the line items and display the line item ID in a message. I can select the line items if I have that code above, but if I don't it's impossible. I thought perhaps it just wasn't getting defined as a list, so I tried editing this code to no avail:
I don't think I wrote that code originally, but maybe I did. ; ) Am I missing something?
Comment #8
fagohm, I was only able to find bundles -except for the default bundle - in your code? Anyway, if you have multiple bundles you can make use of the field by first checking for the bundle, e.g. with the data comparison condition or directly checking for the field (Entity has field). Also, if you have a property referencing to a specific bundle of an order, you can just specify that with 'bundle' => value in the property info.
At last, yep you can just add the property info of the field to the main property info of the entity. I'd suggest to output the generated property info for a bundle (e.g. with dpm(entity_get_property_info())) and provide the same information manually - just as for node bodies (see http://drupalcode.org/viewvc/drupal/contributions/modules/entity/modules...).
Comment #9
rszrama commentedAhh, yeah, at the database level we support multiple order bundles (i.e. commerce_order has a type column), but we aren't really doing anything with it right now in 1.0. I think a good 2.0 target could be to provide some further level of support... but I'm not sure if that'll be wise or not given the possibility of needing to convert from one order type to another. : ?
I finally figured out what you meant with our info declaration... I think it doesn't make sense for us to add field data into the entity property info array, but I do think from a usability standpoint it would be worthwhile for Rules to support showing fields present on every bundle of a particular entity type without having to check the bundle type or for the existence of the field. I'm going to remove our definition for not and just instruct people to use the "Entity has field" condition for now.
In Commerce for practically every entity type there are default locked fields that we put on every bundle, such as price on products, line items on orders, address field on customer profiles, etc. Functionally, these "fields" are properties of the entities, but we don't have to define them as such since Entity adds field data itself. Either Rules could pick up on those fields automatically so they are always available in data selectors or we could perhaps have some way to declare in the entity property array which fields are present on every bundle.
Let me know if I should repost that as a feature request in the Rules queue or if it's better suited for the Entity queue. : )
Comment #10
fago>Let me know if I should repost that as a feature request in the Rules queue or if it's better suited for the Entity queue. : )
It's something we should fix in the entity API - as it is just about hook_entity_property_info() which Rules makes use of via the wrappers. Core misses support for having fields available on any bundle, so the entity API just adds the metadata for what is there. However, with modules caring about having the field available everywhere, it's up to them to adjust the property info accordingly as we have done it for node bodies and comment bodies.
So far I'd say, there is nothing wrong with that approach. However, we could make doing so easier - e.g. by moving the property info automatically once a special flag in $instance is set. Though, it's questionable which label we should pick then, as each instance / bundle might have different labels and descriptions. But probably it would be safe to assume they are all equal (as the fields should be locked).
Comment #11
peem83 commentedentity_list_wrapper.fully_loaded_value.patch queued for re-testing.
Comment #12
fagoAny cause to re-open this issue?
Comment #14
geek-merlinReturning a list<node> from a rule and dsm-ing it gave me this so this seems not fixed.
Comment #15
geek-merlinhmm, it's more complicated than this - need more investigation...
Comment #16
geek-merlinyep, in fact i returned fully loaded entities from a property getter.
looks like the mess is here.
Comment #17
fgjohnson@lojoh.ca commentedHi,
If this is not an appropriate forum please excuse. I hope this is the same issue.
Drupal 7.23 is installed SLEZ 11 -sp3 (which uses php5.3).
We are using a plethora of content types that use Entity_API and Field (I think) to reference each other via content creation and for display via Views.
Worked fine for a while as I was developing then it just WSOD with errors similar to the following.
< start >
Fatal error: Class 'SelectQuery' not found in /srv/www/htdocs/includes/database/database.inc on line 812
< end >
What am I missing!
Thanks
Comment #18
fagoThat's not supported, from the getters/setters you have to use entity IDs if you have lists of entities. The wrappers getters/setters support IDs and entity objects though.
Setting back to fixed.