Hello all, When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. In the same way that the BZ rectifying the problems linked to the payment of fines (bug 33028) obliges the input with a point I will propose a corrective in this same direction.
Created attachment 157302 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point
Created attachment 157626 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com>
You removed the "validated" class from the form, this disables all standard validation that already was applied here. An examples: the mandatory field "Name" now uses the standard HTML5 message instead of our own. Email and website no longer validate at all. You need to keep the Validator active and implement your check additionally.
Created attachment 158921 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point
(In reply to Katrin Fischer from comment #3) > You removed the "validated" class from the form, this disables all standard > validation that already was applied here. > > An examples: the mandatory field "Name" now uses the standard HTML5 message > instead of our own. Email and website no longer validate at all. > > You need to keep the Validator active and implement your check additionally. Hello Katrin, Thanks for your comment, sorry for the error. I tried to fix it, but I kept the deleted class, and added a validation for the 'discount' input, I think it's correct for now, but if you see a problem, I'd be happy to hear from you.
I confirm that it does on longer break the other validation, but I am not sure if it's the right way to do it. Adding Owen in hope of some guidance. Also: Your hint displays as: % - Example: 7.5 % - that doesn't look quite right yet. I wonder if using a standard class of Validator would be better here? We could use number like we do on Bug 34169 - Add validation for monetary input fields in acquisition module, but maybe with its own class and hint? That would also make it reusable - the discount is also set on the order form and there might be other use cases like the search field weighting.
Created attachment 159831 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: Sabrina <kiehl@mpis.mpg.de>
Tested following the mentioned steps, warning was shown correctly.
(In reply to Katrin Fischer from comment #6) > We > could use number like we do on Bug 34169 I agree... There doesn't seem to be a reason not to. In similar cases we don't add any kind of hint (e.g. a <div class="hint"> under the input field), so I don't think we have to include that in this patch.
Created attachment 161128 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point
Thanks for your feedback, I've modified the code to create a specific class for reuse it
Created attachment 161225 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com>
Created attachment 161237 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I thought we were attempting to go the other way generally.. i.e. allowing people to input in their local format and translating at our end. This patch works though technically and resolves a problem so I'm happy to PQA.. at some point I'd really like to see a concerted effort to fix these formatting numerics issues globally.
Hi Thibaud, thanks for your perseverance! Can you please change these lines a bit so that the string is translatable? Maybe assigning it to a variable first? + decimal_rate: $.validator.addMethod("decimal_rate", $.validator.methods.number, "Please add rate in valid format: 0.00"),
Created attachment 161596 [details] [review] Bug 35087: (follow-up) string validation is now translatable
Hello Katrin, Thanks ;) now normally it's done
Created attachment 161597 [details] [review] Bug 35087: alternative patch to display validation Same test plan as for the other patch, but here validation impacts the js asset
Created attachment 161598 [details] [review] Bug 35087: alternative patch to display validation Same test plan as for the other patch, but here validation impacts the js asset
Are all 3 patches needed?
(In reply to Katrin Fischer from comment #20) > Are all 3 patches needed? Hello Katrin, No, the third is only an alternative way to fix it without touch include (all is done in staff-global.js).
Hi Thibaud, what do we need here, additional feedback or are you still working on the patches?
Sorry I should have been more exhaustive, the alternative patch doesn't modify the 'validator-string' include so I thought that in terms of integration it would be simpler to have just one modified file. Nevertheless, I'll let you "choose" the best solution for validating the patch and I'll change the solution not chosen to "obsolete".
Created attachment 161704 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 161705 [details] [review] Bug 35087: alternative patch to display validation Same test plan as for the other patch, but here validation impacts the js asset Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 161706 [details] [review] Bug 35087: (follow-up) Accept only values which will be saved correctly This patch adds a regex (thanks to Katrin for the help) specific to the range of values which will be accepted by the discount column in the database which has the type "float(6,4)". The patch also removes the format() filter from the template so that values returned from the database won't be truncated: Using "format ('%.1f')", a value in the db of '9.009' would appear in the edit form as '9.0', a change which could be missed if the user was editing some other field in the vendor edit form. To test, apply the patch and clear your browser cache if necessary. - Go to Acquisitions -> Vendors -> Edit vendor. - Under "Ordering information," test entering various values in the "Discount" field. It should accept numbers with up to two digits before the decimal and up to three digits after the decimal: 9, 99, -99, 99.9, 0.99, 99.99, 99.999 - In each of these cases, confirm that a value accepted by the client-side validation is also accepted when you submit the form and is correctly displayed when you edit that vendor again. - It should not accept: 100, 100.001, 100.00001 - The error message has been updated to read, "Please enter a decimal number in the format: 0.0" Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 162065 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
Created attachment 162066 [details] [review] Bug 35087: alternative patch to display validation Same test plan as for the other patch, but here validation impacts the js asset Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
Created attachment 162067 [details] [review] Bug 35087: (follow-up) Accept only values which will be saved correctly This patch adds a regex (thanks to Katrin for the help) specific to the range of values which will be accepted by the discount column in the database which has the type "float(6,4)". The patch also removes the format() filter from the template so that values returned from the database won't be truncated: Using "format ('%.1f')", a value in the db of '9.009' would appear in the edit form as '9.0', a change which could be missed if the user was editing some other field in the vendor edit form. To test, apply the patch and clear your browser cache if necessary. - Go to Acquisitions -> Vendors -> Edit vendor. - Under "Ordering information," test entering various values in the "Discount" field. It should accept numbers with up to two digits before the decimal and up to three digits after the decimal: 9, 99, -99, 99.9, 0.99, 99.99, 99.999 - In each of these cases, confirm that a value accepted by the client-side validation is also accepted when you submit the form and is correctly displayed when you edit that vendor again. - It should not accept: 100, 100.001, 100.00001 - The error message has been updated to read, "Please enter a decimal number in the format: 0.0" Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
Hi, tested on a sandbox. When we edit a vendor with a discount with no decimal (ex : 10), nothing is added when we saved it. If we try to enter a number with a comma in the discount, a red message appears "Please enter a decimal number in the format: 0.0" It's no more possible to make the discount not recorded because of a wrong input.
number: _("Please add amount in valid format: 0.00"), + decimal_rate: $.validator.addMethod("decimal_rate", $.validator.methods.number, "Please add rate in valid format: 0.00"), Confusing + <input class="rate" type="text" size="6" id="discount" name="discount" value="[% discount | format ('%.1f') %]" />%</li> .1f linked to 0.00 ?
We have 3 patches. And later patches revert stuff from earlier ones. Please squash, explain, fix ?
Created attachment 164739 [details] [review] Bug 35087: Discount rate only accepts a point as decimal separator When a comma is used to define the discount rate of a vendor, the data is not recorded (as NULL), it is however recorded if the separator is a point. Test plan: 1) Go to acqui module and edit a vendor 2) Set a discount rate with a comma as decimal separator 3) Edit again or request your database to see that there is no data saved 4) Do it again with a point as decimal and see it works 5) Apply this patch and reloard the build for the templates 6) Repeat step 2 and see the warning to inform you that you must use a point Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
Created attachment 164740 [details] [review] Bug 35087: (follow-up) Accept only values which will be saved correctly This patch adds a regex (thanks to Katrin for the help) specific to the range of values which will be accepted by the discount column in the database which has the type "float(6,4)". The patch also removes the format() filter from the template so that values returned from the database won't be truncated: Using "format ('%.1f')", a value in the db of '9.009' would appear in the edit form as '9.0', a change which could be missed if the user was editing some other field in the vendor edit form. To test, apply the patch and clear your browser cache if necessary. - Go to Acquisitions -> Vendors -> Edit vendor. - Under "Ordering information," test entering various values in the "Discount" field. It should accept numbers with up to two digits before the decimal and up to three digits after the decimal: 9, 99, -99, 99.9, 0.99, 99.99, 99.999 - In each of these cases, confirm that a value accepted by the client-side validation is also accepted when you submit the form and is correctly displayed when you edit that vendor again. - It should not accept: 100, 100.001, 100.00001 - The error message has been updated to read, "Please enter a decimal number in the format: 0.0" Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
I just tested it again and it doesn't work. If I put "4,5" in the discount rate of a vendor, then save and edit it again the discount rate hasn't been saved, the discount rate is empty. Sonia
(In reply to Koha Team University Lyon 3 from comment #35) > I just tested it again and it doesn't work. > If I put "4,5" in the discount rate of a vendor, then save and edit it again > the discount rate hasn't been saved, the discount rate is empty. > > Sonia Normally this should not be possible. I just tested again, the validation of the form imposes a comma, did you rebuild the css ? (This is an option on the sandbox management interface).