thewca / worldcubeassociation.org

All of the code that runs on worldcubeassociation.org
https://www.worldcubeassociation.org/
GNU General Public License v3.0
333 stars 176 forks source link

We have 3 sources of English country names, and they don't all agree with each other #4389

Closed jfly closed 5 years ago

jfly commented 5 years ago

(CC-ing @Mollerz because this is something I know he cares about)

Right now, we have 3 places you can go to find the name of a country/state:

  1. The Countries.name column in our database
  2. The name attribute in wca-states.json
  3. The English translation (either in en.yml or the i18n-country-translations gem)

We should drop the Countries.name column from the database. Whenever possible, we should not store human readable text in the database: it's not translatable! Unfortunately, we probably still have legacy PHP code that depends on this column existing, and I don't know that it's worth updating that code to do something different.

I think it does make sense to keep the country names in en.yml and in wca-states.json. en.yml is easy for us to query for in a locale-aware way, and we have infrastructure in place to help people translate those strings. We don't have any such infrastructure in place for wca-states.json, and I don't think we should add anything. It may be possible to remove the names from wca-states.json, but that might make the MD file we use to generate wca-states.json impossible to read.

One idea would be to add a new unit test that loops over the contents of wca-states.json and compares the names there to the English translations of the countries. That way, whenever we update wca-states.json, we are forced to also update en.yml accordingly. Here's a simple script that does that check:

search.rb ```ruby templ = "%40s %40s %40s\n" printf templ, "Countries.name", "wca-states.json", "en.yml or `i18n-country-translations`" Country::WCA_STATES["states_lists"].map do |list| list["states"].map do |state| state_id = state["id"] || I18n.transliterate(state["name"]).tr("'", "_") country = Country.c_find(state_id) db_name = country.read_attribute(:name) wca_regs_name = state["name"] i18n_en_name = country.name if [db_name, wca_regs_name, i18n_en_name].to_set.length > 1 printf templ, db_name, wca_regs_name, i18n_en_name end end end ```
➜  ~/gitting/worldcubeassociation.org/WcaOnRails git:(master) ✗ bin/rails runner search.py
Running via Spring preloader in process 14366
                          Countries.name                          wca-states.json    en.yml or `i18n-country-translations`
                     Antigua and Barbuda                      Antigua and Barbuda                        Antigua & Barbuda
                  Bosnia and Herzegovina                   Bosnia and Herzegovina                     Bosnia & Herzegovina
                              Cabo Verde                               Cabo Verde                               Cape Verde
                                   Congo                                    Congo                      Congo - Brazzaville
                           Côte d'Ivoire                            Côte d'Ivoire                            Côte d’Ivoire
   Democratic People’s Republic of Korea    Democratic People’s Republic of Korea                              North Korea
        Democratic Republic of the Congo         Democratic Republic of the Congo                         Congo - Kinshasa
                           Guinea Bissau                            Guinea Bissau                            Guinea-Bissau
                                Holy See                                 Holy See                             Vatican City
                                   Macau                                    Macau                                    Macao
          Federated States of Micronesia           Federated States of Micronesia                               Micronesia
                                 Myanmar                                  Myanmar                          Myanmar (Burma)
                               Palestine                                Palestine                  Palestinian Territories
                       Republic of Korea                        Republic of Korea                                    Korea
                   Saint Kitts and Nevis                    Saint Kitts and Nevis                        St. Kitts & Nevis
                             Saint Lucia                              Saint Lucia                                St. Lucia
        Saint Vincent and the Grenadines         Saint Vincent and the Grenadines                 St. Vincent & Grenadines
                   São Tomé and Príncipe                    São Tomé and Príncipe                      São Tomé & Príncipe
                     Trinidad and Tobago                      Trinidad and Tobago                        Trinidad & Tobago
Mollerz commented 5 years ago

In the test at the bottom, the wca-states.json is fully correct, and the en.yml or i18n-country-translations will need updating to match.

viroulep commented 5 years ago

Since the wca states file is our source of truth, I highly support a test making sure what our country names under the en locale matches the one in the json file! We definitely should override the names in our English locale for those which don't match. I don't think we should keep all the countries name in the locale file though, just those which need to be overridden.

jfly commented 5 years ago

I'll work on adding the test described above!