search

auntbertha / openeligibility

The Open Eligibility Project
35 stars 24 forks source link

Fix incorrect IDs for Food category #24

Closed monfresh closed 11 years ago

monfresh commented 11 years ago

Food was using 101 for many of its top and second level categories instead of 102.

monfresh commented 11 years ago

If you're adhering to semantic versioning, this pull request should actually not be applied to the existing release. It would require a major version change as this is a non-backwards compatible fix.

monfresh commented 11 years ago

Then again, a taxonomy with duplicate IDs is not valid, and since this bug exists in at least the past 2 versions, no one should be using those versions.

I'm not sure how semantic versioning deals with this case, but since no one should be deploying to production with this bug, it seems like it should be OK to fix the bug in the existing versions. Otherwise, make it clear that versions with that bug should not be used and tell anyone who's using the taxonomy to fix their deployments.

In our case, we discovered this bug when we noticed emergency categories showing up in food-related entries. I had to manually update the OE IDs for the incorrect food categories, and now we're going to have to manually go through all the food entries we tagged to update them where necessary with the correct categories.

erineg1 commented 11 years ago

Moncef-- Thanks for the suggestions and updates. I just want to make sure I understand the issue correctly. I created the numbering portion per a request from Eric. While doing so, I apparently had messed up this category (thanks for pointing this out - and also thanks for the git suggestions - as we're relatively new).

So simply updating this second level category should work for your implementation?

monfresh commented 11 years ago

As far as I understand it, the whole point of numbering is to facilitate the exchange of data. If Aunt Bertha is using "Help Pay for Food" as the display name for the category under "Emergency Payments", and if Aunt Bertha wants to display search results from the Ohana API, which might be using an older version of the taxonomy that uses "Cash for Food", then using the category ID as opposed to the display name is how Aunt Bertha can continue to use the newer taxonomy term. The Ohana API includes the OE category ID to facilitate this exchange.

The problem here, and it's a big problem, is that some of the food categories were using the exact same ID as the emergency categories, which will break this exchange of data and result in incorrect classification of the services.

Simply fixing it on your side will not automatically fix it on our side (or anyone else who might be using OE) since we're doing work on our side to convert the XML to JSON and then store the categories as a nested set in our DB. We already fixed it on our end so that the API now returns the correct IDs.

On your end, since you only have the latest version up on GitHub, you can update it directly by applying my pull request. However, you should make a note of this change in the README in case anyone else is using OE in production to let them know to fix it on their end. If you know who they are, you should also email them directly about this bug.

erineg1 commented 11 years ago

I understand the issue now, thanks. We need then to create a unique ID for each tag. We'll take a closer instance and fix. Thanks again for pointing this out.