fatfreecrm / fat_free_crm

Ruby on Rails CRM platform
http://www.fatfreecrm.com
Other
3.57k stars 1.32k forks source link

Country select preferred values doesn't use country key for option value #213

Closed steveyken closed 11 years ago

steveyken commented 11 years ago

When specifing preferred_countries in a country_select box you end up with where the option value is the name of the country but this should be the ISO code as that is stored in the database for normal countries that are chosen.

balazstar commented 11 years ago

Hi steveyken,

A few lines in country_select.rb would fix the issue that you described. I have not yet created a pull request because I think this issue needs further discussion.

My solution would be:

    # Add iso code for countries 
    priority_countries_iso = COUNTRIES.select{|c| priority_countries.include?(c[0]) }

    # Add iso code for selected countries if not nil
    unless selected.nil?
        selected = COUNTRIES.select{|c| selected.include?(c[1]) }.flatten
    end

    country_options += options_for_select(priority_countries_iso, selected)

But sadly there are issues with the COUNTRIES array in country_select.rb so that lookup by country code would not work.

For example: ['Aland Islands', 'FI'], ['Finland', 'FI'], Here the country code is the same, according to ISO 3166 AALAND ISLANDS is misspelled and should have the code AX.

I think an update of COUNTRIES array is also needed. If you agree with the direction I will gladly make the mentioned fixes.

steveyken commented 11 years ago

Hi tarbalazs, thanks for offering a solution. You're thinking exactly along the same lines as I was so please go ahead and make the changes in a pull request. Thanks for spotting the country list errors. It should be brought up to date with the latest revision of ISO_3166: http://en.wikipedia.org/wiki/ISO_3166-1 (Note the list you linked to was an older revision and didn't contain newly formed countries such as South Sudan). Could you also add Kosovo (XK) as well? This is yet to be included in the ISO list but the European Commission and other bodies are using it with this code. (See http://geonames.wordpress.com/2010/03/08/xk-country-code-for-kosovo/ )

Another thought: the current code assumes a default list of 'preferred countries' if that option is not supplied to the function. I wonder if the default behaviour should be to show no preferred countries'. It would be good to have a setting in FFCRM to set the list of preferred countries in config/settings.yml rather than a hardcoded default lists.

If you feel like doing this second part as well, we'd welcome it. But otherwise, what you describe above is great too.

Thanks for offering to assist. We do appreciate it.

Regards, Steve

steveyken commented 11 years ago

A friend just noticed that the codes for Australia and Austria are switched/incorrect:

    ['Austria', 'AU']          should be            ['Austria', 'AT']
    ['Australia', 'AS']       should be            ['Australia', 'AU']

Looks like there are more (“AS” is actually the code for “American Samoa”)

Here’s some good web references:

http://countrycode.org/ http://en.wikipedia.org/wiki/ISO_3166-1

I think it would be worth checking all codes!

balazstar commented 11 years ago

Thanks Steve,

I have also noticed, I will update it from Wikipedia.

I think a migration is also needed, so that old country codes will be updated in the database. Do you agree?

Cheers, Balázs

steveyken commented 11 years ago

Excellent and you're right, we will need to update the data. I'm in two minds about a migration though... I would suggest a rake task instead? The migration would work fine now but suppose we change the model or methods later on. It might break the migration. Generally, I think it's better to put data manipulation into a rake task.

balazstar commented 11 years ago

Hi Steve,

I think this issue can be closed.

Tin-Nguyen commented 7 years ago

hi all, how about Kosovo in countries list? have we added it yet?