[Bug 28168] New: Manual invoice form pre-fills Amount field with invalid number
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Bug ID: 28168 Summary: Manual invoice form pre-fills Amount field with invalid number Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: joonas.kylmala@helsinki.fi QA Contact: testopia@bugs.koha-community.org Depends on: 27410 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. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27410 [Bug 27410] Update intranet-tmpl/prog/en/modules/members/maninvoice.tt to reflect ACC2 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Peter Vashchuk <stalkernoid@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #1 from Peter Vashchuk <stalkernoid@gmail.com> --- Created attachment 119943 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119943&action=edit 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; -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |stalkernoid@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119943|0 |1 is obsolete| | --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 119945 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119945&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #3 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (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! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (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! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #6 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #7 from Andrew Nugged <nugged@gmail.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #8 from Andrew Nugged <nugged@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #9 from Andrew Nugged <nugged@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119945|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 120007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120007&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #11 from 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21507 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14825 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28168 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on 27410 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org