-
-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: delete fields after removing ingredients #8943
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8943 +/- ##
=======================================
Coverage 46.04% 46.04%
=======================================
Files 64 64
Lines 19805 19807 +2
Branches 4796 4796
=======================================
+ Hits 9119 9121 +2
Misses 9499 9499
Partials 1187 1187
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Set to draft. Need to clarify why this ecoscore test was updated after the changes. EDIT: solved |
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks a lot for fixing this!
What
For https://world.openfoodfacts.org/product/07562174/salade-3-fromages-u.
Percentage 236.5% was introduced in https://world.openfoodfacts.org/product/07562174/salade-3-fromages-u?rev=11 (December 3, 2021 at 12:37:05 PM CET)
Current version has the same percentage.
Not easy to reproduce the quality error (above 105) locally, but, locally create a product with same ingredients and nutrition data as https://world.openfoodfacts.org/product/07562174/salade-3-fromages-u?rev=11, leads to fruits-vegetables-nuts-estimate-from-ingredients_100g of 11.5 (not sure to understand why it is not 236.5%, maybe calculation method has changed since 2021).
Deleting ingredient leave it as it is.
Fix: estimate_nutriscore_fruits_vegetables_nuts_value_from_ingredients($product_ref); was called inside a condition: "if (defined $product_ref->{ingredients})" this does not handle the case when there was an ingredient list and it has been removed. Suggestion solution, add a "else" and delete fields that may have been created in the past.
Also found big typo when handling previous bug: "ingredients_tags, ingredients_original_tags" -> "ingredients_tags", "ingredients_original_tags". Added "ingredients_hierarchy" on the same list.
Screenshot
Before - after
Related issue(s) and discussion
This removes some fields when ingredients are removed.
Not sure for the data quality, but I assume that it is checked AFTER Ingredients.pm and that it will be updated accordingly. But this has to be checked and as described before, I could not get the data quality warning locally with the example of the issue.
Comment
Allergen and traces remain after deleting ingredients. There are 2 fields:
Note however that locally, I DID NOT enter allergens and traces by myself, they were added from the ingredients list.
=> In any cases, explicitly remove allergen in edit mode. This will remove them totally from the api results