Attachment 75954 Details for Bug 12310 – Bug 12310: Decimal separator issues: fixes in fines

[patch] Bug 12310: Decimal separator issues: fixes in fines

Bug-12310-Decimal-separator-issues-fixes-in-fines.patch (text/plain), 15.69 KB, created by Victor Grousset/tuxayo on 2018-06-11 14:49:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Grousset/tuxayo

Created: 2018-06-11 14:49:45 UTC

Size: 15.69 KB

patch

obsolete

>From 4db2944ee35e9c60b9efa522a8a0df2477f21533 Mon Sep 17 00:00:00 2001
>From: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
>Date: Sun, 1 Apr 2018 23:11:25 +0200
>Subject: [PATCH] Bug 12310: Decimal separator issues: fixes in fines
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>The main goal is to prevent
>- x100 or /100 of the amounts
>- decimal part truncation
>
>Fixes Bug 19521 - Partial fine payment is not working with comma decimal separator
>
>Bonuses:
>- Gets rid of the spaces, currency symbols, names (EUR)
>  There is nothing specific about currency symbols is this patch.
>  It just removes everything that is not a digit a minus or a separator.
>
>It handles:
>- by default comma and dot as decimal separators on the same instance
>- dot or comma as thousands separators if choose to use only one of them
>    as decimal separator. See DecimalSeparators syspref.
>- minuses, doesn't remove them
>- non-decimal values: it doesn't touch them
>
>== Test plan ==
>1. Use a translation of the staff interface if possible.
>2. When you pay an existing fine (more details in the next steps if needed).
>     Check that you can't enter any string except like "XX.XX" (examples in
>     Annex 1) without having an issue after validation. (x100,
>     truncation, etc)
>3. Apply this patch
>4. run ~/src/installer/data/mysql/updatedatabase.pl
>5. In the manual invoice (cgi-bin/koha/members/maninvoice.pl)
>6. Apply the annex 1 to the "Amount" field
>7. When paying a fine (cgi-bin/koha/members/paycollect.pl)
>8. Click on "Pay amount" to go to the page to pay all fines
>9. Apply the annex 1 to the "Collect from patron" field
>10. Click on "Pay" to go to the page to pay fine
>11. Apply the annex 1 to the "Collect from patron" field
>12. Go to "Create manual credit" (staff://cgi-bin/koha/members/mancredit.pl)
>13. Apply the annex 1 to the "Amount" field
>14. If you have ideas of stuff to try to find *regressions* from the previous
>    versions, then try. You might spare librarian tears by finding something
>    before it's released. (and prevent me from having bounty on my head)
>15. Express your happiness. This is important, otherwise the sign off spell won't
>    work! The details of expressing your happiness depend on your culture and
>    habits.
>
>=== Annex 1: Subpart for each concerned page ===
>1. Set DecimalSeparators syspref to ",."
>2. Input "12,34"
>3. Press the "Tab/Tabulation" key. It will unfocus the field and trigger the
>     conversion. Validating the form does the same but it's a trick to test more
>     quickly by not switching to another page.
>4. The value should have been converted to 12.34
>5. What follows is just a list of examples. They don't have to be tried on each
>     field.
>6. Do the same with "12,34 EUR" and ensure that it's converted to "12,34"
>7. Same with "EUR 12.34" -> "12.34"
>8. "EUR 12.34â¬hi lyonthree" -> "12.34"
>9. "123 456,7" -> "123456.7"
>10. "2" -> "2"
>11. "2,0" -> "2.0"
>12. "-1'345.95" -> "-1345.95"
>13. ",1234" -> ".1234"
>14. Set DecimalSeparators syspref to ","
>15. Reload the page with the form
>16. "1.000" -> "1000"
>17. "1.000.000,10" -> "1000000.10"
>18. Set DecimalSeparators syspref to "."
>19. Reload the page with the form
>20. "1,000" -> "1000"
>21. "1,000,000.10" -> "1000000.10"
>22. Validate the form and check that the final value with the dot was
>      well handled.
>23. Think about Koalas and smile. ð¨ð¨ð¨
>
>Signed-off-by: SÃ©verine QUEUNE <severine.queune@bulac.fr>
>
>Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
>---
> .../bug12310_add_DecimalSeparators.sql             |  2 +
> installer/data/mysql/sysprefs.sql                  |  1 +
> .../prog/en/includes/decimal-input-js.inc          | 47 ++++++++++++++++++
> .../en/modules/admin/preferences/acquisitions.pref |  2 +-
> .../en/modules/admin/preferences/i18n_l10n.pref    |  4 ++
> .../prog/en/modules/members/mancredit.tt           |  4 +-
> .../prog/en/modules/members/maninvoice.tt          |  3 +-
> .../prog/en/modules/members/paycollect.tt          | 56 ++--------------------
> 8 files changed, 64 insertions(+), 55 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug12310_add_DecimalSeparators.sql
> create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/decimal-input-js.inc
>
>diff --git a/installer/data/mysql/atomicupdate/bug12310_add_DecimalSeparators.sql b/installer/data/mysql/atomicupdate/bug12310_add_DecimalSeparators.sql
>new file mode 100644
>index 0000000..e25582d
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug12310_add_DecimalSeparators.sql
>@@ -0,0 +1,2 @@
>+INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES
>+('DecimalSeparators',  ',.', NULL ,  'The allowed separators between entire and decimal parts of numbers',  'Free');
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 2ea76a4..239d492 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -139,6 +139,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('DefaultToLoggedInLibraryNoticesSlips',  '0', NULL ,  'If enabled,slips and notices editor will default to the logged in library''s rules, rather than the ''all libraries'' rules.',  'YesNo'),
> ('DefaultToLoggedInLibraryOverdueTriggers',  '0', NULL ,  'If enabled, overdue status triggers editor will default to the logged in library''s rules, rather than the ''default'' rules.',  'YesNo'),
> ('delimiter',';',';|tabulation|,|/|\\|#||','Define the default separator character for exporting reports','Choice'),
>+('DecimalSeparators',  '.,', NULL ,  'The allowed separators between entire and decimal parts of numbers',  'Free'),
> ('Display856uAsImage','OFF','OFF|Details|Results|Both','Display the URI in the 856u field as an image, the corresponding Staff Client XSLT option must be on','Choice'),
> ('DisplayClearScreenButton','0','','If set to ON, a clear screen button will appear on the circulation page.','YesNo'),
> ('displayFacetCount','0',NULL,NULL,'YesNo'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/decimal-input-js.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/decimal-input-js.inc
>new file mode 100644
>index 0000000..d3b14ae
>--- /dev/null
>+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/decimal-input-js.inc
>@@ -0,0 +1,47 @@
>+[% USE Koha %]
>+[% USE JSON.Escape %]
>+
>+<script type="text/javascript">
>+var decimal = (function() {
>+
>+    var is_decimal = function(input) {
>+        // JSON escaping to handle the strange case of " in the syspref
>+        var separators = [% Koha.Preference('DecimalSeparators').json %];
>+        var regex = "[0-9]{1,}(["+separators+"][0-9]{1,}|)";
>+        var decimal = new RegExp(regex);
>+        if (decimal.test(input)) {
>+            return true;
>+        }
>+        return false;
>+    }
>+
>+    var decimal_from_string = function(formatted_decimal) {
>+        // remove everything except digits, minuses, and the chars in DecimalSeparators syspref
>+        var formatted_decimal = formatted_decimal
>+                              .replace(/[^0-9-[% Koha.Preference('DecimalSeparators') %]]/g, '');
>+        var decimal = formatted_decimal
>+                    .replace(/[[% Koha.Preference('DecimalSeparators') %]]/g, '.');
>+        return decimal;
>+    }
>+
>+    return {
>+        /**
>+         * Handler made to process an <input> field containing a decimal number
>+         * to unformat it. So it can be processed by the server without having
>+         * to care about decimal separators (see DecimalSeparators syspref) and
>+         * currency symbols for example.
>+         */
>+        handler_unformat_input: function(field) {
>+            if (is_decimal(field.value)) {
>+                field.value = decimal_from_string(field.value);
>+            }
>+        }
>+    };
>+
>+})();
>+
>+// equivalent of $(document).ready, jQuery hasn't load yet so vanilla JS version
>+document.addEventListener("DOMContentLoaded", function() {
>+    $(".decimal-unformating").on("change", function() {decimal.handler_unformat_input(this)});
>+});
>+</script>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref
>index bf5d4bb..00ca61f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref
>@@ -28,7 +28,7 @@ Acquisitions:
>                   branch: from staff member's library.
>                   all: in system, regardless of owner.
>         -
>-            - Display currencies using the following format
>+            - Display currencies using the following format. It you want to choose the possible decimal separators for price input, please use the system preference 'DecimalSeparators'.
>             - pref: CurrencyFormat
>               choices:
>                   US: 360,000.00 (US)
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref
>index 75e4ba2..0efed92 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/i18n_l10n.pref
>@@ -68,3 +68,7 @@ I18N/L10N:
>               no: "Don't allow"
>         - notices to be translated.
>         - If set, notices will be translatable from the "Notices and Slips" interface. The language used to send a notice to a patron will be the one defined for the patron.
>+    -
>+        - "Any of these characters can be used as a separator between integer and decimal parts in the price fields. To work well with thousands separators you must (usually) choose between dot and comma to avoid ambiguity. So thousands separators come at the expense of handling both dots and commas. It you want to format price output, please use the system preference 'CurrencyFormat'"
>+        - pref: DecimalSeparators
>+          class: long
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt
>index 305910f..0dd3cff 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt
>@@ -5,6 +5,7 @@
> [% INCLUDE 'doc-head-open.inc' %]
> <title>Koha &rsaquo; Patrons &rsaquo; Create manual credit</title>
> [% INCLUDE 'doc-head-close.inc' %]
>+[% INCLUDE 'decimal-input-js.inc' %]
> </head>
> 
> <body id="pat_mancredit" class="pat">
>@@ -43,7 +44,8 @@
> 	<li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" /></li>
> 	<li><label for="desc">Description: </label><input type="text" name="desc" size="50" id="desc" /></li>
>     <li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" /></li>
>-    <li><label for="amount">Amount: </label><input type="number" name="amount" id="amount" required="required" value="" step="any" min="0" /> Example: 5.00</li>
>+    <li><label for="amount">Amount: </label><input name="amount" id="amount" class="decimal-unformating" required="required" value="" step="any" min="0" /></li>
>+
> </ol></fieldset>
> 
> <fieldset class="action"><input type="submit" name="add" value="Add credit" /> <a class="cancel" href="/cgi-bin/koha/members/boraccount.pl?borrowernumber=[% patron.borrowernumber %]">Cancel</a></fieldset>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
>index 2e139f7..d6896b6 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt
>@@ -56,7 +56,7 @@
> 	<li><label for="barcode">Barcode: </label><input type="text" name="barcode" id="barcode" /></li>
> 	<li><label for="desc">Description: </label><input type="text" name="desc" id="desc" size="50" /></li>
>     <li><label for="note">Note: </label><input type="text" name="note" size="50" id="note" /></li>
>-    <li><label for="amount">Amount: </label><input type="number" name="amount" id="amount" required="required" value="" step="any" min="0" /> Example: 5.00</li>
>+    <li><label for="amount">Amount: </label><input type="text" name="amount" id="amount" class="decimal-unformating" required="required"/></li>
> 	</ol></fieldset>
> <fieldset class="action"><input type="submit" name="add" value="Save" /> <a class="cancel" href="/cgi-bin/koha/members/boraccount.pl?borrowernumber=[% patron.borrowernumber %]">Cancel</a></fieldset>
> </form>
>@@ -72,6 +72,7 @@
> </div>
> </div>
> 
>+[% INCLUDE 'decimal-input-js.inc' %]
> [% MACRO jsinclude BLOCK %]
>     [% Asset.js("js/members-menu.js") %]
>     <script type="text/javascript">
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
>index fb42d43..364b742 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
>@@ -86,7 +86,7 @@
>     <li>
>         <label for="paid">Collect from patron: </label>
>             
>-        <input name="paid" id="paid" value="[% amountoutstanding %]" />
>+        <input name="paid" id="paid" class="decimal-unformating" value="[% amountoutstanding %]" />
>     </li>
>     [% SET payment_types = AuthorisedValues.GetAuthValueDropbox('PAYMENT_TYPE') %]
>     [% IF payment_types %]
>@@ -169,7 +169,7 @@
>     <li>
>         <label for="paid">Collect from patron: </label>
>         
>-        <input name="paid" id="paid" value="[% total | format('%.2f') %]" />
>+        <input name="paid" id="paid" class="decimal-unformating" value="[% total | format('%.2f') %]" />
>     </li>
>     [% SET payment_types = AuthorisedValues.GetAuthValueDropbox('PAYMENT_TYPE') %]
>     [% IF payment_types %]
>@@ -207,9 +207,6 @@
>     <script type= "text/javascript">
>         $(document).ready(function() {
>             $('#payindivfine, #payfine').preventDoubleFormSubmit();
>-            $("#paid").on("change",function(){
>-                moneyFormat( this );
>-            });
>         });
> 
>         prevent_default = 1;
>@@ -230,55 +227,10 @@
>             }
>         });
> 
>-        function moneyFormat(textObj) {
>-            var newValue = textObj.value;
>-            var decAmount = "";
>-            var dolAmount = "";
>-            var decFlag   = false;
>-            var aChar     = "";
>-
>-            for(i=0; i < newValue.length; i++) {
>-                aChar = newValue.substring(i, i+1);
>-                if (aChar >= "0" && aChar <= "9") {
>-                    if(decFlag) {
>-                        decAmount = "" + decAmount + aChar;
>-                    }
>-                    else {
>-                        dolAmount = "" + dolAmount + aChar;
>-                    }
>-                }
>-                if (aChar == ".") {
>-                    if (decFlag) {
>-                        dolAmount = "";
>-                        break;
>-                    }
>-                    decFlag = true;
>-                }
>-            }
>+    </script>
> 
>-            if (dolAmount == "") {
>-                dolAmount = "0";
>-            }
>-        // Strip leading 0s
>-            if (dolAmount.length > 1) {
>-                while(dolAmount.length > 1 && dolAmount.substring(0,1) == "0") {
>-                    dolAmount = dolAmount.substring(1,dolAmount.length);
>-                }
>-            }
>-            if (decAmount.length > 2) {
>-                decAmount = decAmount.substring(0,2);
>-            }
>-        // Pad right side
>-            if (decAmount.length == 1) {
>-               decAmount = decAmount + "0";
>-            }
>-            if (decAmount.length == 0) {
>-               decAmount = decAmount + "00";
>-            }
>+    [% INCLUDE 'decimal-input-js.inc' %]
> 
>-            textObj.value = dolAmount + "." + decAmount;
>-        }
>-    </script>
> [% END %]
> 
> [% INCLUDE 'intranet-bottom.inc' %]
>-- 
>2.7.4

Actions: View | Diff | Splinter Review

Attachments on bug 12310: 28455 | 60530 | 60563 | 61357 | 61603 | 61604 | 62655 | 64671 | 64672 | 64673 | 64674 | 64675 | 64678 | 64679 | 64696 | 72104 | 72213 | 72983 | 73055 | 73215 | 73546 | 73657 | 73784 | 73785 | 73873 | 73874 | 73880 | 74524 | 74525 | 74526 | 74527 | 74528 | 74529 | 75123 | 75127 | 75128 | 75129 | 75130 | 75947 | 75948 | 75949 | 75950 | 75951 | 75952 | 75954 | 75955 | 75956 | 75957 | 75958 | 75959 | 76896 | 76933 | 77971 | 77972 | 77973 | 77995 | 78017 | 78631 | 78632 | 78633 | 78634 | 78635 | 78636 | 78637 | 78638