Add XK and AN to continental grouping

[Flufd]

What are you trying to accomplish?

Add continent grouping to Kosovo (XK) and Netherlands Antilles (AN)

The associated_continent attribute currently returns nil for these. When we translate these regions' continent names in atlas, we fall back to the EN group name. This gives a mixed language experience, even though we have translations for these continents.

What approach did you choose and why?

I updated the world.yml to reflect the geographic positions of these countries, putting AN into the Caribbean, and XK into Southern Europe. This file is meant to reflect the M49 data, but as AN is a deprecated country code, it doesn't appear in the data, and XK is not fully recognised by all of the UN, so I assume that's why it's not there either.

What should reviewers focus on?

Should this be done another way to avoid overwriting the M49 data?

The impact of these changes

...

Testing

...

Checklist

• I have added a CHANGELOG entry for this change (or determined that it isn't needed)

[cejaekl]

I think that adding XK under 039 ("Southern Europe") is, indeed, the right thing to do. 👍

😕 I'm unsure whether we really want to be adding AN to this list. In some sense, having AN cause an error by having a null value is a feature, not a bug. We have run into problems in the past (many, many times...) where code assumed that AN is a "going concern" and got into trouble down the road. Perhaps @kwiersema or @devanandersen might want to chime in here?

[Flufd]

'm unsure whether we really want to be adding AN to this list.

Initially I was only looking at fixing XK, and was trying to find other instances where a country didn't have an associated continent, to add in a test to stop regressions. Happy to drop this part and add in a test with AN as a known "country" region without a continent. For my use case this should be fine. What we were seeing was an extra group in our country selector when we were grouping by continent name. As AN does not appear in our country list, it makes little difference.

[devanandersen]

In some sense, having AN cause an error by having a null value is a feature, not a bug.

Agree - had the Antilles not have been dissolved, I think this PR on its own would make sense. If anything, we should be slowly removing information for AN, not adding.

Would you be able to briefly go into your use case?

Happy to drop this part and add in a test with AN as a known "country" region without a continent.

This I'm hesitant of - AN isn't technically a known "country" anymore - it was dissolved October 10, 2010.

[cejaekl]

Agree - had the Antilles not have been dissolved, I think this PR on its own would make sense. If anything, we should be slowly removing information for AN, not adding.

The difficulty with removing AN (and the reason why it's still present here) is that we want to be able to process information about historical orders that were placed prior to October 2010, at which time AN was a "going concern".

[Flufd]

Would you be able to briefly go into your use case?

Sure. My use case is a country selector in the Shopify admin that allows you to select countries to be added to a market.

Note the Europa and Europe groups.

We pull this data of the available countries, and their continent "group", from the Atlas API. Atlas falls back to a region's group name if it does not find a continent in the Worldwide::Region's associated_continent. This group name is never translated from English. If we find a continent, we will use it's translated name. This fallback is the legacy behaviour for the continent field.

So for Kosovo, which is supported by Shopify as a region that can be added to a market, we get it sorted into a different untranslated group, if we use a language where "Europe" is not translated into "Europe", e.g. German.

[cejaekl]

My use case is a country selector in the Shopify admin that allows you to select countries to be added to a market.

IMHO, "Netherlands Antilles" should not be shown in the list, and should not be selectable. You shouldn't be able to add a country that no longer exists to a market.

We should be removing any "country" for which .deprecated? returns true when constructing the list of countries to display:

shopify(dev)> Worldwide.region(code: "AN").deprecated?
=> true

[Flufd]

IMHO, "Netherlands Antilles" should not be shown in the list, and should not be selectable. You shouldn't be able to add a country that no longer exists to a market.

Yes, sorry for any confusion, that is the case now. It doesn't appear on our list, I think that we need to specifically ask for deprecated countries on Atlas in order for them to show up, and we aren't doing that.

I'll push an update to remove AN from the change.

[devanandersen]

lgtm once we remove AN 👍🏼