|
Description
Christopher Brannon
2014-11-26 21:53:05 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. *** Bug 13616 has been marked as a duplicate of this bug. *** Created attachment 35517 [details] [review] 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 Created attachment 35703 [details] [review] [SIGNED OFF] Bug 13352: On editing, prices should not be formatted I've applied the patch against master 3.19.00.008 head 7904. Everything's OK as required by the test plan, with either FR or US CurrencyFormat syspref. So I pass the patch to the "Signed Off" status. NB1. The error message to fix is the following [blocking] one: "Form not submitted because of the following problem(s) - Amount must be a valid number, or empty" Editing a budgetperiod, the value 'hhh' in the budget's "Total-amount" input field gets the same error message [IMO RIGHT]. (A) Editing a fund, the 'hhh' in the fund's "Amount" input field doesn't get the message error, and sets the fund's amount to 0 [IMO WRONG]. This (A) error is the same against master, so IMO it's not related to this bug 13352. NB2. Editing a budgetperiod, "Save" brings back to "Budget Administration" [i.e. the budgets' list]. Editing a budgetperiod X, "Cancel" doesn't bring back to "Budget Administration": it brings to the "Funds for X". Is it OK? NB3. On editing the budget/fund, with the "FR" CurrencyFormat syspref the decimal mark was the dot, like with "US". The comma could be the decimal mark with "FR". 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. 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> Created attachment 35725 [details] [review] Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)" (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! Patches pushed to master. Thanks Jonathan! Pushed to 3.18.x will be 3.18.4 Doesn't apply cleanly on 3.18.x (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. Hi, I just tested on 3.18.3 - editing funds is ok, but the problem occurs for the budget. 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> Created attachment 35895 [details] [review] [3.18.x] Bug 13352: QA Follow-up: Remove Price filter from "Warning at (%)" (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). Pushed to 3.18.x will be in 3.18.4 thanks for the 3.18.x patches Jonathan |