Description
Chris Cormack
2010-05-21 00:52:23 UTC
I retested this on current master and it's still a problem: I created manual invoices: €3.00 results in 0.00 3.00€ results in 3.00 Also a similar problem was reported for cost/price fields in items (bug 2635). Created attachment 36989 [details] [review] Bug 2542: Validate the amount of a manual invoice If a manual invoice amount is not correctly set, it should not be accepted. Test plan: Try to create a manual invoice with an amount containing something else than numbers. It should not be possible. Created attachment 36990 [details] [review] Bug 2542: Validate the amount of a manual invoice If a manual invoice amount is not correctly set, it should not be accepted. Test plan: Try to create a manual invoice with an amount containing something else than numbers. It should not be possible. The patch prevents aumount with non-number characters. Questions: I can enter a negative number. Should that be possible? - It changes an invoice to kind of credit. Additionally, I can still enter non-number characters in "Create manual credit" (and put negative numbers here as well, changing a credit to kind of invoice). Should that be fixed here or with a separate Bug? Created attachment 37145 [details] [review] Bug 2542: Validate the amount of a manual invoice/create If a manual invoice/credit amount is not correctly set, it should not be accepted. Test plan: Try to create a manual invoice and a manual credit with an amount containing something else than numbers. It should not be possible. (In reply to Marc Véron from comment #4) > The patch prevents aumount with non-number characters. > > Questions: > > I can enter a negative number. Should that be possible? - It changes an > invoice to kind of credit. > > Additionally, I can still enter non-number characters in "Create manual > credit" (and put negative numbers here as well, changing a credit to kind of > invoice). Ok both issues should be fixed with this patch. Created attachment 37166 [details] [review] [Signed-off] Bug 2542: Validate the amount of a manual invoice/create If a manual invoice/credit amount is not correctly set, it should not be accepted. Test plan: Try to create a manual invoice and a manual credit with an amount containing something else than numbers. It should not be possible. Followed test plan. Patch behaves OK, including that negative values are not allowed. Signed-off-by: Marc Véron <veron@veron.ch> Why start with 0, but empty the field, when the fine type is changed? The 'required' on the field has no effect like this - you can still save 0 value fines and credits if you don't change the fine type but use the preselected value. Was actually watching a coworker running into this problem today... will be good to see it fixed :) Created attachment 37202 [details] [review] Bug 2542: Does not empty the amount field when the fine type is changed Some thoughts on this: 1) I think displaying the 0 is not good for ergonomy. I can type 03.00, but I feel libraries will think they need to delete the 0 first, before typing in the correct amount. 2) Also: with the 0 in the field, you can still accidentally save 0 value credits and fines easily and the 'required' attribute on the field doesn't kick in. 3) The message is very unspecific. For example, when I enter 'agahgag' it will just ask me to 'fill out the field' Can we have the field empty and have a better error message? Maybe something like: 'The entered amount is not valid'? Created attachment 37538 [details] [review] Bug 2542: Replace default value "0" with "" (In reply to Katrin Fischer from comment #11) > Some thoughts on this: > > 1) I think displaying the 0 is not good for ergonomy. I can type 03.00, but > I feel libraries will think they need to delete the 0 first, before typing > in the correct amount. > > 2) Also: with the 0 in the field, you can still accidentally save 0 value > credits and fines easily and the 'required' attribute on the field doesn't > kick in. The default value is now "" > 3) The message is very unspecific. For example, when I enter 'agahgag' it > will just ask me to 'fill out the field' > > Can we have the field empty and have a better error message? Maybe something > like: 'The entered amount is not valid'? Sorry but I did not manage to do that. Created attachment 37555 [details] [review] [PASSED QA] Bug 2542: Validate the amount of a manual invoice/create If a manual invoice/credit amount is not correctly set, it should not be accepted. Test plan: Try to create a manual invoice and a manual credit with an amount containing something else than numbers. It should not be possible. Followed test plan. Patch behaves OK, including that negative values are not allowed. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> This is a good start to prevent empty fine lines and other mistakes - the style is a little off, but I think that's not a blocker. Created attachment 37556 [details] [review] [PASSED QA] Bug 2542: Replace default value "0" with "" Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Patch pushed to master. Thanks Jonathan! Pushed to 3.18.x will be in 3.18.6 |