Bug 7243: Be selective when summing up charges for blocking issues

Added RentalsInNoissueCharges and ManInvlsInNoissueCharges sys prefs

Created C4::Members::cwGetMemberAccountBallance()
* A wrapper for GetMemberAccountRecords that gives info on non-issue and
  other charges
* Other charges are:
  'Res'
  'Rent' if RentalsInNoissueCharges is Mo
  authorised_values MANUAL_INV if ManInvlsInNoissueCharges is No

C4::Members::GetMemberAccountRecords() changes:
* Dropped input param $date, it is not used

Use split charges in C4::Circulation::CanBookBeIssued() and
C4::Members::patronflags(). That way only fines decide whether an item
can be issued, and not other non-fine charges

Signed-off-by: Marc Veron <veron@veron.ch>
Rebased (updatedatabase.pl)

ManInvInNoissueCharges and RentalsInNoissueCharges ar both included by default (= behaviour as before)

All variants tested: Both included, none included, manual invoice included, rentals included.
Works fine, blocks/does not blok as appropirate, messages appear as expected.

[Oct 12, 2012 marcelr:] Amended for updatedatabase.pl
Signed-off-by: M. de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 71fe20f..1545f89 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -772,29 +772,32 @@ sub CanBookBeIssued {
     #
 
     # DEBTS
-    my ($amount) =
-      C4::Members::GetMemberAccountRecords( $borrower->{'borrowernumber'}, '' && $duedate->ymd() );
+    my ($ballance, $non_issue_ballance, $other_charges) =
+      C4::Members::GetMemberAccountBallance( $borrower->{'borrowernumber'} );
     my $amountlimit = C4::Context->preference("noissuescharge");
     my $allowfineoverride = C4::Context->preference("AllowFineOverride");
     my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
     if ( C4::Context->preference("IssuingInProcess") ) {
-        if ( $amount > $amountlimit && !$inprocess && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > $amountlimit && !$inprocess && $allowfineoverride) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $allfinesneedoverride && $amount > 0 && $amount <= $amountlimit && !$inprocess ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        if ( $non_issue_ballance > $amountlimit && !$inprocess && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_ballance );
+        } elsif ( $non_issue_ballance > $amountlimit && !$inprocess && $allowfineoverride) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_ballance );
+        } elsif ( $allfinesneedoverride && $non_issue_ballance > 0 && $non_issue_ballance <= $amountlimit && !$inprocess ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_ballance );
         }
     }
     else {
-        if ( $amount > $amountlimit && $allowfineoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > $amountlimit && !$allowfineoverride) {
-            $issuingimpossible{DEBT} = sprintf( "%.2f", $amount );
-        } elsif ( $amount > 0 && $allfinesneedoverride ) {
-            $needsconfirmation{DEBT} = sprintf( "%.2f", $amount );
+        if ( $non_issue_ballance > $amountlimit && $allowfineoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_ballance );
+        } elsif ( $non_issue_ballance > $amountlimit && !$allowfineoverride) {
+            $issuingimpossible{DEBT} = sprintf( "%.2f", $non_issue_ballance );
+        } elsif ( $non_issue_ballance > 0 && $allfinesneedoverride ) {
+            $needsconfirmation{DEBT} = sprintf( "%.2f", $non_issue_ballance );
         }
     }
+    if ($ballance > 0 && $other_charges > 0) {
+        $alerts{OTHER_CHARGES} = sprintf( "%.2f", $other_charges );
+    }
 
     my ($blocktype, $count) = C4::Members::IsMemberBlocked($borrower->{'borrowernumber'});
     if ($blocktype == -1) {

diff --git a/C4/Members.pm b/C4/Members.pm
index 6388a0f..7d4f894 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -432,21 +432,21 @@ sub patronflags {
     my %flags;
     my ( $patroninformation) = @_;
     my $dbh=C4::Context->dbh;
-    my ($amount) = GetMemberAccountRecords( $patroninformation->{'borrowernumber'});
-    if ( $amount > 0 ) {
+    my ($ballance, $owing) = GetMemberAccountBallance( $patroninformation->{'borrowernumber'});
+    if ( $owing > 0 ) {
         my %flaginfo;
         my $noissuescharge = C4::Context->preference("noissuescharge") || 5;
-        $flaginfo{'message'} = sprintf "Patron owes \$%.02f", $amount;
-        $flaginfo{'amount'}  = sprintf "%.02f", $amount;
-        if ( $amount > $noissuescharge && !C4::Context->preference("AllowFineOverride") ) {
+        $flaginfo{'message'} = sprintf "Patron owes \$%.02f", $owing;
+        $flaginfo{'amount'}  = sprintf "%.02f", $owing;
+        if ( $owing > $noissuescharge && !C4::Context->preference("AllowFineOverride") ) {
             $flaginfo{'noissues'} = 1;
         }
         $flags{'CHARGES'} = \%flaginfo;
     }
-    elsif ( $amount < 0 ) {
+    elsif ( $ballance < 0 ) {
         my %flaginfo;
-        $flaginfo{'message'} = sprintf "Patron has credit of \$%.02f", -$amount;
-        $flaginfo{'amount'}  = sprintf "%.02f", $amount;
+        $flaginfo{'message'} = sprintf "Patron has credit of \$%.02f", -$ballance;
+        $flaginfo{'amount'}  = sprintf "%.02f", $ballance;
         $flags{'CREDITS'} = \%flaginfo;
     }
     if (   $patroninformation->{'gonenoaddress'}
@@ -1128,9 +1128,8 @@ total amount outstanding for all of the account lines.
 
 =cut
 
-#'
 sub GetMemberAccountRecords {
-    my ($borrowernumber,$date) = @_;
+    my ($borrowernumber) = @_;
     my $dbh = C4::Context->dbh;
     my @acctlines;
     my $numlines = 0;
@@ -1138,14 +1137,10 @@ sub GetMemberAccountRecords {
                         SELECT * 
                         FROM accountlines 
                         WHERE borrowernumber=?);
-    my @bind = ($borrowernumber);
-    if ($date && $date ne ''){
-            $strsth.=" AND date < ? ";
-            push(@bind,$date);
-    }
     $strsth.=" ORDER BY date desc,timestamp DESC";
     my $sth= $dbh->prepare( $strsth );
-    $sth->execute( @bind );
+    $sth->execute( $borrowernumber );
+
     my $total = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
         if ( $data->{itemnumber} ) {
@@ -1161,6 +1156,42 @@ sub GetMemberAccountRecords {
     return ( $total, \@acctlines,$numlines);
 }
 
+=head2 GetMemberAccountBallance
+
+  ($total_ballance, $non_issue_ballance, $other_charges) = &GetMemberAccountBallance($borrowernumber);
+
+Calculates amount immediately owing by the patron - non-issue charges.
+Based on GetMemberAccountRecords.
+Charges exempt from non-issue are:
+* Res (reserves)
+* Rent (rental) if RentalsInNoissueCharges syspref is set to false
+* Manual invoices if ManInvInNoissueCharges syspref is set to false
+
+=cut
+
+my $ACCOUNT_TYPE_LENGTH = 5; # this is plain ridiculous...
+
+my @not_fines = ('Res');
+push @not_fines, 'Rent' unless C4::Context->preference('RentalsInNoissueCharges');
+unless ( C4::Context->preference('ManInvInNoissueCharges') ) {
+    my $dbh = C4::Context->dbh;
+    my $man_inv_types = $dbh->selectcol_arrayref(qq{SELECT authorised_value FROM authorised_values WHERE category = 'MANUAL_INV'});
+    push @not_fines, map substr($_, 0, $ACCOUNT_TYPE_LENGTH), @$man_inv_types;
+}
+my %not_fine = map {$_ => 1} @not_fines;
+
+sub GetMemberAccountBallance {
+    my ($borrowernumber) = @_;
+
+    my ($total, $acctlines) = GetMemberAccountRecords($borrowernumber);
+    my $other_charges = 0;
+    foreach (@$acctlines) {
+        $other_charges += $_->{amountoutstanding} if $not_fine{ substr($_->{accounttype}, 0, $ACCOUNT_TYPE_LENGTH) };
+    }
+
+    return ( $total, $total - $other_charges, $other_charges);
+}
+
 =head2 GetBorNotifyAcctRecord
 
   ($total, $acctlines, $count) = &GetBorNotifyAcctRecord($params,$notifyid);

diff --git a/circ/circulation.pl b/circ/circulation.pl
index fe9ae60..3d24a4a 100755 (executable)
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -190,8 +190,7 @@ if ( $print eq 'yes' && $borrowernumber ne '' ) {
 my $borrowerslist;
 my $message;
 if ($findborrower) {
-    my $borrowers = Search($findborrower, 'cardnumber');
-    my @borrowers = @$borrowers;
+    my $borrowers = Search($findborrower, 'cardnumber') || [];
     if (C4::Context->preference("AddPatronLists")) {
         $template->param(
             "AddPatronLists_".C4::Context->preference("AddPatronLists")=> "1",
@@ -202,17 +201,17 @@ if ($findborrower) {
             $template->param(categories=>$categories);
         }
     }
-    if ( $#borrowers == -1 ) {
+    if ( @$borrowers == 0 ) {
         $query->param( 'findborrower', '' );
         $message = "'$findborrower'";
     }
-    elsif ( $#borrowers == 0 ) {
-        $query->param( 'borrowernumber', $borrowers[0]->{'borrowernumber'} );
+    elsif ( @$borrowers == 1 ) {
+        $borrowernumber = $borrowers->[0]->{'borrowernumber'};
+        $query->param( 'borrowernumber', $borrowernumber );
         $query->param( 'barcode',           '' );
-        $borrowernumber = $borrowers[0]->{'borrowernumber'};
     }
     else {
-        $borrowerslist = \@borrowers;
+        $borrowerslist = $borrowers;
     }
 }
 
@@ -541,7 +540,6 @@ foreach my $flag ( sort keys %$flags ) {
     $flags->{$flag}->{'message'} =~ s#\n#<br />#g;
     if ( $flags->{$flag}->{'noissues'} ) {
         $template->param(
-            flagged  => 1,
             noissues => 'true',
         );
         if ( $flag eq 'GNA' ) {
@@ -573,7 +571,6 @@ foreach my $flag ( sort keys %$flags ) {
         if ( $flag eq 'CHARGES' ) {
             $template->param(
                 charges    => 'true',
-                flagged    => 1,
                 chargesmsg => $flags->{'CHARGES'}->{'message'},
                 chargesamount => $flags->{'CHARGES'}->{'amount'},
             );
@@ -588,7 +585,6 @@ foreach my $flag ( sort keys %$flags ) {
         elsif ( $flag eq 'ODUES' ) {
             $template->param(
                 odues    => 'true',
-                flagged  => 1,
                 oduesmsg => $flags->{'ODUES'}->{'message'}
             );
 
@@ -600,7 +596,6 @@ foreach my $flag ( sort keys %$flags ) {
         elsif ( $flag eq 'NOTES' ) {
             $template->param(
                 notes    => 'true',
-                flagged  => 1,
                 notesmsg => $flags->{'NOTES'}->{'message'}
             );
         }

diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
index d5d15d7..796a553 100644 (file)
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -327,6 +327,8 @@ INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('EasyAnalyticalRecords','0','If on, display in the catalogue screens tools to easily setup analytical record relationships','','YesNo');
 INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('OpacShowRecentComments',0,'If ON a link to recent comments will appear in the OPAC masthead',NULL,'YesNo');
 INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('CircAutoPrintQuickSlip', '1', 'Choose what should happen when an empty barcode field is submitted in circulation: Display a print quick slip window or Clear the screen.',NULL,'YesNo');
+INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('RentalsInNoissueCharges', '1', 'Include rental charges when summing up charges for NoissueCharge.',NULL,'YesNo');
+INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('ManInvInNoissueCharges', '1', 'Include MAN_INV charges when summing up charges for NoissueCharge.',NULL,'YesNo');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('NoticeCSS','','Notices CSS url.',NULL,'free');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('SlipCSS','','Slips CSS url.',NULL,'free');
 INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('TransferWhenCancelAllWaitingHolds','0','Transfer items when cancelling all waiting holds',NULL,'YesNo');

diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 77265f4..a03b3e6 100755 (executable)
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -5658,6 +5658,7 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     SetVersion ($DBversion);
 }
 
+
 $DBversion = '3.09.00.033';
 if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
    $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('OpacSuppressionByIPRange','','Restrict the suppression to IP adresses outside of the IP range','','free');");
@@ -5972,7 +5973,6 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     SetVersion($DBversion);
 }
 
-
 $DBversion = "3.09.00.057";
 if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     $dbh->do("ALTER TABLE aqbasket ADD deliveryplace VARCHAR(10) default NULL AFTER basketgroupid;");
@@ -6339,6 +6339,14 @@ if ( CheckVersion($DBversion) ) {
    SetVersion ($DBversion);
 }
 
+$DBversion = "3.11.00.XXX";
+if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
+    $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('RentalsInNoissueCharges', '1', 'Include rental charges when summing up charges for NoissueCharge.',NULL,'YesNo');");
+    $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('ManInvInNoissueCharges', '1', 'Include MAN_INV charges when summing up charges for NoissueCharge.',NULL,'YesNo');");
+    print "Upgrade to $DBversion done (Add sysprefs RentalsInNoissueCharges and ManInvInNoissueCharges.)\n";
+    SetVersion($DBversion);
+}
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
index 645b19b..1156b46 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -264,6 +264,18 @@ Circulation:
             - pref: noissuescharge
               class: integer
             - '[% local_currency %] in fines.'
+        -
+            - pref: RentalsInNoissueCharges
+              choices:
+                  yes: Include
+                  no: "Don't include"
+            - rental charges when summing up charges for NoissueCharge.
+        -
+            - pref: ManInvInNoissueCharges
+              choices:
+                  yes: Include
+                  no: "Don't include"
+            - MAN_INV charges when summing up charges for NoissueCharge.
         -
             - pref: ReturnBeforeExpiry
               choices:

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index 9c045cd..9af072f 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -315,6 +315,10 @@ function validate1(date) {
     <div class="dialog message">This item has been lost with a status of "[% alert.ITEM_LOST %]".</div>
 [% END %]
 
+[% IF ( alert.OTHER_CHARGES ) %]
+    <div class="dialog message">The patron has unpaid charges for reserves, rentals etc of [% alert.OTHER_CHARGES %]</div>
+[% END %]
+
 [% IF ( NEEDSCONFIRMATION ) %]
 <div class="yui-g">
 

diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 5ef5531..2b8563c 100755 (executable)
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -142,7 +142,6 @@ $template->param(   BORROWER_INFO     => \@bordat,
                     OPACMySummaryHTML => (C4::Context->preference("OPACMySummaryHTML")) ? 1 : 0,
                     surname           => $borr->{surname},
                     showname          => $borr->{showname},
-
                 );
 
 #get issued items ....

diff --git a/t/db_dependent/lib/KohaTest/Members.pm b/t/db_dependent/lib/KohaTest/Members.pm
index ea12cb7..edaa85b 100644 (file)
--- a/t/db_dependent/lib/KohaTest/Members.pm
+++ b/t/db_dependent/lib/KohaTest/Members.pm
@@ -27,6 +27,7 @@ sub methods : Test( 1 ) {
                       GetPendingIssues 
                       GetAllIssues 
                       GetMemberAccountRecords 
+                      GetMemberAccountBallance
                       GetBorNotifyAcctRecord 
                       checkuniquemember 
                       checkcardnumber