The Amount field on Manual invoice form (/cgi-bin/koha/members/maninvoice.pl?borrowernumber=XXXX) used to be empty by default before bug 27410. Now we get a "0.00" or "0,00" value there. This adds one more click to the workflow but more importantly because the form only accepts X.XX (decimal separator being ".") values the pre-filled number is incorrectly in the wrong format if using a comma and it confuses the user because they might think it is okay to enter a comma there.
Created attachment 119943 [details] [review] Bug 28168: leave the field empty if amount variable is empty Add "if" check to the template so that it uses the value of "debit_type.default_amount" only if it's true, i.e. not null or 0. The reason for this patch is there's requirement from librarians - to have this field completely empty if it's 0, so they could paste the amount (as they usually do) without the need to clear the field first. And anyway if you try to save the form with 0.00 value, it won't accept it saying "Debit amount passed is not positive" so in my opinion there's no point to preset it with zeroes to begin with. To reproduce: 1) head to /cgi-bin/koha/members/maninvoice.pl?borrowernumber=XXXX and check that field "Amount" is prefilled with 0.00; 2) apply patch; 3) refresh page and ensure that field "Amount" is empty now; 4) go to /cgi-bin/koha/admin/debit_types.pl and change default amount to some decimal amount; 5) refresh manual invoice page again and ensure that "Amount" field is prefilled with that exact decimal number;
Created attachment 119945 [details] [review] Bug 28168: leave the field empty if amount variable is empty Add "if" check to the template so that it uses the value of "debit_type.default_amount" only if it's true, i.e. not null or 0. The reason for this patch is there's requirement from librarians - to have this field completely empty if it's 0, so they could paste the amount (as they usually do) without the need to clear the field first. And anyway if you try to save the form with 0.00 value, it won't accept it saying "Debit amount passed is not positive" so in my opinion there's no point to preset it with zeroes to begin with. To reproduce: 1) head to /cgi-bin/koha/members/maninvoice.pl?borrowernumber=XXXX and check that field "Amount" is prefilled with 0.00; 2) apply patch; 3) refresh page and ensure that field "Amount" is empty now; 4) go to /cgi-bin/koha/admin/debit_types.pl and change default amount to some decimal amount; 5) refresh manual invoice page again and ensure that "Amount" field is prefilled with that exact decimal number; Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Joonas Kylmälä from comment #0) > The Amount field on Manual invoice form > (/cgi-bin/koha/members/maninvoice.pl?borrowernumber=XXXX) used to be empty > by default before bug 27410. Now we get a "0.00" or "0,00" value there. This > adds one more click to the workflow but more importantly because the form > only accepts X.XX (decimal separator being ".") values the pre-filled number > is incorrectly in the wrong format if using a comma and it confuses the user > because they might think it is okay to enter a comma there. After doing more investigation I want to clarify that the default value used to be empty only with CurrencyFormat = "FR". After this patch the default value will be either empty or 0.00 depending whether the user has set the default fee in /cgi-bin/koha/admin/debit_types.pl to be empty/NULL or 0.00. So I think the proposed solution from Petro here is great because the old behaviour in US currency format can be changed via debit_types.pl page. Now with this patch both US and FR format users will be happy! :)
Right.. this is an improvement.. but i think we're still missing the localisation side of it... I'm not sure if we aught to handle that on it's own bug as I know it's been contentious in the past.
(In reply to Martin Renvoize from comment #4) > Right.. this is an improvement.. but i think we're still missing the > localisation side of it... I'm not sure if we aught to handle that on it's > own bug as I know it's been contentious in the past. Problem was, apart from other things, the thousands separator. That's why we had concluded to only allow 100,00.00 for now I think to keep things consistent all over Koha for now. I haven't tested this, just trying to summarize where discussion about this stopped to my knowledge!
(In reply to Martin Renvoize from comment #4) > Right.. this is an improvement.. but i think we're still missing the > localisation side of it... I'm not sure if we aught to handle that on it's > own bug as I know it's been contentious in the past. It can be handled as separate bug as this just fixes the regression introduced by bug 27410 (whether the field is empty or not). Actually I'm unsure if there is already a bug for the localization error because Petro said something about filing a bug report for that.
2 Martin / all: Peter exactly started with second part of this ticket (actually we thought to create another next one ticket): localization We today with him found that amount value in POST just added and treated as "Perl number", so in our case with comma record just NOT added (we still testing and researching). Let's give a day more for Peter (Petro) to bring more research and seems solution: localization of result. Please give any comments by the way which will help us to move efficiently and not take a wrong turn
But this not related to current solution in this ticket, it will be another ticket from Petro or me. This ticket is better to be go through QA if possible.
P.S. We need this field to be empty if there's zeroes for usability people should be able to paste values easier, without precleaning fields, Also this won't affect our next ticket about localization. Moreover: system won't allow 0,00 nor 0.00 to be saved later in POST server-side validation.
Created attachment 120007 [details] [review] Bug 28168: leave the field empty if amount variable is empty Add "if" check to the template so that it uses the value of "debit_type.default_amount" only if it's true, i.e. not null or 0. The reason for this patch is there's requirement from librarians - to have this field completely empty if it's 0, so they could paste the amount (as they usually do) without the need to clear the field first. And anyway if you try to save the form with 0.00 value, it won't accept it saying "Debit amount passed is not positive" so in my opinion there's no point to preset it with zeroes to begin with. To reproduce: 1) head to /cgi-bin/koha/members/maninvoice.pl?borrowernumber=XXXX and check that field "Amount" is prefilled with 0.00; 2) apply patch; 3) refresh page and ensure that field "Amount" is empty now; 4) go to /cgi-bin/koha/admin/debit_types.pl and change default amount to some decimal amount; 5) refresh manual invoice page again and ensure that "Amount" field is prefilled with that exact decimal number; Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
OK, Agreed.. lets handle localization on another ticket (there are already a bunch of them open regarding decimal handling, comma's vs dots etc) With that in mind, I'm going to PQA on this one.. it resolve the issue and is a clean solution.
Bug 21507 is one of the long-running bugs around decimal separators. I'd love fresh eyes on that.. or even an entirely new approach.. it was opened after another attempt failed before it...
Pushed to master for 21.05, thanks to everybody involved!
Depends on 27410 not in 20.11.x