Closed (fixed)
Project:
Location
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
21 Feb 2011 at 22:36 UTC
Updated:
11 Apr 2011 at 12:51 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
tr commentedBetter patch, uses public Drupal function country_get_list() rather than internal Drupal function _country_get_predefined_list().
Comment #2
tr commentedSigh, that was the same patch. Here's the new one, I hope.
Comment #3
tr commentedHonest, I really do know how to roll a patch. Don't know what's gotten into me today. Here's what I believe is the correct patch.
Comment #4
rooby commentedBeen meaning to do this, thanks for the patch.
Committed a slightly different version, which is what is used in location 7.x-4.x (may as well keep in line where possible).
It also does away with the static part because _country_get_predefined_list() does that anyway.
http://drupal.org/cvs?commit=503642
Comment #5
tr commentedThe core _country_get_predefined_list() / country_get_list() maintains a static copy of the array with upper class country code keys. Because the Location module uses lower case country code keys, this static copy cannot be used directly. Instead, the upper case keys need to be converted to lower case keys within location_get_iso3166_list() before being returned. My patch did this conversion once then statically cached the converted array with lower case keys - this is in addition to and separate from the static caching done by _country_get_predefined_list() because the array is different. If you don't do the static part in the patch, then the array keys need to be re-converted to lower case and the entire array needs to be reconstructed each time location_get_iso3166_list() is called on a page.
Comment #6
rooby commentedI see your point.
Will need to address this in DRUPAL-7--3 and DRUPAL-7--4.
Will do it tonight.
Comment #7
Anonymous (not verified) commentedHi there,
I'm using 7.x-3.x-dev and kept getting the error 'The specified province was not found in the specified country.'
I found where the error message is being created and it refers back to what's happening in this issue so thought I'd post here.
I used dd($obj['country']) and dd($provinces) to find that Drupal's core list in iso.inc is passing through 'GB' but the location module uses 'UK' so the $provinces array is returned empty.
I've temporarily fixed this by changing iso.inc to 'UK', but of course this will lead to the death of kittens, however I'm not sure as to the correct way to fix this so thought I'd post and see if those more knowledgeable than I can be of any help...
Comment #8
Anonymous (not verified) commentedOK, here's a first attempt at a patch to fix what I was talking about in #7. I've just copied location.uk.inc to location.gb.inc and renamed functions replacing uk with gb. Works for me...
Comment #9
tr commented@stevepurkiss: So to summarize, Location is using the wrong 2-character code for the United Kingdom. The ISO 3166 standard says it should be GB (or 'gb' for Location), but Location is using 'uk' instead. When the patch went in that switched to using the Drupal function for the country codes, the Drupal function returned the proper 'gb' instead of the expected 'uk', breaking Location.
I think in addition to #8, there needs to be a renaming of database/zipcodes.uk.mysql (with a corresponding change in the content of that file) and a hook_update() to modify any of the zipcode information already installed in the database. These changes ought to be done anyway - perhaps this should be split off into a new issue?
Comment #10
Anonymous (not verified) commented@TR thanks for the summary!
Just spent some time comparing the two lists - iso.inc and Location's supported country files. There's a few more inconsistencies:
Exists in iso.inc but not in location/supported/location.XX.inc
'BL' - Saint Barthélemy - there is no location.bl.inc
'MF'- Saint Martin (French part) - there is no location.mf.inc
Exists in location/supported/location.XX.inc but not in iso.inc
location.ac.inc - Ascension Island - this is listed as an 'exceptional case' (http://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#Exceptional_reservations) and doesn't appear in the ISO list
location.cs.inc - Serbia and Montenegro - split into RS & ME as of 2006 (http://www.iso.org/iso/pressrelease.htm?refid=Ref1031) so presume location.cs.inc is deprecated
location.sw.inc - Sweden - this is SE in ISO, there is also a location.se.inc so presume location.sw.inc is deprecated
Comment #11
tr commented@stevepurkiss: I rolled a patch for Drupal core about a week ago: #1068840: core/includes/standard.inc contains inaccurate country data. It corrects iso.inc by adding missing countries / deleting an obsolete country / renaming some countries and in general bringing iso.inc into conformance with ISO 3166. I wouldn't mind a review :-)
Ascension Island is part of 'Saint Helena, Ascension and Tristan da Cunha' 654, SH, SHN
I agree BL and MF need to be added to Location, likewise CS needs to be removed in favor of RS and ME, and SW is the wrong code, needs to be SE. There may be a few more that Location is missing if you compare to the updated list from the above patch.
Comment #12
Anonymous (not verified) commented@TR done: http://drupal.org/node/1068840#comment-4148432
Comment #13
rooby commented@stevepurkiss:
There is an existing issue for the uk/gb issue - #1083128: Change country code for UK to GB so you can post your patch for that over there.
This issue is just to use the drupal core country list.
Comment #14
Anonymous (not verified) commentedOK cool thanks!
Comment #15
rooby commentedI have now committed the static caching of the lower case country list.
With a little change in the logic.
Committed to 7.x-3.x and 7.x-4.x.
http://drupalcode.org/project/location.git/commit/3462e15
http://drupalcode.org/project/location.git/commit/a9d62a1