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: nutrition edit warnings #8411

Merged
merged 1 commit into from
May 15, 2023
Merged

fix: nutrition edit warnings #8411

merged 1 commit into from
May 15, 2023

Conversation

stephanegigandet
Copy link
Contributor

This PR builds upon the work of @jnsereko : #8258
It fixes some issues with the original PR, that were found when doing more testing before pushing to production.

  • We need to use parseFloat() on input values, otherwise "47" < "8"

image

  • Commas are replaced by dots, so that French numbers like 4,7 are converted to 4.7
  • Re-ordered the logic so that some warnings like the sugars > 100g warning is not cleared if carbohydrates are then modified (and are above sugars). In practice the "Please enter a value between 0 and 100" now takes priority over the other.
  • Added translations in .po files.
  • Allow values > 100g if nutrients are per serving and not per 100g

@stephanegigandet stephanegigandet requested a review from a team as a code owner May 12, 2023 14:25
@github-actions github-actions bot added Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org labels May 12, 2023
@sonarqubecloud
Copy link

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
0.0% 0.0% Duplication

@codecov-commenter
Copy link

Codecov Report

Merging #8411 (88b8d6f) into main (a0cfac9) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #8411   +/-   ##
=======================================
  Coverage   48.46%   48.47%           
=======================================
  Files         114      114           
  Lines       21268    21277    +9     
  Branches     4768     4770    +2     
=======================================
+ Hits        10308    10314    +6     
- Misses       9677     9679    +2     
- Partials     1283     1284    +1     
Impacted Files Coverage Δ
lib/ProductOpener/Import.pm 30.73% <0.00%> (-0.04%) ⬇️
lib/ProductOpener/Packaging.pm 75.00% <75.00%> (ø)

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

@github-actions github-actions bot added the ⭐ top pull request Top pull request. label May 13, 2023
var nutrient_value = \$(this).val().replace(',','.');
var is_above_or_below_100 = (isNaN(nutrient_value) && nutrient_value != '-') || nutrient_value < 0 || nutrient_value > 100;
// if the nutrition facts are indicated per serving, the value can be above 100
if ((nutrient_value > 100) && (\$('#nutrition_data_per_serving').is(':checked'))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we check that it's below serving size ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky, because serving size is parsed on the server (it can be specified with different units etc.), but the user can change it in the form while editing nutrients.

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧽 on-the-fly quality checks ⭐ top pull request Top pull request. Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants