Bug 8365: Add a renewal duration in the issuing rules
authorJonathan Druart <jonathan.druart@biblibre.com>
Fri, 7 Sep 2012 14:31:44 +0000 (16:31 +0200)
committerJared Camins-Esakov <jcamins@cpbibliography.com>
Fri, 22 Mar 2013 11:56:59 +0000 (07:56 -0400)
Renew an issue for a number of days (filled in the issuing rules).

Test if rules work for any i[item]types and if there is no regression.

- new column issuingrules.renewalperiod
- remove all occurrences of an already removed syspref (globalDueDate)
- remove an unused routine (Overdues::GetIssuingRules)

How it works:
- On existing installations, the issuingrules.renewalperiod =
  issuingrules.loanlength. So the behaviour is the same before and after
  this patch.
- when you add a rule, you can choose a renewal period (the unit value
  is the issuingrules.unit). So you can have a renewal period in hours
  or days.
- The default value for the renewal period is 21 days (same as
  loanlength)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Test comments on second patch.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
C4/Circulation.pm
C4/Overdues.pm
admin/smart-rules.pl
installer/data/mysql/kohastructure.sql
installer/data/mysql/updatedatabase.pl
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
t/db_dependent/lib/KohaTest/Overdues.pm

index bb5b718..425e16c 100644 (file)
@@ -1334,13 +1334,18 @@ Get loan length for an itemtype, a borrower type and a branch
 sub GetLoanLength {
     my ( $borrowertype, $itemtype, $branchcode ) = @_;
     my $dbh = C4::Context->dbh;
-    my $sth =
-      $dbh->prepare(
-'select issuelength, lengthunit from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null'
-      );
-# warn "in get loan lenght $borrowertype $itemtype $branchcode ";
-# try to find issuelength & return the 1st available.
-# check with borrowertype, itemtype and branchcode, then without one of those parameters
+    my $sth = $dbh->prepare(qq{
+        SELECT issuelength, lengthunit, renewalperiod
+        FROM issuingrules
+        WHERE   categorycode=?
+            AND itemtype=?
+            AND branchcode=?
+            AND issuelength IS NOT NULL
+    });
+
+    # try to find issuelength & return the 1st available.
+    # check with borrowertype, itemtype and branchcode, then without one of those parameters
+
     $sth->execute( $borrowertype, $itemtype, $branchcode );
     my $loanlength = $sth->fetchrow_hashref;
     return $loanlength
@@ -1384,6 +1389,7 @@ sub GetLoanLength {
     # if no rule is set => 21 days (hardcoded)
     return {
         issuelength => 21,
+        renewalperiod => 21,
         lengthunit => 'days',
     };
 
@@ -1418,7 +1424,7 @@ sub GetHardDueDate {
 
 FIXME - This is a copy-paste of GetLoanLength
 as a stop-gap.  Do not wish to change API for GetLoanLength 
-this close to release, however, Overdues::GetIssuingRules is broken.
+this close to release.
 
 Get the issuing rule for an itemtype, a borrower type and a branch
 Returns a hashref from the issuingrules table.
@@ -2440,63 +2446,29 @@ sub CanBookBeRenewed {
     my $dbh       = C4::Context->dbh;
     my $renews    = 1;
     my $renewokay = 0;
-       my $error;
+    my $error;
 
-    # Look in the issues table for this item, lent to this borrower,
-    # and not yet returned.
+    my $borrower    = C4::Members::GetMemberDetails( $borrowernumber, 0 )   or return;
+    my $item        = GetItem($itemnumber)                                  or return;
+    my $itemissue   = GetItemIssue($itemnumber)                             or return;
 
-    # Look in the issues table for this item, lent to this borrower,
-    # and not yet returned.
-    my %branch = (
-            'ItemHomeLibrary' => 'items.homebranch',
-            'PickupLibrary'   => 'items.holdingbranch',
-            'PatronLibrary'   => 'borrowers.branchcode'
-            );
-    my $controlbranch = $branch{C4::Context->preference('CircControl')};
-    my $itype         = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype';
-    
-    my $sthcount = $dbh->prepare("
-                   SELECT 
-                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
-                   FROM  issuingrules, 
-                   issues
-                   LEFT JOIN items USING (itemnumber) 
-                   LEFT JOIN borrowers USING (borrowernumber) 
-                   LEFT JOIN biblioitems USING (biblioitemnumber)
-                   
-                   WHERE
-                    (issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')
-                   AND
-                    (issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')
-                   AND
-                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
-                   AND 
-                    borrowernumber = ? 
-                   AND
-                    itemnumber = ?
-                   ORDER BY
-                    issuingrules.categorycode desc,
-                    issuingrules.itemtype desc,
-                    issuingrules.branchcode desc
-                   LIMIT 1;
-                  ");
-
-    $sthcount->execute( $borrowernumber, $itemnumber );
-    if ( my $data1 = $sthcount->fetchrow_hashref ) {
-        if ( ( $data1->{renewalsallowed} && $data1->{renewalsallowed} > $data1->{renewals} ) || $override_limit ) {
-            $renewokay = 1;
-        }
-        else {
-            $error = "too_many";
-        }
+    my $branchcode  = _GetCircControlBranch($item, $borrower);
 
-        my $resstatus = C4::Reserves::GetReserveStatus($itemnumber);
-        if ( $resstatus eq "Waiting" or $resstatus eq "Reserved" ) {
-            $renewokay = 0;
-            $error = "on_reserve";
-        }
+    my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode);
+
+    if ( ( $issuingrule->{renewalsallowed} > $itemissue->{renewals} ) || $override_limit ) {
+        $renewokay = 1;
+    } else {
+        $error = "too_many";
     }
-    return ($renewokay,$error);
+
+    my $resstatus = C4::Reserves::GetReserveStatus($itemnumber);
+    if ( $resstatus eq "Waiting" or $resstatus eq "Reserved" ) {
+        $renewokay = 0;
+        $error = "on_reserve";
+    }
+
+    return ( $renewokay, $error );
 }
 
 =head2 AddRenewal
@@ -2557,7 +2529,7 @@ sub AddRenewal {
         $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
                                         $issuedata->{date_due} :
                                         DateTime->now( time_zone => C4::Context->tz());
-        $datedue =  CalcDateDue($datedue,$itemtype,$issuedata->{'branchcode'},$borrower);
+        $datedue =  CalcDateDue($datedue, $itemtype, $issuedata->{'branchcode'}, $borrower, 'is a renewal');
     }
 
     # Update the issues record to have the new due date, and a new count
@@ -3026,58 +2998,51 @@ C<$borrower> = Borrower object
 =cut
 
 sub CalcDateDue {
-    my ( $startdate, $itemtype, $branch, $borrower ) = @_;
+    my ( $startdate, $itemtype, $branch, $borrower, $isrenewal ) = @_;
+
+    $isrenewal ||= 0;
 
     # loanlength now a href
     my $loanlength =
-      GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch );
+            GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch );
+
+    my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} )
+            ? qq{renewalperiod}
+            : qq{issuelength};
 
     my $datedue;
 
-    # if globalDueDate ON the datedue is set to that date
-    if (C4::Context->preference('globalDueDate')
-        && ( C4::Context->preference('globalDueDate') =~
-            C4::Dates->regexp('syspref') )
-      ) {
-        $datedue = dt_from_string(
-            C4::Context->preference('globalDueDate'),
-            C4::Context->preference('dateformat')
-        );
+    # calculate the datedue as normal
+    if ( C4::Context->preference('useDaysMode') eq 'Days' )
+    {    # ignoring calendar
+        my $dt =
+          DateTime->now( time_zone => C4::Context->tz() )
+          ->truncate( to => 'minute' );
+        if ( $loanlength->{lengthunit} eq 'hours' ) {
+            $dt->add( hours => $loanlength->{$length_key} );
+        } else {    # days
+            $dt->add( days => $loanlength->{$length_key} );
+            $dt->set_hour(23);
+            $dt->set_minute(59);
+        }
+        # break
+        return $dt;
     } else {
-
-        # otherwise, calculate the datedue as normal
-        if ( C4::Context->preference('useDaysMode') eq 'Days' )
-        {    # ignoring calendar
-            my $dt =
-              DateTime->now( time_zone => C4::Context->tz() )
-              ->truncate( to => 'minute' );
-            if ( $loanlength->{lengthunit} eq 'hours' ) {
-                $dt->add( hours => $loanlength->{issuelength} );
-            } else {    # days
-                $dt->add( days => $loanlength->{issuelength} );
-                $dt->set_hour(23);
-                $dt->set_minute(59);
-            }
-            # break
-            return $dt;
-
-        } else {
-            my $dur;
-            if ($loanlength->{lengthunit} eq 'hours') {
-                $dur = DateTime::Duration->new( hours => $loanlength->{issuelength});
-            }
-            else { # days
-                $dur = DateTime::Duration->new( days => $loanlength->{issuelength});
-            }
-            if (ref $startdate ne 'DateTime' ) {
-                $startdate = dt_from_string($startdate);
-            }
-            my $calendar = Koha::Calendar->new( branchcode => $branch );
-            $datedue = $calendar->addDate( $startdate, $dur, $loanlength->{lengthunit} );
-            if ($loanlength->{lengthunit} eq 'days') {
-                $datedue->set_hour(23);
-                $datedue->set_minute(59);
-            }
+        my $dur;
+        if ($loanlength->{lengthunit} eq 'hours') {
+            $dur = DateTime::Duration->new( hours => $loanlength->{$length_key});
+        }
+        else { # days
+            $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
+        }
+        if (ref $startdate ne 'DateTime' ) {
+            $startdate = dt_from_string($startdate);
+        }
+        my $calendar = Koha::Calendar->new( branchcode => $branch );
+        $datedue = $calendar->addDate( $startdate, $dur, $loanlength->{lengthunit} );
+        if ($loanlength->{lengthunit} eq 'days') {
+            $datedue->set_hour(23);
+            $datedue->set_minute(59);
         }
     }
 
index 80771ae..bc1c7a7 100644 (file)
@@ -72,10 +72,6 @@ BEGIN {
        push @EXPORT, qw(
         &GetIssuesIteminfo
        );
-    #
-       # &GetIssuingRules - delete.
-       # use C4::Circulation::GetIssuingRule instead.
-       
        # subs to move to Members.pm
        push @EXPORT, qw(
         &CheckBorrowerDebarred
@@ -696,41 +692,6 @@ sub GetFine {
     return 0;
 }
 
-
-=head2 GetIssuingRules
-
-FIXME - This sub should be deprecated and removed.
-It ignores branch and defaults.
-
-    $data = &GetIssuingRules($itemtype,$categorycode);
-
-Looks up for all issuingrules an item info 
-
-C<$itemnumber> is a reference-to-hash whose keys are all of the fields
-from the borrowers and categories tables of the Koha database. Thus,
-
-C<$categorycode> contains  information about borrowers category 
-
-C<$data> contains all information about both the borrower and
-category he or she belongs to.
-=cut 
-
-sub GetIssuingRules {
-       warn "GetIssuingRules is deprecated: use GetIssuingRule from C4::Circulation instead.";
-   my ($itemtype,$categorycode)=@_;
-   my $dbh   = C4::Context->dbh();    
-   my $query=qq|SELECT *
-        FROM issuingrules
-        WHERE issuingrules.itemtype=?
-            AND issuingrules.categorycode=?
-        |;
-    my $sth = $dbh->prepare($query);
-    #  print $query;
-    $sth->execute($itemtype,$categorycode);
-    return $sth->fetchrow_hashref;
-}
-
-
 sub ReplacementCost2 {
     my ( $itemnum, $borrowernumber ) = @_;
     my $dbh   = C4::Context->dbh();
index c221d71..5d4166d 100755 (executable)
@@ -101,8 +101,8 @@ elsif ($op eq 'delete-branch-item') {
 # save the values entered
 elsif ($op eq 'add') {
     my $sth_search = $dbh->prepare('SELECT COUNT(*) AS total FROM issuingrules WHERE branchcode=? AND categorycode=? AND itemtype=?');
-    my $sth_insert = $dbh->prepare('INSERT INTO issuingrules (branchcode, categorycode, itemtype, maxissueqty, renewalsallowed, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, firstremind, chargeperiod,rentaldiscount, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)');
-    my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, reservesallowed=?, issuelength=?, lengthunit = ?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, overduefinescap=?  WHERE branchcode=? AND categorycode=? AND itemtype=?");
+    my $sth_insert = $dbh->prepare('INSERT INTO issuingrules (branchcode, categorycode, itemtype, maxissueqty, renewalsallowed, renewalperiod, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, firstremind, chargeperiod,rentaldiscount, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)');
+    my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, renewalperiod=?, reservesallowed=?, issuelength=?, lengthunit = ?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, overduefinescap=?  WHERE branchcode=? AND categorycode=? AND itemtype=?");
     
     my $br = $branch; # branch
     my $bor  = $input->param('categorycode'); # borrower category
@@ -113,6 +113,7 @@ elsif ($op eq 'add') {
     my $chargeperiod = $input->param('chargeperiod');
     my $maxissueqty  = $input->param('maxissueqty');
     my $renewalsallowed  = $input->param('renewalsallowed');
+    my $renewalperiod    = $input->param('renewalperiod');
     my $reservesallowed  = $input->param('reservesallowed');
     $maxissueqty =~ s/\s//g;
     $maxissueqty = undef if $maxissueqty !~ /^\d+/;
@@ -128,9 +129,9 @@ elsif ($op eq 'add') {
     $sth_search->execute($br,$bor,$cat);
     my $res = $sth_search->fetchrow_hashref();
     if ($res->{total}) {
-        $sth_update->execute($fine, $finedays,$firstremind, $chargeperiod, $maxissueqty, $renewalsallowed,$reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount,$overduefinescap, $br,$bor,$cat);
+        $sth_update->execute($fine, $finedays,$firstremind, $chargeperiod, $maxissueqty, $renewalsallowed, $renewalperiod, $reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount,$overduefinescap, $br,$bor,$cat);
     } else {
-        $sth_insert->execute($br,$bor,$cat,$maxissueqty,$renewalsallowed,$reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays,$firstremind,$chargeperiod,$rentaldiscount,$overduefinescap);
+        $sth_insert->execute($br,$bor,$cat,$maxissueqty,$renewalsallowed, $renewalperiod, $reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays,$firstremind,$chargeperiod,$rentaldiscount,$overduefinescap);
     }
 } 
 elsif ($op eq "set-branch-defaults") {
index 15eb4d7..7b5da3f 100644 (file)
@@ -1017,6 +1017,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules
   `hardduedate` date default NULL, -- hard due date
   `hardduedatecompare` tinyint NOT NULL default "0", -- type of hard due date (1 = after, 0 = on, -1 = before)
   `renewalsallowed` smallint(6) NOT NULL default "0", -- how many renewals are allowed
+  `renewalperiod` int(4) default NULL -- renewal period in the unit set in issuingrules.lengthunit
   `reservesallowed` smallint(6) NOT NULL default "0", -- how many holds are allowed
   `branchcode` varchar(10) NOT NULL default '', -- the branch this rule is for (branches.branchcode)
   overduefinescap decimal default NULL, -- the maximum amount of an overdue fine
index 5c7b323..747c0d8 100755 (executable)
@@ -6526,6 +6526,7 @@ if ( CheckVersion($DBversion) ) {
 $DBversion = "3.11.00.100";
 if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
     print "Upgrade to $DBversion done (3.12-alpha release)\n";
+   SetVersion ($DBversion);
 }
 
 $DBversion = "3.11.00.101";
@@ -6697,6 +6698,18 @@ if ( CheckVersion($DBversion) ) {
         $sth_update->execute($row->{content}, $row->{module}, $row->{code}, $row->{branchcode});
     }
     print "Upgrade to $DBversion done (use new <<items.fine>> syntax in notices)\n";
+    SetVersion($DBversion);
+}
+
+$DBversion = "3.11.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do(qq{
+        ALTER TABLE issuingrules ADD COLUMN renewalperiod int(4) DEFAULT NULL AFTER renewalsallowed
+    });
+    $dbh->do(qq{
+        UPDATE issuingrules SET renewalperiod = issuelength
+    });
+    print "Upgrade to $DBversion done (Bug 8365: Add colum issuingrules.renewalperiod)\n";
     SetVersion ($DBversion);
 }
 
index bf7242e..0326cf6 100644 (file)
@@ -149,6 +149,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                 <th>Overdue Fines Cap ($)</th>
                 <th>Suspension in days (day)</th>
                 <th>Renewals allowed (count)</th>
+                <th>Renewals period</th>
                 <th>Holds allowed (count)</th>
                 <th>Rental discount (%)</th>
                 <th colspan="2">&nbsp;</th>
@@ -205,6 +206,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                             <td>[% rule.overduefinescap FILTER format("%.2f") %]</td>
                                                        <td>[% rule.finedays %]</td>
                                                        <td>[% rule.renewalsallowed %]</td>
+                            <td>[% rule.renewalperiod %]</td>
                                                        <td>[% rule.reservesallowed %]</td>
                                                        <td>[% rule.rentaldiscount %]</td>
                             <td><a href="#" class="editrule">Edit</a></td>
@@ -253,6 +255,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
                     <td><input type="text" name="overduefinescap" id="overduefinescap" size="6" /> </td>
                     <td><input type="text" name="finedays" id="fined" size="3" /> </td>
                     <td><input type="text" name="renewalsallowed" id="renewalsallowed" size="2" /></td>
+                    <td><input type="text" name="renewalperiod" id="renewalperiod" size="3" /></td>
                     <td><input type="text" name="reservesallowed" id="reservesallowed" size="2" /></td>
                     <td><input type="text" name="rentaldiscount" id="rentaldiscount" size="2" /></td>
                     <td colspan="2">
index 13eb1f2..07d9d41 100644 (file)
@@ -23,7 +23,6 @@ sub methods : Test( 1 ) {
                        BorType 
                        ReplacementCost 
                        GetFine 
-                       GetIssuingRules 
                        ReplacementCost2 
                        GetNextIdNotify 
                        NumberNotifyId