Bug 2542 - Manual fine/credit fail on superfluous characters ( e.g. '$' )
Summary: Manual fine/credit fail on superfluous characters ( e.g. '$' )
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: PC All
: P3 normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-21 12:55 UTC by Chris Cormack
Modified: 2015-12-03 22:03 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 2542: Validate the amount of a manual invoice (1.70 KB, patch)
2015-03-18 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2542: Validate the amount of a manual invoice (1.71 KB, patch)
2015-03-18 15:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2542: Validate the amount of a manual invoice/create (2.94 KB, patch)
2015-03-23 16:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 2542: Validate the amount of a manual invoice/create (3.18 KB, patch)
2015-03-24 06:22 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 2542: Does not empty the amount field when the fine type is changed (1.18 KB, patch)
2015-03-25 08:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 2542: Replace default value "0" with "" (2.81 KB, patch)
2015-04-07 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 2542: Validate the amount of a manual invoice/create (3.23 KB, patch)
2015-04-07 20:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 2542: Replace default value "0" with "" (2.88 KB, patch)
2015-04-07 20:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 00:52:23 UTC


---- Reported by rch@liblime.com 2008-08-21 12:55:48 ----

creating a manual invoice fails if you add a currency mark.



---- Additional Comments From wizzyrea@gmail.com 2009-06-08 19:35:54 ----

I added a note next to the field to remind people that they do not need (and should not, per this bug) to insert currency marks. Pushed that shortly after KohaCon09.

Doesn't fix the problem, but does help keep it from being especially bothersome.



---- Additional Comments From nengard@gmail.com 2010-02-07 14:01:05 ----

This is still an issue.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:52 UTC  ---

This bug was previously known as _bug_ 2542 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2542

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was rch@liblime.com.

Comment 1 Katrin Fischer 2012-04-02 19:15:12 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).
Comment 2 Jonathan Druart 2015-03-18 15:51:58 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-03-18 15:55:13 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2015-03-20 14:18:46 UTC
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?
Comment 5 Jonathan Druart 2015-03-23 16:09:18 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-03-23 16:09:55 UTC
(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.
Comment 7 Marc Véron 2015-03-24 06:22:56 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2015-03-24 21:56:20 UTC
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.
Comment 9 Katrin Fischer 2015-03-24 21:56:54 UTC
Was actually watching a coworker running into this problem today... will be good to see it fixed :)
Comment 10 Jonathan Druart 2015-03-25 08:07:19 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2015-04-05 22:07:08 UTC
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'?
Comment 12 Jonathan Druart 2015-04-07 09:25:40 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2015-04-07 09:26:33 UTC
(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.
Comment 14 Katrin Fischer 2015-04-07 20:24:58 UTC
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>
Comment 15 Katrin Fischer 2015-04-07 20:26:03 UTC
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.
Comment 16 Katrin Fischer 2015-04-07 20:26:11 UTC
Created attachment 37556 [details] [review]
[PASSED QA] Bug 2542: Replace default value "0" with ""

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Tomás Cohen Arazi 2015-04-08 18:00:17 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 18 Chris Cormack 2015-04-12 07:57:29 UTC
Pushed to 3.18.x will be in 3.18.6