Bug 13352 - Editing amount in Acquisitions budget causes error due to formatting
Summary: Editing amount in Acquisitions budget causes error due to formatting
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 13616 (view as bug list)
Depends on: 12979
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-26 21:53 UTC by Christopher Brannon
Modified: 2015-12-03 22:02 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 13352: On editing, prices should not be formatted (8.95 KB, patch)
2015-01-26 09:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 13352: On editing, prices should not be formatted (9.22 KB, patch)
2015-02-06 14:33 UTC, Paola Rossi
Details | Diff | Splinter Review
[PASSED QA] Bug 13352: On editing, prices should not be formatted (9.07 KB, patch)
2015-02-08 15:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)" (1.11 KB, patch)
2015-02-08 15:26 UTC, Katrin Fischer
Details | Diff | Splinter Review
[3.18.x] Bug 13352: On editing, prices should not be formatted (7.98 KB, patch)
2015-02-13 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
[3.18.x] Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)" (1.11 KB, patch)
2015-02-13 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2014-11-26 21:53:05 UTC
If you edit a fund (aqbudgets.pl --> Edit --> Edit budget xxxx), the value is formatted with a comma.  If you click save changes without changing anything, you get an error due to the formatting.  You have to remove the comma.

Either the value on the edit screen should not include the comma here, or better yet, it should accept the value with or without the comma.

Koha should also be checked to see if this happens anywhere else.

Christopher
Comment 1 Christopher Brannon 2014-11-26 22:05:48 UTC
(In reply to Christopher Brannon from comment #0)
> If you edit a fund (aqbudgets.pl --> Edit --> Edit budget xxxx), the value
> is formatted with a comma.  If you click save changes without changing
> anything, you get an error due to the formatting.  You have to remove the
> comma.

Whoops.  That should read:

If you edit a budget (aqbudgets.pl --> Edit --> Edit budget xxxx), the value
is formatted with a comma.  If you click save changes without changing
anything, you get an error due to the formatting.  You have to remove the
comma.
Comment 2 Katrin Fischer 2015-01-22 23:19:33 UTC
*** Bug 13616 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Druart 2015-01-26 09:36:49 UTC Comment hidden (obsolete)
Comment 4 Paola Rossi 2015-02-06 14:33:44 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2015-02-08 15:21:44 UTC
Comment on attachment 35703 [details] [review]
[SIGNED OFF] Bug 13352: On editing, prices should not be formatted

Review of attachment 35703 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
@@ +462,5 @@
>      </li>
>  
>      <li>
>        <label for="budget_encumb">Warning at (%): </label>
> +      <input type="text" name="budget_encumb" id="budget_encumb" value="[% budget_encumb | $Price on_editing => 1 %]" size="10" />

I think it's not needed here - this is a percentage, not a price.
Comment 6 Katrin Fischer 2015-02-08 15:26:21 UTC
Created attachment 35724 [details] [review]
[PASSED QA] Bug 13352: On editing, prices should not be formatted

Bug 12979 refactored the way to display prices.
The price format configuration was duplicated everywhere it was used.

All calls looks good except the one in admin/aqbudgetperiods.pl
In this one, the prices are formatted for an edition field (input).
This means the input is incorrectly filled even if the user does not
update the field.

At the end, maybe should we manage formatted prices everywhere, even in
inputs, but it's not in the scope of this bug.

Technically, a new subroutine format_for_editing is added to the
Koha::Number::Price module.
It should be called everywhere a price is displayed in an input field.
At the moment, it only does a sprintf("%.2f"), but it is a first step to
let the number of decimals to display configurable.

To test:
1/ Verify the issue described is fixed (editing a budget with a total
amount > 1000 (With CurrencyFormat is US or FR).
2/ Verify you can edit a fund with a total amount > 1000

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 7 Katrin Fischer 2015-02-08 15:26:45 UTC
Created attachment 35725 [details] [review]
Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)"
Comment 8 Jonathan Druart 2015-02-09 13:51:59 UTC
(In reply to Katrin Fischer from comment #7)
> Created attachment 35725 [details] [review] [review]
> Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)"

Looks consistent, thanks for the follow-up!
Comment 9 Tomás Cohen Arazi 2015-02-09 16:49:03 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 10 Chris Cormack 2015-02-12 22:11:14 UTC
Pushed to 3.18.x will be 3.18.4
Comment 11 Chris Cormack 2015-02-13 01:20:55 UTC
Doesn't apply cleanly on 3.18.x
Comment 12 Jonathan Druart 2015-02-13 08:22:12 UTC
(In reply to Chris Cormack from comment #11)
> Doesn't apply cleanly on 3.18.x

I didn't try to reproduce on 3.18.x, but since bug 12979 is not in it, it should not be needed.
Comment 13 Katrin Fischer 2015-02-13 08:28:54 UTC
Hi, I just tested on 3.18.3 - editing funds is ok, but the problem occurs for the budget.
Comment 14 Jonathan Druart 2015-02-13 08:57:43 UTC
Created attachment 35894 [details] [review]
[3.18.x] Bug 13352: On editing, prices should not be formatted

Bug 12979 refactored the way to display prices.
The price format configuration was duplicated everywhere it was used.

All calls looks good except the one in admin/aqbudgetperiods.pl
In this one, the prices are formatted for an edition field (input).
This means the input is incorrectly filled even if the user does not
update the field.

At the end, maybe should we manage formatted prices everywhere, even in
inputs, but it's not in the scope of this bug.

Technically, a new subroutine format_for_editing is added to the
Koha::Number::Price module.
It should be called everywhere a price is displayed in an input field.
At the moment, it only does a sprintf("%.2f"), but it is a first step to
let the number of decimals to display configurable.

To test:
1/ Verify the issue described is fixed (editing a budget with a total
amount > 1000 (With CurrencyFormat is US or FR).
2/ Verify you can edit a fund with a total amount > 1000

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 15 Jonathan Druart 2015-02-13 08:57:56 UTC
Created attachment 35895 [details] [review]
[3.18.x] Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)"
Comment 16 Jonathan Druart 2015-02-13 08:58:58 UTC
(In reply to Katrin Fischer from comment #13)
> Hi, I just tested on 3.18.3 - editing funds is ok, but the problem occurs
> for the budget.

Yes, you are right, the patches are needed in 3.18.x (because of bug 12844).
Comment 17 Chris Cormack 2015-02-15 05:53:32 UTC
Pushed to 3.18.x will be in 3.18.4 thanks for the 3.18.x patches Jonathan