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: allow unchecking checked boxes in product edit form #6203

Merged
merged 2 commits into from
Jan 5, 2022

Conversation

stephanegigandet
Copy link
Contributor

This fixes an issue with my recent refactoring of the code to handle nutrition facts edit from the web product edit form and the API.

The issue is that when a checkbox is unchecked on a form, the browser actually does not send the field at all. So if the parameter is not there, we don't know if it's because it is not set on purpose (e.g. the API is changing something else), or the box is there but has been unset (through the web edit form).

The trick is to add a hidden field with the same name of the checkbox on the web form. That way we do get the hidden value if the box is unchecked, and the checked value if it is checked.

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.

Does not work for me.

@@ -96,12 +96,16 @@

<!--nutrient fieldset-->
<div class="fieldset" id="nutrition"><legend>[% lang('nutrition_data') %]</legend>
<!-- empty hidden checkbox with the same name so that the browser sends a value when the box is unchecked -->
<input type="hidden" name="no_nutrition_data" value="" />
Copy link
Member

Choose a reason for hiding this comment

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

Hum this does not work for me !

If I check the box, this is not taken into account.

In Food.pm, in assign_nutriments_values_from_request_parameters, param("no_nutrition_data") is ''

@sonarcloud
Copy link

sonarcloud bot commented Jan 3, 2022

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

@stephanegigandet
Copy link
Contributor Author

@alexgarel I updated the code to add a new hidden field, as you suggested if I remember correctly.

elsif (defined param($checkbox . "_displayed")) {
$product_ref->{$checkbox} = "";
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Cool

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.

Works !

@alexgarel alexgarel merged commit dd25800 into main Jan 5, 2022
@alexgarel alexgarel deleted the uncheck-boxes branch January 5, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants