Enums for Ingredients and Location
lukasdenk opened this issue · 11 comments
Enum for Ingredients
Maybe one could merge the ingredients?
E.g., the ingredient list for the StudentenwerkParser contains a field with peanut while the list for the FMIParser contains a field Erdnüsse (engl. peanuts) (see entities.py.Ingredients).
I would suggest to create an Enum Ingredient with fields like studentenwerk_lookup, fmi_lookup. This way, our clients have a clear definition on the ingredients a dish might contain. Also they do not need to handle dishes from different locations in different ways.
Enum for DishType
For the same reasons, one could create an Enum DishType.
Discussion
Since this would require breaking changes, I would like to know what the project members think about it (@COM8, @srehwald). If they agree, I would like to discuss on how we want to handle backwards compatibility.
Maybe we could create a version 2 for the API. Along with, e.g. https://tum-dev.github.io/eat-api/mensa-garching/2019/20.json, we could also serve the new dishes with https://tum-dev.github.io/eat-api/v2/mensa-garching/2019/20.json
Edit
Add Location enum.
Do not add DishType enum. For reasons see here.
I think this would be a good idea. Maybe, we could reuse the label we currently have, so it would be no breaking change?
However, more readable labels could be a better solution.
Additionally, I would propose to add a ingredients.json/allergenes.json into the api, just as the canteens.json.
Here we could also include a short description and maybe even a symbol/emoji, as it is used in the windows app, and now also in the webversion. This file could look like this:
Lines 8 to 60 in 572a79e
Yes, we should unify ingredients and add a command to export them as JSON with their emoji (and translated? @Philipp000 ) description. The same for dish types.
@lukasdenk I don't see a reason why changing the ingredients names should break compatibility at all?
Would you like to include an enum value instead of the actual ingredient in the resulting JSON?
@COM8: As I see it, when an FMI dish contains food with peanuts, then the entities.Ingredient.ingredient_set will contain the string Erdnuss. When a Studentenwerk dish contains peanuts, the ingredient set will contain with peanut.
When we merge the ingredients, our API returns the same enum in both cases. As a consequence, a client relying on the string with peanut for Studentenwerk canteens and for the string Erdnuss for the FMI bistro would get into problems.
Ah, OK. I don't think this will be a problem, since currently only the website and the Windows app use ingredients.
Ok, so I won't make the ingredients backwards compatible then :)
Ah, OK. I don't think this will be a problem, since currently only the website and the Windows app use ingredients.
@COM8 Is this the same with dish types?
Yes.
Additionally, I would propose to add a ingredients.json/allergenes.json into the api, just as the canteens.json
@Philipp000, @COM8: I think a better idea might be to generate the JSONs from the Ingredient enum and automatically update them. This way, we can be sure that the JSON always contains the complete list.
@lukasdenk I am joining a little late and, well, I haven't been an active member of this project for a while now :)
Anyways, I like your suggestions! Go for it!
@srehwald Thanks.
@COM8 I think I will also make locations an enum. I think there are several benefits from it:
- Stricter typing (since a function taking a Canteen enum can only take a canteen then. Right now, a function taking a canteen can take any string).
- Code completion support
- The supported locations are clearly defined and documented
What do you think?
Sounds really nice 👍🏻.