Skip to content
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

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

benbenben2
Copy link
Collaborator

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
Screenshot_20230903_145949

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:

  • allergens_from_ingredients is ""
  • allergens_from_user that is populated
    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

@benbenben2 benbenben2 requested a review from a team as a code owner September 3, 2023 13:09
@benbenben2 benbenben2 self-assigned this Sep 3, 2023
@github-actions github-actions bot added 🥗 Ingredients 🥗🔍 Ingredients analysis https://wiki.openfoodfacts.org/Ingredients_Extraction_and_Analysis labels Sep 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2023

Codecov Report

Merging #8943 (8064523) into main (f4fa9ce) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           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           
Files Changed Coverage Δ
lib/ProductOpener/Ingredients.pm 91.60% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@benbenben2 benbenben2 marked this pull request as draft September 4, 2023 20:56
@benbenben2
Copy link
Collaborator Author

benbenben2 commented Sep 4, 2023

Set to draft. Need to clarify why this ecoscore test was updated after the changes.

EDIT: solved

@benbenben2 benbenben2 marked this pull request as ready for review September 6, 2023 15:42
@sonarcloud
Copy link

sonarcloud bot commented Sep 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@stephanegigandet stephanegigandet left a 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!

@stephanegigandet stephanegigandet merged commit aea40ca into main Sep 7, 2023
13 checks passed
@stephanegigandet stephanegigandet deleted the fix-8589-fruit-veggies-estimate branch September 7, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥗🔍 Ingredients analysis https://wiki.openfoodfacts.org/Ingredients_Extraction_and_Analysis 🥗 Ingredients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fruits-vegetables-nuts-estimate-from-ingredients_100g is not reset when ingredients are deleted
3 participants