Bug 5549 : Overdues : Handle some date comparison and display issues
[koha.git] / C4 / Overdues.pm
index 7ea969c..583fb96 100644 (file)
@@ -2,6 +2,7 @@ package C4::Overdues;
 
 
 # Copyright 2000-2002 Katipo Communications
+# copyright 2010 BibLibre
 #
 # This file is part of Koha.
 #
@@ -78,7 +79,6 @@ BEGIN {
        # subs to move to Members.pm
        push @EXPORT, qw(
         &CheckBorrowerDebarred
-        &UpdateBorrowerDebarred
        );
        # subs to move to Biblio.pm
        push @EXPORT, qw(
@@ -124,7 +124,7 @@ sub Getoverdues {
    SELECT issues.*, items.itype as itemtype, items.homebranch, items.barcode
      FROM issues 
 LEFT JOIN items       USING (itemnumber)
-    WHERE date_due < CURDATE() 
+    WHERE date_due < NOW()
 ";
     } else {
         $statement = "
@@ -132,7 +132,7 @@ LEFT JOIN items       USING (itemnumber)
      FROM issues 
 LEFT JOIN items       USING (itemnumber)
 LEFT JOIN biblioitems USING (biblioitemnumber)
-    WHERE date_due < CURDATE() 
+    WHERE date_due < NOW()
 ";
     }
 
@@ -156,7 +156,7 @@ LEFT JOIN biblioitems USING (biblioitemnumber)
 
 =head2 checkoverdues
 
-($count, $overdueitems) = checkoverdues($borrowernumber);
+    ($count, $overdueitems) = checkoverdues($borrowernumber);
 
 Returns a count and a list of overdueitems for a given borrowernumber
 
@@ -164,13 +164,42 @@ Returns a count and a list of overdueitems for a given borrowernumber
 
 sub checkoverdues {
     my $borrowernumber = shift or return;
+    # don't select biblioitems.marc or biblioitems.marcxml... too slow on large systems
     my $sth = C4::Context->dbh->prepare(
-        "SELECT * FROM issues
+        "SELECT biblio.*, items.*, issues.*,
+                biblioitems.volume,
+                biblioitems.number,
+                biblioitems.itemtype,
+                biblioitems.isbn,
+                biblioitems.issn,
+                biblioitems.publicationyear,
+                biblioitems.publishercode,
+                biblioitems.volumedate,
+                biblioitems.volumedesc,
+                biblioitems.collectiontitle,
+                biblioitems.collectionissn,
+                biblioitems.collectionvolume,
+                biblioitems.editionstatement,
+                biblioitems.editionresponsibility,
+                biblioitems.illus,
+                biblioitems.pages,
+                biblioitems.notes,
+                biblioitems.size,
+                biblioitems.place,
+                biblioitems.lccn,
+                biblioitems.url,
+                biblioitems.cn_source,
+                biblioitems.cn_class,
+                biblioitems.cn_item,
+                biblioitems.cn_suffix,
+                biblioitems.cn_sort,
+                biblioitems.totalissues
+         FROM issues
          LEFT JOIN items       ON issues.itemnumber      = items.itemnumber
          LEFT JOIN biblio      ON items.biblionumber     = biblio.biblionumber
          LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
             WHERE issues.borrowernumber  = ?
-            AND   issues.date_due < CURDATE()"
+            AND   issues.date_due < NOW()"
     );
     # FIXME: SELECT * across 4 tables?  do we really need the marc AND marcxml blobs??
     $sth->execute($borrowernumber);
@@ -180,8 +209,9 @@ sub checkoverdues {
 
 =head2 CalcFine
 
-  ($amount, $chargename, $daycount, $daycounttotal) =
-    &CalcFine($item, $categorycode, $branch, $days_overdue, $description, $start_date, $end_date );
+    ($amount, $chargename,  $daycounttotal) = &CalcFine($item,
+                                  $categorycode, $branch,
+                                  $start_dt, $end_dt );
 
 Calculates the fine for a book.
 
@@ -199,13 +229,8 @@ the book.
 
 C<$branchcode> is the library (string) whose issuingrules govern this transaction.
 
-C<$days_overdue> is the number of days elapsed since the book's due date.
-  NOTE: supplying days_overdue is deprecated.
-
-C<$start_date> & C<$end_date> are C4::Dates objects 
+C<$start_date> & C<$end_date> are DateTime objects
 defining the date range over which to determine the fine.
-Note that if these are defined, we ignore C<$difference> and C<$dues> , 
-but retain these for backwards-comptibility with extant fines scripts.
 
 Fines scripts should just supply the date range over which to calculate the fine.
 
@@ -219,8 +244,6 @@ the categoryitem table, whatever that is.
 C<$daycount> is the number of days between start and end dates, Calendar adjusted (where needed), 
 minus any applicable grace period.
 
-C<$daycounttotal> is C<$daycount> without consideration of grace period.
-
 FIXME - What is chargename supposed to be ?
 
 FIXME: previously attempted to return C<$message> as a text message, either "First Notice", "Second Notice",
@@ -229,55 +252,45 @@ or "Final Notice".  But CalcFine never defined any value.
 =cut
 
 sub CalcFine {
-    my ( $item, $bortype, $branchcode, $difference ,$dues , $start_date, $end_date  ) = @_;
-       $debug and warn sprintf("CalcFine(%s, %s, %s, %s, %s, %s, %s)",
-                       ($item ? '{item}' : 'UNDEF'), 
-                       ($bortype    || 'UNDEF'), 
-                       ($branchcode || 'UNDEF'), 
-                       ($difference || 'UNDEF'), 
-                       ($dues       || 'UNDEF'), 
-                       ($start_date ? ($start_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF'), 
-                       (  $end_date ? (  $end_date->output('iso') || 'Not a C4::Dates object') : 'UNDEF')
-       );
+    my ( $item, $bortype, $branchcode, $start_date, $end_date  ) = @_;
     my $dbh = C4::Context->dbh;
     my $amount = 0;
-       my $daystocharge;
-       # get issuingrules (fines part will be used)
-    $debug and warn sprintf("CalcFine calling GetIssuingRule(%s, %s, %s)", $bortype, $item->{'itemtype'}, $branchcode);
-    my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'}, $branchcode);
-       if($difference) {
-               # if $difference is supplied, the difference has already been calculated, but we still need to adjust for the calendar.
-       # use copy-pasted functions from calendar module.  (deprecated -- these functions will be removed from C4::Overdues ).
-           my $countspecialday    =    &GetSpecialHolidays($dues,$item->{itemnumber});
-           my $countrepeatableday = &GetRepeatableHolidays($dues,$item->{itemnumber},$difference);    
-           my $countalldayclosed  = $countspecialday + $countrepeatableday;
-           $daystocharge = $difference - $countalldayclosed;
-       } else {
-               # if $difference is not supplied, we have C4::Dates objects giving us the date range, and we use the calendar module.
-               if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
-                       my $calendar = C4::Calendar->new( branchcode => $branchcode );
-                       $daystocharge = $calendar->daysBetween( $start_date, $end_date );
-               } else {
-                       $daystocharge = Date_to_Days(split('-',$end_date->output('iso'))) - Date_to_Days(split('-',$start_date->output('iso')));
-               }
-       }
-       # correct for grace period.
-       my $days_minus_grace = $daystocharge - $data->{'firstremind'};
-    if ($data->{'chargeperiod'} > 0 && $days_minus_grace > 0 ) { 
-        $amount = int($daystocharge / $data->{'chargeperiod'}) * $data->{'fine'};
+    my $charge_duration;
+    # get issuingrules (fines part will be used)
+    my $data = C4::Circulation::GetIssuingRule($bortype, $item->{itemtype}, $branchcode);
+    if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
+        my $calendar = Koha::Calendar->new( branchcode => $branchcode );
+        $charge_duration = $calendar->days_between( $start_date, $end_date );
+    } else {
+        $charge_duration = $end_date - $start_date;
+    }
+    # correct for grace period.
+    my $fine_unit = $data->{lengthunit};
+    $fine_unit ||= 'days';
+    my $chargeable_units;
+    if ($fine_unit eq 'hours') {
+        $chargeable_units = $charge_duration->hours(); # TODO closed times???
+    }
+    else {
+        $chargeable_units = $charge_duration->day;
+    }
+    my $days_minus_grace = $chargeable_units - $data->{firstremind};
+    if ($data->{'chargeperiod'}  && $days_minus_grace  ) {
+        $amount = int($chargeable_units / $data->{'chargeperiod'}) * $data->{'fine'};# TODO fine calc should be in cents
     } else {
         # a zero (or null)  chargeperiod means no charge.
     }
-       $amount = C4::Context->preference('maxFine') if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine')));
-       $debug and warn sprintf("CalcFine returning (%s, %s, %s, %s)", $amount, $data->{'chargename'}, $days_minus_grace, $daystocharge);
-    return ($amount, $data->{'chargename'}, $days_minus_grace, $daystocharge);
+    if(C4::Context->preference('maxFine') && ( $amount > C4::Context->preference('maxFine'))) {
+        $amount = C4::Context->preference('maxFine');
+    }
+    return ($amount, $data->{chargename}, $days_minus_grace);
     # FIXME: chargename is NEVER populated anywhere.
 }
 
 
 =head2 GetSpecialHolidays
 
-&GetSpecialHolidays($date_dues,$itemnumber);
+    &GetSpecialHolidays($date_dues,$itemnumber);
 
 return number of special days  between date of the day and date due
 
@@ -335,7 +348,7 @@ AND branchcode=?
 
 =head2 GetRepeatableHolidays
 
-&GetRepeatableHolidays($date_dues, $itemnumber, $difference,);
+    &GetRepeatableHolidays($date_dues, $itemnumber, $difference,);
 
 return number of day closed between date of the day and date due
 
@@ -373,7 +386,7 @@ sub GetRepeatableHolidays {
 
 =head2 GetWayFromItemnumber
 
-&Getwdayfromitemnumber($itemnumber);
+    &Getwdayfromitemnumber($itemnumber);
 
 return the different week day from repeatable_holidays table
 
@@ -401,7 +414,7 @@ sub GetWdayFromItemnumber {
 
 =head2 GetIssuesIteminfo
 
-&GetIssuesIteminfo($itemnumber);
+    &GetIssuesIteminfo($itemnumber);
 
 return all data from issues about item
 
@@ -425,7 +438,7 @@ sub GetIssuesIteminfo {
 
 =head2 UpdateFine
 
-  &UpdateFine($itemnumber, $borrowernumber, $amount, $type, $description);
+    &UpdateFine($itemnumber, $borrowernumber, $amount, $type, $description);
 
 (Note: the following is mostly conjecture and guesswork.)
 
@@ -487,12 +500,12 @@ sub UpdateFine {
 
     if ( my $data = $sth->fetchrow_hashref ) {
 
-               # we're updating an existing fine.  Only modify if we're adding to the charge.
+               # we're updating an existing fine.  Only modify if amount changed
         # Note that in the current implementation, you cannot pay against an accruing fine
         # (i.e. , of accounttype 'FU').  Doing so will break accrual.
        if ( $data->{'amount'} != $amount ) {
             my $diff = $amount - $data->{'amount'};
-            $diff = 0 if ( $data->{amount} > $amount);
+           #3341: diff could be positive or negative!
             my $out  = $data->{'amountoutstanding'} + $diff;
             my $query = "
                 UPDATE accountlines
@@ -552,7 +565,7 @@ sub UpdateFine {
 
 =head2 BorType
 
-  $borrower = &BorType($borrowernumber);
+    $borrower = &BorType($borrowernumber);
 
 Looks up a patron by borrower number.
 
@@ -578,7 +591,7 @@ sub BorType {
 
 =head2 ReplacementCost
 
-  $cost = &ReplacementCost($itemnumber);
+    $cost = &ReplacementCost($itemnumber);
 
 Returns the replacement cost of the item with the given item number.
 
@@ -599,7 +612,7 @@ sub ReplacementCost {
 
 =head2 GetFine
 
-$data->{'sum(amountoutstanding)'} = &GetFine($itemnum,$borrowernumber);
+    $data->{'sum(amountoutstanding)'} = &GetFine($itemnum,$borrowernumber);
 
 return the total of fine
 
@@ -613,13 +626,16 @@ C<$borrowernumber> is the borrowernumber
 sub GetFine {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
-    my $query = "SELECT sum(amountoutstanding) FROM accountlines
-    where accounttype like 'F%'  
-  AND amountoutstanding > 0 AND itemnumber = ? AND borrowernumber=?";
+    my $query = q|SELECT sum(amountoutstanding) as fineamount FROM accountlines
+    where accounttype like 'F%'
+  AND amountoutstanding > 0 AND itemnumber = ? AND borrowernumber=?|;
     my $sth = $dbh->prepare($query);
     $sth->execute( $itemnum, $borrowernumber );
-    my $data = $sth->fetchrow_hashref();
-    return ( $data->{'sum(amountoutstanding)'} );
+    my $fine = $sth->fetchrow_hashref();
+    if ($fine->{fineamount}) {
+        return $fine->{fineamount};
+    }
+    return 0;
 }
 
 
@@ -628,7 +644,7 @@ sub GetFine {
 FIXME - This sub should be deprecated and removed.
 It ignores branch and defaults.
 
-$data = &GetIssuingRules($itemtype,$categorycode);
+    $data = &GetIssuingRules($itemtype,$categorycode);
 
 Looks up for all issuingrules an item info 
 
@@ -675,7 +691,7 @@ sub ReplacementCost2 {
 
 =head2 GetNextIdNotify
 
-($result) = &GetNextIdNotify($reference);
+    ($result) = &GetNextIdNotify($reference);
 
 Returns the new file number
 
@@ -714,7 +730,7 @@ sub GetNextIdNotify {
 
 =head2 NumberNotifyId
 
-(@notify) = &NumberNotifyId($borrowernumber);
+    (@notify) = &NumberNotifyId($borrowernumber);
 
 Returns amount for all file per borrowers
 C<@notify> array contains all file per borrowers
@@ -740,7 +756,7 @@ sub NumberNotifyId{
 
 =head2 AmountNotify
 
-($totalnotify) = &AmountNotify($notifyid);
+    ($totalnotify) = &AmountNotify($notifyid);
 
 Returns amount for all file per borrowers
 C<$notifyid> is the file number
@@ -767,7 +783,7 @@ sub AmountNotify{
 
 =head2 GetNotifyId
 
-($notify_id) = &GetNotifyId($borrowernumber,$itemnumber);
+    ($notify_id) = &GetNotifyId($borrowernumber,$itemnumber);
 
 Returns the file number per borrower and itemnumber
 
@@ -797,7 +813,9 @@ sub GetNotifyId {
 
 =head2 CreateItemAccountLine
 
-() = &CreateItemAccountLine($borrowernumber,$itemnumber,$date,$amount,$description,$accounttype,$amountoutstanding,$timestamp,$notify_id,$level);
+    () = &CreateItemAccountLine($borrowernumber, $itemnumber, $date, $amount,
+                               $description, $accounttype, $amountoutstanding, 
+                               $timestamp, $notify_id, $level);
 
 update the account lines with file number or with file level
 
@@ -850,7 +868,7 @@ sub CreateItemAccountLine {
 
 =head2 UpdateAccountLines
 
-() = &UpdateAccountLines($notify_id,$notify_level,$borrowernumber,$itemnumber);
+    () = &UpdateAccountLines($notify_id,$notify_level,$borrowernumber,$itemnumber);
 
 update the account lines with file number or with file level
 
@@ -893,7 +911,7 @@ sub UpdateAccountLines {
 
 =head2 GetItems
 
-($items) = &GetItems($itemnumber);
+    ($items) = &GetItems($itemnumber);
 
 Returns the list of all delays from overduerules.
 
@@ -922,7 +940,7 @@ sub GetItems {
 
 =head2 GetOverdueDelays
 
-(@delays) = &GetOverdueDelays($categorycode);
+    (@delays) = &GetOverdueDelays($categorycode);
 
 Returns the list of all delays from overduerules.
 
@@ -945,27 +963,27 @@ sub GetOverdueDelays {
 
 =head2 GetBranchcodesWithOverdueRules
 
-=over 4
-
-my @branchcodes = C4::Overdues::GetBranchcodesWithOverdueRules()
+    my @branchcodes = C4::Overdues::GetBranchcodesWithOverdueRules()
 
 returns a list of branch codes for branches with overdue rules defined.
 
-=back
-
 =cut
 
 sub GetBranchcodesWithOverdueRules {
     my $dbh               = C4::Context->dbh;
-    my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> ''");
+    my $rqoverduebranches = $dbh->prepare("SELECT DISTINCT branchcode FROM overduerules WHERE delay1 IS NOT NULL AND branchcode <> '' ORDER BY branchcode");
     $rqoverduebranches->execute;
     my @branches = map { shift @$_ } @{ $rqoverduebranches->fetchall_arrayref };
+    if (!$branches[0]) {
+       my $availbranches = C4::Branch::GetBranches();
+       @branches = keys %$availbranches;
+    }
     return @branches;
 }
 
 =head2 CheckAccountLineLevelInfo
 
-($exist) = &CheckAccountLineLevelInfo($borrowernumber,$itemnumber,$accounttype,notify_level);
+    ($exist) = &CheckAccountLineLevelInfo($borrowernumber,$itemnumber,$accounttype,notify_level);
 
 Check and Returns the list of all overdue books.
 
@@ -999,7 +1017,7 @@ sub CheckAccountLineLevelInfo {
 
 =head2 GetOverduerules
 
-($overduerules) = &GetOverduerules($categorycode);
+    ($overduerules) = &GetOverduerules($categorycode);
 
 Returns the value of borrowers (debarred or not) with notify level
 
@@ -1026,7 +1044,7 @@ sub GetOverduerules {
 
 =head2 CheckBorrowerDebarred
 
-($debarredstatus) = &CheckBorrowerDebarred($borrowernumber);
+    ($debarredstatus) = &CheckBorrowerDebarred($borrowernumber);
 
 Check if the borrowers is already debarred
 
@@ -1044,39 +1062,18 @@ sub CheckBorrowerDebarred {
         SELECT debarred
         FROM borrowers
         WHERE borrowernumber=?
+        AND debarred > NOW()
     |;
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
-    my ($debarredstatus) = $sth->fetchrow;
-    return ( $debarredstatus eq '1' ? 1 : 0 );
+    my $debarredstatus = $sth->fetchrow;
+    return $debarredstatus;
 }
 
-=head2 UpdateBorrowerDebarred
-
-($borrowerstatut) = &UpdateBorrowerDebarred($borrowernumber);
-
-update status of borrowers in borrowers table (field debarred)
-
-C<$borrowernumber> borrower number
-
-=cut
-
-sub UpdateBorrowerDebarred{
-    my($borrowernumber) = @_;
-    my $dbh = C4::Context->dbh;
-        my $query=qq|UPDATE borrowers
-             SET debarred='1'
-                     WHERE borrowernumber=?
-            |;
-    my $sth=$dbh->prepare($query);
-        $sth->execute($borrowernumber);
-        $sth->finish;
-        return 1;
-}
 
 =head2 CheckExistantNotifyid
 
-  ($exist) = &CheckExistantNotifyid($borrowernumber,$itemnumber,$accounttype,$notify_id);
+    ($exist) = &CheckExistantNotifyid($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
 Check and Returns the notify id if exist else return 0.
 
@@ -1103,7 +1100,7 @@ sub CheckExistantNotifyid {
 
 =head2 CheckAccountLineItemInfo
 
-  ($exist) = &CheckAccountLineItemInfo($borrowernumber,$itemnumber,$accounttype,$notify_id);
+    ($exist) = &CheckAccountLineItemInfo($borrowernumber,$itemnumber,$accounttype,$notify_id);
 
 Check and Returns the list of all overdue items from the same file number(notify_id).
 
@@ -1177,12 +1174,14 @@ sub GetOverduesForBranch {
             borrowers.phone,
             borrowers.email,
                biblio.title,
+               biblio.author,
                biblio.biblionumber,
                issues.date_due,
                issues.returndate,
                issues.branchcode,
              branches.branchname,
                 items.barcode,
+                items.homebranch,
                 items.itemcallnumber,
                 items.location,
                 items.itemnumber,
@@ -1202,7 +1201,7 @@ sub GetOverduesForBranch {
     WHERE (accountlines.amountoutstanding  != '0.000000')
       AND (accountlines.accounttype         = 'FU'      )
       AND (issues.branchcode =  ?   )
-      AND (issues.date_due  < CURDATE())
+      AND (issues.date_due  < NOW())
     ";
     my @getoverdues;
     my $i = 0;
@@ -1228,9 +1227,9 @@ sub GetOverduesForBranch {
 
 =head2 AddNotifyLine
 
-&AddNotifyLine($borrowernumber, $itemnumber, $overduelevel, $method, $notifyId)
+    &AddNotifyLine($borrowernumber, $itemnumber, $overduelevel, $method, $notifyId)
 
-Creat a line into notify, if the method is phone, the notification_send_date is implemented to
+Create a line into notify, if the method is phone, the notification_send_date is implemented to
 
 =cut
 
@@ -1258,7 +1257,7 @@ sub AddNotifyLine {
 
 =head2 RemoveNotifyLine
 
-&RemoveNotifyLine( $borrowernumber, $itemnumber, $notify_date );
+    &RemoveNotifyLine( $borrowernumber, $itemnumber, $notify_date );
 
 Cancel a notification
 
@@ -1283,6 +1282,6 @@ __END__
 
 =head1 AUTHOR
 
-Koha Development Team <info@koha.org>
+Koha Development Team <http://koha-community.org/>
 
 =cut