Countries entity implementation may be wrong; fails if countries are Organic Groups

name is the key that the entity api needs to tell it that the db storage is the iso2 key, so without this, the cid will be used and everything will eventually go bang :(

So what is the bug exactly?

"you are no longer able to save a country reference"

Validation error? Nothing is saved to the DB? CID is saved rather than the ISO2?

Appears to have a corresponding issue in the Entity API queue. Marking as a duplicate as these sound similar.

#1937856: Always allow entity id for EntityDrupalWrapper

There is a patch albeit throwing errors

I have moved the steps to replicate there. Cheers for reporting.

(Copied from the other thread)

I haven't the time to setup / check, but a workaround within the countries module could be to define the country name key type in countries_entity_property_info_alter(). No idea of the format, so start with the devel debug:

dpm($info['country']['properties']['name']);

if set, add this:

$info['country']['properties']['name']['type'] = 'string';

Note that the token type fails as the module uses the ISO standard of using Uppercase and the token validation is the Drupal default of validating the machine codes lower case char followed by * lowercase chars, numbers or underscores :/

Please let me know if this works and I'll update the module and also mark that other thread as fixed ;)

Nice, so there are two things here maybe to alter, iso3 is text or token?

This should be the fix:

$info['country']['properties']['iso2']['type'] = 'text';

and things should just work.

Using Entity Reference, cid is probably used in the db, using the country widgets iso2 should be used in the db. Track a new content item through to see what is happening and if it is returned correctly. Then if there are issues with the old existing content, resaving these should fix those

Great, if you upload a patch, I can commit it through to dev.

The bit about saving old content was the hope that the ER was using ISO2 rather than CID now (blind hope that some magic somewhere was doing something different). This is clearly not the case from what you have said. If you were using the country fields, (not ER fields) the storage would have been ISO2.

Anyways, there must be an issue with the reference field / entity api itself (outside of our control). So the ER widget uses the ID (cid) but the Entity API part is correctly thinking that it is the NAME (iso2).

I guess that this is working at least and validation probably is not required, and hopefully will continue to work, but maybe one of these two options:

a) followup with the Entity Reference issue queue to extend the field to allow conditional support for the NAME property if present. I say conditionally allow as if it blindly changes to use NAME if set over ID, then all existing sites that use that field will loss their data.

b) find the integration point between Entity API and Entity Reference and somehow make ER tell the Entity API that this is the ID rather than the NAME property being used as a key (...even without looking, this could be hard)

PS: If the ER widget is a select list, it has alternative built in validation that is very hard if not impossible to bypass. I'd stop chasing the bug at that point unless you have free time to help out resolving the root issue / know of another way to trigger the error.

"why have both?"

Countries is far older than ER and it is nice to be able to have very customizable widgets without complex additional integration with another module ;)

If you create / find the ER issue, can you cross post back here for others to track?

I found #2143171: Entity Metadata Wrapper support broken for named, exportable entities., but I'm not sure if this is the answer either

black heart of Entity Reference

lol, love the description. I have extended early plugins of this module, only to have api changes that blew everything up and at the point I have limited myself to never extending the plugins again.

Cheers for everything, I'll be looking at doing some real work in the queues over the next month or so. Till then, I think that this is rtbtc

@Alan D, What needs to be done to get this patch committed?

A co-maintainer... interested?

Yea, I know the feeling. But no thanks.

I think we fix from #12 is correct. We need to switch to 'text' as data type because tokens are lowercase (see Data Type Reference" or in code entity_property_verify_data_type()). But we can specify an additional callback.

@Alan: OG is not required to reproduce the issue. All you need is "drush php-eval 'print_r(entity_metadata_wrapper('country', "DE")->raw());'".

+function countries_entity_validate_iso2($data) {
+  return is_scalar($data) && preg_match('!^[A-Z]{3}$!', $data);
 }

iso2 is a 2 chars long string, not 3, isn't it ?

First of all, validation callback must be available in a global scope because the Entity API module will not load the *.info.inc.

Additionally, I've modified the validation callback allowing it to recognize the objects having __toString() methods.