feat: Prevention - On the fly quality checks in the product edit form #8258

jnsereko · 2023-03-27T07:53:18Z

What

Created an invisible span having a warning on saturated fats and sugars values. inputing an element above the range displays the span.

Screenshot

onfly.mov

Related issue(s) and discussion

Fixes Prevention - On the fly quality checks in the product edit form (javascript) #1905

github-advanced-security · 2023-03-27T07:57:06Z

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

jnsereko · 2023-03-27T08:00:27Z

templates/web/pages/product_edit/product_edit_form_display.tt.js

@@ -34,3 +34,21 @@
 		\$('#nutrition_data_div').show();
 	}
 });
+
+  \$('#nutriment_sugars').on('input', function() {
+    var inputValue = $(this).val();


hey @stephanegigandet @yuktea
I jQuery is blocking this from working

its like the .val() is not a function. Uncaught TypeError: this.val is not a function

think i have called the element correctly so it must be working

Hi @jnsereko , it's because this file is a template file, so anything starting with $ is interpreted as a variable in the template. If you want the $ to be output in the JS code, you need to escape it like this: \$

thanks @stephanegigandet

jnsereko · 2023-03-27T08:36:45Z

this @stephanegigandet somewhat messes up the table. i thought of creating a warning div, just like https://github.com/jnsereko/openfoodfacts-server/blob/d9631fdd0e73ac311a9512ebf156533369f775ae/templates/web/pages/product_edit/product_edit_form_display.tt.html#L78-L82 but since the table is somewhat long, covering almost the whole screen users might not see it

stephanegigandet · 2023-03-27T14:18:09Z

this @stephanegigandet somewhat messes up the table. i thought of creating a warning div, just like https://github.com/jnsereko/openfoodfacts-server/blob/d9631fdd0e73ac311a9512ebf156533369f775ae/templates/web/pages/product_edit/product_edit_form_display.tt.html#L78-L82 but since the table is somewhat long, covering almost the whole screen users might not see it

One option could be to have an extra table row with the warning, just below the value that is problematic. This way you could use the whole row width (the 3 columns).

stephanegigandet · 2023-03-27T14:22:45Z

templates/web/pages/product_edit/product_edit_form_display.tt.html

@@ -227,6 +227,11 @@ <h1>[% title %]</h1>

                                            <td class="nutriment_col" [% column_display_style_nutrition_data %]>
                                                <input class="nutriment_value nutriment_value_as_sold" id="nutriment_[% nutriment.enid %]" name="nutriment_[% nutriment.enid %]" value="[% nutriment.value %]" [% nutriment.disabled %] autocomplete="off"/>
+                                                [% IF nutriment.nid == 'saturated-fat' %]


The issue was not specific enough, but the warnings we have in mind are more things like the warnings we have in https://github.com/openfoodfacts/openfoodfacts-server/blob/main/lib/ProductOpener/DataQualityFood.pm#L631

e.g. if the nutrients are per 100g, then we should not have any value greater than 100.

same for sub values: sugars should not be higher than carbohydrates, and saturated fats should not be higher than fat.

Maybe we could start with only those 2 checks, and then gradually add more.

CharlesNepote · 2023-03-29T13:06:42Z

Thanks a lot @jnsereko. The video is nice to clearly understand how it works.

About the user interface: I'm afraid that the way it is displayed will have bad side effects:

Displaying a red message could lead the user to abort. Many people do, when seeing a red alert.
The column is taking too much place: it will make the side image far smaller and could lead to image readability issues.

I suggest two points:

When the error occurs, colorize the field background (with a soft color, not too flashy).
When the error occurs, display a simple (?) (a question mark inside a circle, I guess you can do it in CSS), and a text displayed when the mouse is going over it (in Power User Script I use the title HTML attribute, no need to use javascript).

Here is an example I use in Power User Script (but the background color here is too flashy):

I'm not sure if the color should be displayed while typing or when the user is leaving the field for another one.

jnsereko · 2023-04-25T09:09:29Z

hello @stephanegigandet @CharlesNepote @yuktea @alexgarel
Sorry this has taken some time.

The current push has some changes attending to some of @stephanegigandet and @CharlesNepote suggestions.
ie

When the error occurs, colorize the field background (with a soft color, not too flashy).

When the error occurs, display a simple (?) (a question mark inside a circle, I guess you can do it in CSS), and a text displayed when the mouse is going over it (in Power User Script I use the title HTML attribute, no need to use javascript).

if the nutrients are per 100g, then we should not have any value greater than 100.

same for sub values: sugars should not be higher than carbohydrates, and saturated fats should not be higher than fat.

The current video in the PR description shows a visual result.

stephanegigandet · 2023-04-25T09:43:16Z

Hi @jnsereko , this looks quite good, in terms of functionality and how to display the warning, it's great I think.

stephanegigandet · 2023-04-25T09:45:19Z

templates/web/pages/product_edit/product_edit_form_display.tt.js

+var saturated_fats_value;
+var fat_value;
+
+[% FOREACH nutriment IN nutriments %]


This works, but it will generate a huge JS file, with the code repeated for all nutrients.

It would be better to create a javascript array that contains all nutrients, and then have a for loop in Javascript, instead of a foreach loop in the template language.

CharlesNepote · 2023-04-25T12:12:25Z

Well done @jnsereko! The video looks really cool. Last point: I would choose a lighter color to highlight errors, not to frighten people. The yellow is a bit flashy...

@stephanegigandet would it be possible to test it on openfoodfacts.net?

jnsereko · 2023-04-26T23:39:52Z

Well done @jnsereko! The video looks really cool. Last point: I would choose a lighter color to highlight errors, not to frighten people. The yellow is a bit flashy...

@stephanegigandet would it be possible to test it on openfoodfacts.net?

hey @CharlesNepote
Would pale pink work better?

jnsereko · 2023-04-26T23:43:55Z

templates/web/pages/product_edit/product_edit_form_display.tt.js

+
+var required_nutrients_id = ['energy-kj', 'energy-kcal', 'fat', 'saturated-fat', 'sugars', 'carbohydrates', 'fiber', 'proteins', 'salt', 'sodium', 'alcohol'];
+
+required_nutrients_id.forEach(nutirent_id => {


@stephanegigandet, using the ordinary for loop is tricky. Only the last nutrient, "alcohol" is accessed the colour but the on-input event exists on other elements

Is this okay?

It looks good this way, thank you! :)

Could you just replace all the "nutirent" by "nutrient" ? :)

thanks @stephanegigandet
That slipped my eyes

jnsereko · 2023-05-02T09:37:01Z

hey @stephanegigandet @alexgarel
All tests are passing now 😄

stephanegigandet · 2023-05-02T10:28:16Z

@jnsereko Thank you!

stephanegigandet · 2023-05-02T10:30:27Z

templates/web/pages/product_edit/product_edit_form_display.tt.js

+	\$('#nutriment_' + nutrient_id).on('input', function() {
+		var nutrient_value = \$(this).val();
+		var is_above_or_below_100 = isNaN(nutrient_value) || nutrient_value < 0 || nutrient_value > 100;
+		show_warning(is_above_or_below_100, nutrient_id, "Please enter a value between 0 and 100");


The warnings should be added to po/common/common.pot and po/common/en.po so they can be translated, but we can also do that later in another PR.

@stephanegigandet can we do the translations in another PR
i have tried them locally but it might take me some time to make the logic work.

stephanegigandet · 2023-05-02T11:32:10Z

tests/integration/fix_non_normalized_codes.t

 		# product_non_normalized_code moved
 		"Moved 0000012345678 to 12345678",
+		# removed product with existing id


This is a bug that is not related to your PR, could you remove this change?
The bug is fixed in this PR: #8384

sonarqubecloud · 2023-05-02T16:47:25Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

CharlesNepote · 2023-05-05T14:21:03Z

The question mark should not appear when there is a "hyphen", meaning this data is not displayed on the packaging. It's important.
Should I open another bug or not?

See:

jnsereko requested a review from a team as a code owner March 27, 2023 07:53

github-actions bot assigned jnsereko Mar 27, 2023

github-actions bot added ✏️ Editing Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. labels Mar 27, 2023

jnsereko commented Mar 27, 2023

View reviewed changes

jnsereko mentioned this pull request Mar 27, 2023

Prevention - On the fly quality checks in the product edit form (javascript) #1905

Closed

2 tasks

stephanegigandet reviewed Mar 27, 2023

View reviewed changes

jnsereko force-pushed the 1905 branch 4 times, most recently from ea935b4 to 4ff4132 Compare April 24, 2023 22:19

jnsereko force-pushed the 1905 branch 2 times, most recently from f08256e to 085445f Compare April 25, 2023 09:15

jnsereko changed the title ~~Prevention - On the fly quality checks in the product edit form~~ feat: Prevention - On the fly quality checks in the product edit form Apr 25, 2023

stephanegigandet reviewed Apr 25, 2023

View reviewed changes

jnsereko added 2 commits April 26, 2023 22:24

feat: Add quality checks on product edit page.

a864cd2

change warning color to pale pink

4eceb15

jnsereko force-pushed the 1905 branch from 085445f to 4eceb15 Compare April 26, 2023 23:37

jnsereko commented Apr 26, 2023

View reviewed changes

jnsereko and others added 3 commits April 28, 2023 04:42

rerun build

3b3073e

remove unnecessary css

a3f20ad

Fix typo: replace nutirent with nutrient

5d4794f

jnsereko and others added 2 commits May 2, 2023 11:30

Merge branch 'main' into 1905

2d4ca6d

fix unit test

b913100

github-actions bot added the 🧪 tests label May 2, 2023

jnsereko requested a review from stephanegigandet May 2, 2023 10:15

stephanegigandet approved these changes May 2, 2023

View reviewed changes

stephanegigandet reviewed May 2, 2023

View reviewed changes

Merge branch 'main' into 1905

07b9c11

stephanegigandet reviewed May 2, 2023

View reviewed changes

remove non required changes

7502a4d

github-actions bot removed the 🧪 tests label May 2, 2023

Merge branch 'main' into 1905

b702302

stephanegigandet merged commit 89da8ff into openfoodfacts:main May 3, 2023

openfoodfacts-bot mentioned this pull request May 3, 2023

chore(main): release 2.12.0 #8363

Merged

stephanegigandet mentioned this pull request May 12, 2023

fix: nutrition edit warnings #8411

Merged

CharlesNepote mentioned this pull request May 15, 2023

On the fly quality checks issues #8420

Closed

teolemon added the 🧽 on-the-fly quality checks label May 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Prevention - On the fly quality checks in the product edit form #8258

feat: Prevention - On the fly quality checks in the product edit form #8258

jnsereko commented Mar 27, 2023 •

edited

Loading

github-advanced-security bot commented Mar 27, 2023

jnsereko Mar 27, 2023

stephanegigandet Mar 27, 2023 •

edited

Loading

jnsereko Mar 27, 2023

jnsereko commented Mar 27, 2023

stephanegigandet commented Mar 27, 2023

stephanegigandet Mar 27, 2023

CharlesNepote commented Mar 29, 2023

jnsereko commented Apr 25, 2023

stephanegigandet commented Apr 25, 2023

stephanegigandet Apr 25, 2023

CharlesNepote commented Apr 25, 2023

jnsereko commented Apr 26, 2023

jnsereko Apr 26, 2023

stephanegigandet May 2, 2023

jnsereko May 2, 2023

jnsereko commented May 2, 2023

stephanegigandet commented May 2, 2023

stephanegigandet May 2, 2023

jnsereko May 2, 2023

stephanegigandet May 2, 2023

sonarqubecloud bot commented May 2, 2023

CharlesNepote commented May 5, 2023


		var required_nutrients_id = ['energy-kj', 'energy-kcal', 'fat', 'saturated-fat', 'sugars', 'carbohydrates', 'fiber', 'proteins', 'salt', 'sodium', 'alcohol'];

		required_nutrients_id.forEach(nutirent_id => {

feat: Prevention - On the fly quality checks in the product edit form #8258

feat: Prevention - On the fly quality checks in the product edit form #8258

Conversation

jnsereko commented Mar 27, 2023 • edited Loading

What

Screenshot

Related issue(s) and discussion

github-advanced-security bot commented Mar 27, 2023

Choose a reason for hiding this comment

stephanegigandet Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnsereko commented Mar 27, 2023

stephanegigandet commented Mar 27, 2023

Choose a reason for hiding this comment

CharlesNepote commented Mar 29, 2023

jnsereko commented Apr 25, 2023

stephanegigandet commented Apr 25, 2023

Choose a reason for hiding this comment

CharlesNepote commented Apr 25, 2023

jnsereko commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnsereko commented May 2, 2023

stephanegigandet commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 2, 2023

CharlesNepote commented May 5, 2023

jnsereko commented Mar 27, 2023 •

edited

Loading

stephanegigandet Mar 27, 2023 •

edited

Loading