search

ThreeSixtyGiving / standard

The 360Giving data standard for UK philanthropic giving
http://www.threesixtygiving.org
Other
10 stars 15 forks source link

Removed type 'null' from countryCode #378

Closed mrshll1001 closed 4 months ago

mrshll1001 commented 4 months ago

countryCode allows for a type of either 'string' or 'null'. Unless there is a specific reason why null is permitted as a countryCode we should amend this.

The semantics of having a type of null in JSON Schema:

https://json-schema.org/understanding-json-schema/reference/null

I did want to flag that this is such an odd choice that it may have been deliberate i.e. if we wanted to accommodate unrecognised territories which are not yet assigned a countryCode.

If accepted, I believe this should represent a PATCH level change however if there is existing data with null as a countryCode it would become invalid.

We may well reject this PR on the basis that it constitutes a breaking change and not a bugfix, which I'm fine with.

KDuerden commented 4 months ago

thanks @mrshll1001

if there is existing data with null as a countryCode it would become invalid.

I interpret the rules that breaking changes are changes that have the potential to break data, whether or not actual data would be broken. This should be packaged alongside any other "MAJOR in theory not practice" changes as part of any future version upgrade.

odd choice that it may have been deliberate i.e. if we wanted to accommodate unrecognised territories which are not yet assigned a countryCode.

As this field is a property of the location definition, it gets applied to funding or recipient organisations or beneficiaries. Although it doesn't make much sense in the context of organisations, apart from as an outlet for unrecognised countries, for 'beneficiaries' there is a sense that 'null' mirrors what we did with the location scope codelist, in terms of allowing for a deliberate 'n/a' value.

I'm now curious whether there is any documentation that explains this as a 'bug' or deliberate. Does schema.org or IATI's country code fields include a similar null option which we inherited this from? However, as I believe this change is blocked as a consequence of its status as a breaking change, finding out the answer to this question isn't a high priority right now.

mrshll1001 commented 4 months ago

Good question.

There's nothing obvious on schema.org which makes me think that we inherited it from there. I can see that Country and a few other objects have an addressCountry property which accepts either a plaintext string e.g. "United Kingdom" or a two-letter ISO 3166-1 country code.

In terms of IATI, I've asked some of our ODS colleagues which work more closely with IATI to see if they have any insight but it looks like they also just state a preference for the country code in the summary tables for each organization and activity for the recipient country specifically. I'm not as fluent in XML Schema as JSON schema but as far as I can tell, there's no allowance for null there either.

I'm going to close this PR as blocked by the fact it's a MAJOR upgrade as it has the potential to break data, and raise an issue to reference the conversation and document it for if/when we do version 2.0.