[Bug 33028] New: The decimal separator 'comma' and its consequences on the calculation of fines (crons)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Bug ID: 33028 Summary: The decimal separator 'comma' and its consequences on the calculation of fines (crons) Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Fines and fees Assignee: koha-bugs@lists.koha-community.org Reporter: thibaud.guillot@biblibre.com QA Contact: testopia@bugs.koha-community.org Hello @ll, I hesitate between 'fines and fee' section or 'cron' so don't move it if you think that's better in cron section.. When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I know there is a lot of discussion around this behavior and decimal separator by 'countries'.. like bug 12310 and bug 21507 but I would like to propose this patch to solve at least the behavior on the fine calculations related to cronjob 'fines.pl' which will use some circulation rules and therefore their formatting is important if you want to make calculations with -- 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=33028 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |thibaud.guillot@biblibre.co |ity.org |m -- 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=33028 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #1 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 147058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147058&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 147058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147058 Bug 33028: Fix calculations around cronjob fines.pl Review of attachment 147058: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33028&attachment=147058) ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/fix_calc_fines_fr-currency_21-11-MT39815.pl @@ +40,5 @@
+ $rule_id, $rule_value + ) + ); + } + }
We need some additional handling here for the .00/,00 case.. in particular 0.00/0,00 Perl will treat 0.00 as a string for truthyness and thus we need to reduce to just the integer 0 in this case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 32271 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 147058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147058 Bug 33028: Fix calculations around cronjob fines.pl Review of attachment 147058: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33028&attachment=147058) ----------------------------------------------------------------- ::: Koha/CirculationRules.pm @@ +392,5 @@
+ if ( $rule + && ( $rule->rule_name eq 'overduefinescap' || $rule->rule_name eq 'fine' ) ) + { + $rule_value = Koha::Number::Price->new($rule_value)->unformat; + }
I think we should generalise this and add it as a parameter in the hash above.. i.e. . . . overduefinescap => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], is_monetary => 1, }, . . . my $is_monetary = defined $kind_info->{is_monetary} ? $kind_info->{is_monetary} : 0; $rule_value = Koha::Number::Price->new($rule_value)->unformat if $is_monetary; . . . -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147058|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147686&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147687&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. 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=33028 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147688&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've added follow-ups for my comments.. signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #9 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- wow thanks Martin, that's great ^^ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've asked Jonathan D to review it for QA too as I think he's pretty close to how the circ rules stuff all fits together these days. Be nice to have this fixed. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
$rule_value !~ /,.*?,/
.*? What are you trying to do here? .* should be enough. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
'Many commas in rule id %s ("%s") - fix it before restart this update',
Do you mean "More than one commas"? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #13 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to Jonathan Druart from comment #11)
$rule_value !~ /,.*?,/
.*? What are you trying to do here? .* should be enough.
Hello Jonathan, Here I just want to avoid several commas and check if there are numbers after the comma and before another one.. that's useless maybe but I think that user must change values himself if there are not simply like xx,xx no ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #14 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to Jonathan Druart from comment #12)
'Many commas in rule id %s ("%s") - fix it before restart this update',
Do you mean "More than one commas"?
Yes, in circ_rules tables declaration it's only a varchar type if I remember, so user can have bad values and must fix it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #15 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- I take advantage of this patch to ask you a wider opinion on the management of the decimal separator, there is the BZ 21507 but I do not know in which direction to direct it to fix the problem once and for all Cause this problem (have a comma in rule values, occurs another problems around supplier update for example - when we want to change discount input with comma separator..) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I think this is wrong. PriceFormat=US, edit a rule with "360,000" (meaning 360000), and "360.00" is saved. In my opinion we should keep things simple to avoid bugs, and don't try anything else than warn the user that they are doing something wrong. I would: Run a dbrev to warn about incorrect values Add a JS alert when the user is trying to save a rule that is not %.2f formatted Raise an exception in Koha::Circulation->store in case incorrect values arrived there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think I'd also throw in a database update just replacing , with . If that's not enough, it won't work before or after... but we might fix a ton of problematic values. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #16)
I think this is wrong.
PriceFormat=US, edit a rule with "360,000" (meaning 360000), and "360.00" is saved.
In my opinion we should keep things simple to avoid bugs, and don't try anything else than warn the user that they are doing something wrong.
I would: Run a dbrev to warn about incorrect values Add a JS alert when the user is trying to save a rule that is not %.2f formatted Raise an exception in Koha::Circulation->store in case incorrect values arrived there.
So, are you saying the simple 'unformat' call isn't working as we would expect.. That's a shame :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wbeimar23@gmail.com --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 33359 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Circ rules are not enforcing currency formats either. This might fix things once, but does not keep the issue from coming back. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Kyle M Hall from comment #20)
Circ rules are not enforcing currency formats either. This might fix things once, but does not keep the issue from coming back.
Hi Kyle, what do you suggest? A format check on the inputs in the circ rules? We could use the pattern check we use in other forms. Or did you think of something else? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #21)
(In reply to Kyle M Hall from comment #20)
Circ rules are not enforcing currency formats either. This might fix things once, but does not keep the issue from coming back.
Hi Kyle, what do you suggest? A format check on the inputs in the circ rules? We could use the pattern check we use in other forms. Or did you think of something else?
Yes. Ideally we'd enforce the chosen format using javascript on the frontend, and in perl on the backend. Since the js should enforce it on the frontend, the backend could just die or throw an exception if it sees a violation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We had multiple reports of fines not working in 22.11 on the mailing lists and on bugzilla - I'd like to move this forward in some way since I am pretty sure that this one is to blame from the logs we saw complaining about invalid values. I am upping severity for now. This is definitely not an enhancement. The problem is, that when you edit an existing rule, you might create problematic values without noticing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dhanestraj@gmail.com --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 33240 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- To test: * Set CurrencyFormat to FR * Enable UseRecalls * Administration > circulation and fine rules * Add a new rule: * Fine amount: 0.10 * Fine charging interval: 1 * Overdue fines cap: 1.50 * Recall overdue fine amount: 1.10 * Save rule * Verify that both Overdue fines cap and recall overdue fine amount show formatted with comma as: 1,50, 1,10 * Edit rule * Fine amount: 0.10 - OK! * Fine charging interval: 1 * Overdue fines cap: 1,50 - NOT OK! * Recall overdue fine amount: 1,10 - NOT OK! * Save rule * With SQL: select * from circulation_rules where rule_name in ("fine","overduefinescap","recall_overdue_fine"); +----+------------+--------------+----------+---------------------+------------+ | id | branchcode | categorycode | itemtype | rule_name | rule_value | +----+------------+--------------+----------+---------------------+------------+ | 9 | NULL | NULL | NULL | fine | 0.10 | | 11 | NULL | NULL | NULL | overduefinescap | 1,50 | | 32 | NULL | NULL | NULL | recall_overdue_fine | 1,10 | +----+------------+--------------+----------+---------------------+------------+ So the problem is, that you will save false values unnoticed that will cause errors. You cannot tell wrong and right values apart in the table, because all values show formatted, if correct in the database or not. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|The decimal separator |Wrongly formatted monetary |'comma' and its |amounts in circulation |consequences on the |rules break scripts and |calculation of fines |calculations |(crons) | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am working on a few follow-ups here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147686|0 |1 is obsolete| | --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152441 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152441&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147687|0 |1 is obsolete| | --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152442&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147688|0 |1 is obsolete| | --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152443&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. 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=33028 --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152444&action=edit Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee This patch marks the 2 missing monetary values for recal over due fines and article request fees as monetary. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152445&action=edit Bug 33028: Add TT filters for Price and pattern checks to input fields With this patch, all monetary values in the table will be displayed formatted. Also, the input will be checked against our agreed pattern to make sure no false values can be entered. Missing: When editing a rule, we need to unformat the value, so that instead of the display format we have the input format available for editing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152446 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152446&action=edit Bug 33028: (follow-up) Rewrite database update This rewrite the database update with some things in mind: * We now use a positive value list of allowed characters to check This makes sure that all of those are recognized: 1,00 1.00€ abc * Instead of dying after finding one wrong value, we loop through all values first, building up an error string * When we have errors... we die and print the full list of things that need fixing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Updated test plan: * Set CurrencyFormat to FR * Enable UseRecalls * Enable ArticleRequests * Administration > circulation and fine rules * Add a new rule: * Fine amount: 0.10 * Fine charging interval: 1 * Overdue fines cap: 1.50 * Recall overdue fine amount: 1.10 * Save rule * Verify that both Overdue fines cap and recall overdue fine amount show formatted with comma as: 1,50, 1,10 * Edit rule * Fine amount: 0.10 - OK! * Fine charging interval: 1 * Overdue fines cap: 1,50 - NOT OK! * Recall overdue fine amount: 1,10 - NOT OK! * Save rule * With SQL: select * from circulation_rules where rule_name in ("fine","overduefinescap","recall_overdue_fine","article_request_fee"); +----+------------+--------------+----------+---------------------+------------+ | id | branchcode | categorycode | itemtype | rule_name | rule_value | +----+------------+--------------+----------+---------------------+------------+ | 9 | NULL | NULL | NULL | fine | 0.10 | | 11 | NULL | NULL | NULL | overduefinescap | 1,50 | | 32 | NULL | NULL | NULL | recall_overdue_fine | 1,10 | +----+------------+--------------+----------+---------------------+------------+ * Edit rule for Article request fee * Fee: 1,0 * Verify it's also wrong in the database with above SQL. --> This is already fixed by the patch set now, as we don't allow to edit the rules and the pattern check works. Missing bits and pieces: Have to: * When a rule is edited, the input field will be set to the display value (1,00). I didn't find an "unformat" equivalent for JS. This now triggers the pattern check, so you need to edit all values before being able to save them. We could use the inputmode="decimal" on the input fields to select for unformatting. * Should we be able to deactivate overduefinescap by leaving it empty? (Unlimited?) Would be nice: * It would be nice to provide more help in fixing wrong values or automatically fixing obvious ones like xx,xx. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 30672 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152441|0 |1 is obsolete| | --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152509&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152442|0 |1 is obsolete| | --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152510&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152443|0 |1 is obsolete| | --- Comment #37 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152511 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152511&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152444|0 |1 is obsolete| | --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152512 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152512&action=edit Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee This patch marks the 2 missing monetary values for recal over due fines and article request fees as monetary. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152445|0 |1 is obsolete| | --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152513 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152513&action=edit Bug 33028: Add TT filters for Price and pattern checks to input fields With this patch, all monetary values in the table will be displayed formatted. Also, the input will be checked against our agreed pattern to make sure no false values can be entered. Missing: When editing a rule, we need to unformat the value, so that instead of the display format we have the input format available for editing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152446|0 |1 is obsolete| | --- Comment #40 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152514 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152514&action=edit Bug 33028: (follow-up) Rewrite database update This rewrite the database update with some things in mind: * We now use a positive value list of allowed characters to check This makes sure that all of those are recognized: 1,00 1.00€ abc * Instead of dying after finding one wrong value, we loop through all values first, building up an error string * When we have errors... we die and print the full list of things that need fixing. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152515 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152515&action=edit Bug 33028: Remove Price formatting as we are missing JS unformat Taking a step back with the formatting as we don't have an implementation of unformat in JS yet. It could be readded later on a separate report. With this we will avoid the problem that editing a rule copies falsely formatted values into the input fields that trigger the pattern check on saving and require manual changing (or without the check would be falsely saved to the db) Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32271 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32271 [Bug 32271] Overdue fines cap (amount) set to 0.00 when editing rule -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #42 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Adjusted test plan: * Set CurrencyFormat to FR * Enable UseRecalls * Enable ArticleRequests * Administration > circulation and fine rules * Add a new rule: * Fine amount: 0.10 * Fine charging interval: 1 * Overdue fines cap: 1.50 * Recall overdue fine amount: 1.10 * Save rule * Verify that both Overdue fines cap and recall overdue fine amount show formatted with comma as: 1,50, 1,10 * Edit rule * Fine amount: 0.10 - OK! * Fine charging interval: 1 * Overdue fines cap: 1,50 - NOT OK! * Recall overdue fine amount: 1,10 - NOT OK! * Save rule * With SQL: select * from circulation_rules where rule_name in ("fine","overduefinescap","recall_overdue_fine","article_request_fee"); You should see the false values in the database: +----+------------+--------------+----------+---------------------+---------- --+ | id | branchcode | categorycode | itemtype | rule_name | rule_value | +----+------------+--------------+----------+---------------------+---------- --+ | 9 | NULL | NULL | NULL | fine | 0.10 | | 11 | NULL | NULL | NULL | overduefinescap | 1,50 | | 32 | NULL | NULL | NULL | recall_overdue_fine | 1,10 | +----+------------+--------------+----------+---------------------+---------- --+ * Edit rule for Article request fee * Fee: 1,0 * Verify it's also wrong in the database with above SQL. * Apply patch * Run database update * Verify your wrong values cause the update to stop * Fix them (one or all at once) * Verify the update now continues * Repeat editing and adding rules actions from before, a pattern check will prevent you now from saving false values. ------------------ Missing bits and pieces:
Have to: * When a rule is edited, the input field will be set to the display value (1,00). I didn't find an "unformat" equivalent for JS. This now triggers the pattern check, so you need to edit all values before being able to save them. We could use the inputmode="decimal" on the input fields to select for unformatting.
I have decided that this could be a separate bug. The JS used for editing/cloning the rules is quite complex and it's easier to make a step back first to get this moving and into old versions. --> I have removed the Price formatting from the circulation rules table in the last follow-up.
* Edit rule for Article request fee * Fee: 1,0 * Verify it's also wrong in the database with above SQL. --> This is already fixed by the patch set now, as we don't allow to edit the rules and the pattern check works.
Missing bits and pieces:
Have to: * When a rule is edited, the input field will be set to the display value (1,00). I didn't find an "unformat" equivalent for JS. This now triggers the pattern check, so you need to edit all values before being able to save them. We could use the inputmode="decimal" on the input fields to select for unformatting.
I have now fixed this by removing the Price formatting from all fields. I think this can be filed separately but is the quickest fix now that can also be backported.
* Should we be able to deactivate overduefinescap by leaving it empty? (Unlimited?)
Yes, definitely. But there is already a separate bug (and a duplicate) for that: Bug 32271 - Overdue fines cap (amount) set to 0.00 when editing rule So I decided to split this issue out in hope to be able to move this forward and get help with the other issue.
Would be nice: * It would be nice to provide more help in fixing wrong values or automatically fixing obvious ones like xx,xx.
I think we don't need that to move on here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152752 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152752&action=edit Bug 33028: (follow-up) Add unformat_price js function -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152753 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152753&action=edit Revert "Bug 33028: Remove Price formatting as we are missing JS unformat" This reverts commit c10aa96e4f1fac0f34e02caade3aa259d7233c71. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I ran out of tuits here I'm afraid.. passing back to Katrin.. I created an 'unformat_price' function for String and exported it for you.. I'll come back and try to apply it to the right editing fields here when I can.. Adding the patches as is for now in case you have a moment before me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #46 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152754 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152754&action=edit Bug 33028: (follow-up) Apply unformat_price to decimal fields -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK.. I found the 5 minutes! Please test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152509|0 |1 is obsolete| | --- Comment #48 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152755 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152755&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152510|0 |1 is obsolete| | --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152756&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152511|0 |1 is obsolete| | --- Comment #50 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152757&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152512|0 |1 is obsolete| | --- Comment #51 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152758&action=edit Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee This patch marks the 2 missing monetary values for recal over due fines and article request fees as monetary. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152513|0 |1 is obsolete| | --- Comment #52 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152759&action=edit Bug 33028: Add TT filters for Price and pattern checks to input fields With this patch, all monetary values in the table will be displayed formatted. Also, the input will be checked against our agreed pattern to make sure no false values can be entered. Missing: When editing a rule, we need to unformat the value, so that instead of the display format we have the input format available for editing. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152514|0 |1 is obsolete| | --- Comment #53 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152760 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152760&action=edit Bug 33028: (follow-up) Rewrite database update This rewrite the database update with some things in mind: * We now use a positive value list of allowed characters to check This makes sure that all of those are recognized: 1,00 1.00€ abc * Instead of dying after finding one wrong value, we loop through all values first, building up an error string * When we have errors... we die and print the full list of things that need fixing. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152515|0 |1 is obsolete| | --- Comment #54 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152761&action=edit Bug 33028: Remove Price formatting as we are missing JS unformat Taking a step back with the formatting as we don't have an implementation of unformat in JS yet. It could be readded later on a separate report. With this we will avoid the problem that editing a rule copies falsely formatted values into the input fields that trigger the pattern check on saving and require manual changing (or without the check would be falsely saved to the db) Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152752|0 |1 is obsolete| | --- Comment #55 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152762&action=edit Bug 33028: (follow-up) Add unformat_price js function 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152753|0 |1 is obsolete| | --- Comment #56 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152763&action=edit Revert "Bug 33028: Remove Price formatting as we are missing JS unformat" This reverts commit c10aa96e4f1fac0f34e02caade3aa259d7233c71. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152754|0 |1 is obsolete| | --- Comment #57 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152764&action=edit Bug 33028: (follow-up) Apply unformat_price to decimal fields 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=33028 --- Comment #58 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Added my signoff line to all patches.. I'm pretty happy with is solid now.. but given all the challenges we've had with it I'd love a final QA run from an independance party. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Needs Signoff |Failed QA --- Comment #59 from Nick Clemens <nick@bywatersolutions.com> --- Editing a rule with 1.50 shows 1.5 - and tells me to 'please match the requested format' - but doesn't say what that format is Entering 1.50 and saving, i see: 150,00 in the matrix and 150 in the DB -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152762|0 |1 is obsolete| | --- Comment #60 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152803 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152803&action=edit Bug 33028: (follow-up) Add unformat_price js function 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152763|0 |1 is obsolete| | --- Comment #61 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152804&action=edit Revert "Bug 33028: Remove Price formatting as we are missing JS unformat" This reverts commit c10aa96e4f1fac0f34e02caade3aa259d7233c71. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152764|0 |1 is obsolete| | --- Comment #62 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152805&action=edit Bug 33028: (follow-up) Apply unformat_price to decimal fields 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=33028 --- Comment #63 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I resolved this: Editing a rule with 1.50 shows 1.5 But I can't replicate this: Entering 1.50 and saving, i see: 150,00 in the matrix and 150 in the DB -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #64 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK.. I should have read Katrins patches in more depth before continuing here.. Bug 33028: Add TT filters for Price and pattern checks to input fields This actually takes us backwards and breaks input more.. We are expecting currency format to be correctly passed to us in system currency format.. NOT always US decimal.. as such by adding the pattern and setting the input types to decimal only we are forcing US decimal input which means the unformat call on the serverside is breaking the inputs. I think we should go all the way back to `Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee` and we had a working system at that point following Katrins test plan.. at least it was working for me for all cases at that point. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #65 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152829&action=edit Bug 33028: (follow-up) Remove inputmode and pattern -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #66 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK... with that last patch I think we should be working correctly in all cases.. Please follow the test plan in comment 25 to confirm this. We can drop/squash half the patches here.. the bug has become a real mess with things getting added then reverted, then added and finally reverted again. I'm not sure we need the js unformat function I created at all at this point.. it's no longer used here.. but might be helpful migrated to another bug in case anyone needs it elsewhere. I'm also not sure about the location of the unformat call deep down in Circulation rules.. it feels like we should perhaps be calling this higher up in the controller rather than the module expecting different input types based on configuration. Anyway.. I'd love to see this move forward. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #67 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I feel like we moved into contrary directions: I tried to restore the former behaviour. Koha expects us to enter monetary amounts with decimal dot everywhere, so I was trying to get back to this here. I'd like to see input formats fixed, but I feel that a bugfix that needs to go back into stables is the wrong place. I used the same pattern that we agreed on and use in patron accounting and removed the Price TT filters to make sure we get back to decimal dot everywhere, giving up the display in favor of having the input fixed. I believe Martin tried to allow entering with decimal comma with his patches, which appears more risky to me. Also: how to communicate that this page expects different input than anywhere else? I might be mistaken - I need to take a closer look for sure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #68 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, well that may be the case.. but you'll need to also undo the first patch here as that make it expect bon decimal dot -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152755|0 |1 is obsolete| | --- Comment #69 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152928&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152756|0 |1 is obsolete| | --- Comment #70 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152929&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152757|0 |1 is obsolete| | --- Comment #71 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152930&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152758|0 |1 is obsolete| | --- Comment #72 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152931&action=edit Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee This patch marks the 2 missing monetary values for recal over due fines and article request fees as monetary. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152759|0 |1 is obsolete| | --- Comment #73 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152932&action=edit Bug 33028: Add TT filters for Price and pattern checks to input fields With this patch, all monetary values in the table will be displayed formatted. Also, the input will be checked against our agreed pattern to make sure no false values can be entered. Missing: When editing a rule, we need to unformat the value, so that instead of the display format we have the input format available for editing. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152760|0 |1 is obsolete| | --- Comment #74 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152933&action=edit Bug 33028: (follow-up) Rewrite database update This rewrite the database update with some things in mind: * We now use a positive value list of allowed characters to check This makes sure that all of those are recognized: 1,00 1.00€ abc * Instead of dying after finding one wrong value, we loop through all values first, building up an error string * When we have errors... we die and print the full list of things that need fixing. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152803|0 |1 is obsolete| | --- Comment #75 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152934 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152934&action=edit Bug 33028: (follow-up) Add unformat_price js function 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152805|0 |1 is obsolete| | --- Comment #76 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152935&action=edit Bug 33028: (follow-up) Apply unformat_price to decimal fields 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=33028 --- Comment #77 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 152936 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152936&action=edit Bug 33028: Throw exception if not passed a decimal number -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152761|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152804|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152829|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #78 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK.. I believe this now works as Katrin intended as a whole... * Displays are in localised format * Inputs are in US Decimal Dot with decimal input and a pattern match I prefer the patchset prior to this one, where both display and inputs followed currency format (though I do prefer the exception this raises if passed a bad format and would like to have moved the unformat into the controller level instead of where it was) This is, however, more consistent with the rest of the UI where we appear to expect non-localised decimal inputs regardless of how we then display them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34231 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #79 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Testing here again... back and forth we go! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152928|0 |1 is obsolete| | --- Comment #80 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153313 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153313&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) 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=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153313|0 |1 is obsolete| | --- Comment #81 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153314 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153314&action=edit Bug 33028: Fix calculations around cronjob fines.pl When currency format is set on FR commas are decimals separators but when cron like fines.pl try to calculate fines it's fails due to this format. I changed this behavior by targetted 'fine' and 'overduefinescap' in circulation_rules.rule_name to unformat them when we save them. This also fix the display in smart_rules table (before with commas price was not good displayed - without decimals) Test Plan : 1) Set your currency format on 'FR' and 'fine' OR/AND 'overduefinescap' with commas 2) Be sure to have some patron overdues 3) Run ~/misc/cronjobs/fines.pl with args to find overdues 4) See an error like 'isn't numeric in substraction[..] or gt > [...]' 5) Run updatedatabase script (it will replace commas in your rules changed in step 1) ) 6) Repeat step 3 and see that everything was going "fine" (:tada:) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152929|0 |1 is obsolete| | --- Comment #82 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153315 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153315&action=edit Bug 33028: (follow-up) Move monetary definition into hash This patch moves the defintion of monetary rule type into the rule kinds hash. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152930|0 |1 is obsolete| | --- Comment #83 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153316 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153316&action=edit Bug 33028: (follow-up) Fix trailing 0 decimals We want to recognise the truthyness of a number vs string so we drop trailing decimals if they're just 0. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152931|0 |1 is obsolete| | --- Comment #84 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153317 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153317&action=edit Bug 33028: Add is_monetary to recall_overdue_fine and article_request_fee This patch marks the 2 missing monetary values for recal over due fines and article request fees as monetary. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152932|0 |1 is obsolete| | --- Comment #85 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153318&action=edit Bug 33028: Add TT filters for Price and pattern checks to input fields With this patch, all monetary values in the table will be displayed formatted. Also, the input will be checked against our agreed pattern to make sure no false values can be entered. Missing: When editing a rule, we need to unformat the value, so that instead of the display format we have the input format available for editing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152933|0 |1 is obsolete| | --- Comment #86 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153319 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153319&action=edit Bug 33028: (follow-up) Rewrite database update This rewrite the database update with some things in mind: * We now use a positive value list of allowed characters to check This makes sure that all of those are recognized: 1,00 1.00€ abc * Instead of dying after finding one wrong value, we loop through all values first, building up an error string * When we have errors... we die and print the full list of things that need fixing. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152934|0 |1 is obsolete| | --- Comment #87 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153320 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153320&action=edit Bug 33028: (follow-up) Add unformat_price js function Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152935|0 |1 is obsolete| | --- Comment #88 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153321 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153321&action=edit Bug 33028: (follow-up) Apply unformat_price to decimal fields Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152936|0 |1 is obsolete| | --- Comment #89 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153322 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153322&action=edit Bug 33028: Throw exception if not passed a decimal number Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #90 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 153323 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153323&action=edit Bug 33028: Perltidy database update script Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #91 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #79)
Testing here again... back and forth we go!
Heh, tested as well, Katrin beat me by about 5 minutes, but adding my support too :-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #92 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Nick Clemens from comment #91)
(In reply to Katrin Fischer from comment #79)
Testing here again... back and forth we go!
Heh, tested as well, Katrin beat me by about 5 minutes, but adding my support too :-)
I think another set of eyes definitely won't hurt for sure! I'd love to see a fix for bug 32271 as next step, so we hopefully get in a state again where editing rules doesn't create unwanted side effects. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #93 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 153619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153619&action=edit Bug 33028: (QA follow-up) Tidy introduced code Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #94 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 153620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153620&action=edit Bug 33028: Make exception less generic While testing this bug I found Circulation.t was failing, but the exception doesn't actually display anything useful in terms of helping debug what's going on. This patch makes it add the rule_name and rule_value to the message. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |tomascohen@gmail.com --- Comment #95 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This is almost done, and already on my branch. But... prove t/db_dependent/Koha/CirculationRules.t t/db_dependent/Koha/CirculationRules.t .. 1/7 # Failed test 'setting fine with branch/category/itemtype succeeds' # at t/db_dependent/Koha/CirculationRules.t line 211. # died: Koha::Exceptions::CirculationRule::NotDecimal (Exception 'Koha::Exceptions::CirculationRule::NotDecimal' thrown 'The circulation rule expected a decimal value' with name => fine, value => # ) # Looks like you failed 1 test of 5. Please fix ASAP so I can push tomorrow morning. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #96 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Tomás Cohen Arazi from comment #95)
This is almost done, and already on my branch. But...
prove t/db_dependent/Koha/CirculationRules.t t/db_dependent/Koha/CirculationRules.t .. 1/7 # Failed test 'setting fine with branch/category/itemtype succeeds' # at t/db_dependent/Koha/CirculationRules.t line 211. # died: Koha::Exceptions::CirculationRule::NotDecimal (Exception 'Koha::Exceptions::CirculationRule::NotDecimal' thrown 'The circulation rule expected a decimal value' with name => fine, value => # ) # Looks like you failed 1 test of 5.
Please fix ASAP so I can push tomorrow morning.
Martin, do you have a moment to have a look? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #97 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153632 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153632&action=edit Bug 33028: Unit tests Add unit tests for is_monetary functionality introduced in the CirculationRules module. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153632|0 |1 is obsolete| | --- Comment #98 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153633&action=edit Bug 33028: Unit tests Add unit tests for is_monetary functionality introduced in the CirculationRules module. 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=33028 --- Comment #99 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153634&action=edit Bug 33028: (follow-up) Add POD to new Exception 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #100 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Short of tidying the whole CirculationRule module I couldn't get the perltidy QA complaint to go away (I'd love to tidy the whole file.. there's isn't all that many changes to be honest so I don't think it would really hurt for future conflicts much) Unit tests corrected and additional one's added. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153633|0 |1 is obsolete| | --- Comment #101 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153635&action=edit Bug 33028: Unit tests Add unit tests for is_monetary functionality introduced in the CirculationRules module. 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153634|0 |1 is obsolete| | --- Comment #102 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 153636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153636&action=edit Bug 33028: (follow-up) Add POD to new Exception 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=33028 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #103 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #104 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 153677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153677&action=edit Bug 33028: (follow-up) Lower the two digits requirement This patch makes the code not require two decimal digits, as the main intention here is to forbid (locale) formatted strings to reach the DB. The number of digits we support needs to be discussed on its own bug, and a centralized check implemented. This patch fixes tests: prove t/db_dependent/Circulation.t t/db_dependent/Circulation.t .. 1/67 Exception 'Koha::Exceptions::CirculationRule::NotDecimal' thrown 'The circulation rule expected a decimal value' with name => fine, value => 0.1 t/db_dependent/Circulation.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 53/67 subtests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34279 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #105 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This requires backporting down to 22.11 please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable Version(s)|23.11.00 |23.11.00,23.05.03 released in| | --- Comment #106 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.03 |23.11.00,23.05.03,22.11.09 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #107 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Nice work everyone! Pushed to 22.11.x for next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 --- Comment #108 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thank you, everyone and Pedro :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #109 from Lucas Gass <lucas@bywatersolutions.com> --- If this is needed in 22.05.x the patchset will need a rebase for that version. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33028 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35087 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org