From c55ec0f561cc73fc5477e983b7d0d36a550b8b4c Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatetsolutions.com>
Date: Fri, 9 Mar 2018 09:20:40 -0500
Subject: [PATCH] Bug 20120: Prevent writeoffs of more than the amount owed for
 a fee

In short, it is possible to 'over-writeoff' a fee such that the value of the fee is now a credit.

Test Plan:
1) Attempt to writeoff a single fee, set the amount input to more than
   the amount of the fee.
2) Note the writeoff makes the fee go negative.
3) Apply this patch
4) Attempt to repeat step 1, you should be unable to write-off an
   amount more than the amount outstanding for the fee!

Signed-off-by: George Williams <george@nekls.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 .../prog/en/modules/members/paycollect.tt     | 31 +++++++++++++++---
 members/pay.pl                                | 32 +++++++++++++------
 members/paycollect.pl                         |  4 +++
 3 files changed, 54 insertions(+), 13 deletions(-)

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 a4467b1258..22bbf22db2 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
@@ -2,6 +2,7 @@
 [% USE Koha %]
 [% USE Branches %]
 [% USE AuthorisedValues %]
+[% USE Price %]
 [% SET footerjs = 1 %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Patrons &rsaquo; Collect fine payment for  [% patron.firstname %] [% patron.surname %]</title>
@@ -119,6 +120,8 @@
     <input type="hidden" name="accountlines_id" id="accountlines_id" value="[% accountlines_id %]" />
     <input type="hidden" name="title" id="title" value="[% title %]" />
     <input type="hidden" name="payment_note" id="payment_note" value="[% payment_note %]" />
+    <input type="hidden" name="amountoutstanding" id="amountoutstanding" value="[% amountoutstanding %]" />
+    <input type="hidden" name="confirm_writeoff" id="confirm_writeoff" value="1" />
     <table>
     <thead><tr>
             <th>Description</th>
@@ -139,12 +142,14 @@
                 <li>
                     <label for="paid">Writeoff amount: </label>
                     <!-- default to writing off all -->
-                    <input name="amountwrittenoff" id="amountwrittenoff" value="[% amountoutstanding %]" />
+                    <input name="amountwrittenoff" id="amountwrittenoff" value="[% amountoutstanding | $Price %]" />
                 </li>
             </ol>
         </fieldset>
-        <div class="action"><input type="submit" name="confirm_writeoff" id="confirm_writeoff" value="Write off this charge" />
-        <a class="cancel" href="/cgi-bin/koha/members/pay.pl?borrowernumber=[% patron.borrowernumber %]">Cancel</a></div>
+        <div class="action">
+            <input type="submit" value="Write off this charge" />
+            <a class="cancel" href="/cgi-bin/koha/members/pay.pl?borrowernumber=[% patron.borrowernumber %]">Cancel</a>
+        </div>
     </form>
 [% ELSE %]
 
@@ -201,12 +206,30 @@
     [% Asset.js("js/members-menu.js") %]
     <script type= "text/javascript">
         $(document).ready(function() {
-            $('#payindivfine, #woindivfine, #payfine').preventDoubleFormSubmit();
+            $('#payindivfine, #payfine').preventDoubleFormSubmit();
             $("#paid").on("change",function(){
                 moneyFormat( this );
             });
         });
 
+        prevent_default = 1;
+        $('#woindivfine').on('submit', function(e){
+            if ( prevent_default ) {
+                e.preventDefault();
+
+                let amount_outstanding = parseFloat( $('#amountoutstanding').attr('value') );
+                let amount_writeoff = parseFloat( $('#amountwrittenoff').attr('value') );
+                if ( amount_writeoff > amount_outstanding ) {
+                    alert(_("You are attemping to writeoff more than the value of the fee."));
+                    $('#woindivfine').beenSubmitted = false;
+                } else {
+                    prevent_default = 0;
+                    $('#woindivfine').preventDoubleFormSubmit();
+                    $('#woindivfine').submit();
+                }
+            }
+        });
+
         function moneyFormat(textObj) {
             var newValue = textObj.value;
             var decAmount = "";
diff --git a/members/pay.pl b/members/pay.pl
index 05b73e5b74..3ef12008ae 100755
--- a/members/pay.pl
+++ b/members/pay.pl
@@ -93,15 +93,29 @@ if ($writeoff_all) {
     my $amount       = $input->param('amountwrittenoff');
     my $payment_note = $input->param("payment_note");
 
-    Koha::Account->new( { patron_id => $borrowernumber } )->pay(
-        {
-            amount     => $amount,
-            lines      => [ scalar Koha::Account::Lines->find($accountlines_id) ],
-            type       => 'writeoff',
-            note       => $payment_note,
-            library_id => $branch,
-        }
-    );
+    my $accountline = Koha::Account::Lines->find( $accountlines_id );
+
+    if ( $amount > $accountline->amountoutstanding ) {
+        print $input->redirect( "/cgi-bin/koha/members/paycollect.pl?"
+              . "borrowernumber=$borrowernumber"
+              . "&amount=$amount"
+              . "&amountoutstanding=" . $accountline->amountoutstanding
+              . "&accounttype=" . $accountline->accounttype
+              . "&accountlines_id=" . $accountlines_id
+              . "&writeoff_individual=1"
+              . "&error_over=1" );
+
+    } else {
+        Koha::Account->new( { patron_id => $borrowernumber } )->pay(
+            {
+                amount     => $amount,
+                lines      => [ scalar Koha::Account::Lines->find($accountlines_id) ],
+                type       => 'writeoff',
+                note       => $payment_note,
+                library_id => $branch,
+            }
+        );
+    }
 }
 
 for (@names) {
diff --git a/members/paycollect.pl b/members/paycollect.pl
index 82b5d2798e..26ccc7bb19 100755
--- a/members/paycollect.pl
+++ b/members/paycollect.pl
@@ -182,6 +182,10 @@ borrower_add_additional_fields($borrower, $template);
 
 $template->param(%$borrower);
 
+if ( $input->param('error_over') ) {
+    $template->param( error_over => 1, total_due => $input->param('amountoutstanding') );
+}
+
 $template->param(
     borrowernumber => $borrowernumber,    # some templates require global
     patron        => $patron,
-- 
2.39.5