Closed mblennegard closed 4 months ago
Unsaturated fat property implemented as part of PR https://github.com/reaper47/recipya/pull/382
Edit: Note that the Unsaturated fat property does not save properly when editing a recipe. It works fine when scraping. @reaper47 Do you know what code is being triggered when manual edits are done to a recipe?
Here's the flow of manual edits:
It seems I have indeed forgotten the unsaturated fat property when mapping the request to the nutrition: https://github.com/reaper47/recipya/blob/main/internal/server/handlers_recipes.go#L751-L761
Edit: The handler's name should have been recipesEditPutHandler
rather than recipesEditPostHandler
because the request is actually a PUT.
@reaper47 Thanks. I have fixed the Unsaturated fat property in another branch and am currently adding the Trans fat property as well (a bit more work since that one was missing from the schema and therefore all handler tests etc.).
However, I think I might have stumbled upon another error whilst doing the above. I noticed that in the below lines the conversion for ug and mg are the same.
I believe that ug should instead be 1e6 on below line, correct? https://github.com/reaper47/recipya/blob/2a07cf9da7dd579d8ca8c8766996e073c276269e/internal/models/recipe.go#L679
Edit: Regarding the Trans fat property, I found that it was not only missing from the nutrition schema but missing entirely from the database. I believe I have found all the places in terms of DB interaction where I need to add the new property, but I am a bit uncertain in terms of adding the field to the actual database. Is it enough to add the new property to the below file and location, or does changes to the DB encompass other work that needs to be done (since the below file has a timestamp in the name I am thinking that this file might be generated from elsewhere)? https://github.com/reaper47/recipya/blob/2a07cf9da7dd579d8ca8c8766996e073c276269e/internal/services/migrations/20230609214207_add_recipes.sql#L57-L71
Yes, the conversion is the same for ug and mg to format tiny numbers properly. Let's say the initial nutrition value is 0.0000056g. The first if condition would bump itto 0.0056mg. Since the value is still below 1, the second if will bump it to 5.6ug. Perhaps the second if should be within the first because we have a duplicate comparison when the value is above 1.
Another great catch! How could I have missed the transFatContent
property from the recipe schema :/ Since we are unfortunately (or fortunately :D) past the v1.0.0 release, we should not modify old SQL files because updates won't apply to current users of the software. This being said, changes to the database require a new migration file alongside the others. You can create one by running task new-migration name=add_transfat
. Here is an example that adds a field to the nutrition table: https://github.com/reaper47/recipya/blob/main/internal/services/migrations/20240107135944_add_nutrition_serving.sql
@reaper47 Oh, got it.
Excellent! Thank you for that .sql example code. :pray:
Describe the bug In the recipe schema, as well as already being handled by the scraper, there are properties for
transFatContent
andunsaturatedFatContent
. However, these two properties are missing from the nutritional information table (see screenshot below).To Reproduce Steps to reproduce the behavior:
Expected behavior Expected to see Trans fat and Unsaturated fat in the table, along with the already existing Saturated fat and Total fat properties. When having a sum of all three different fat types, the Total fat should probably be displayed in a different manner, e.g. as a header, having bold text or having its value located to the left instead of the right etc. to make it clear that is indeed a sum of other properties.
Screenshots
Desktop (please complete the following information):