Bugzilla – Attachment 13972 Details for
Bug 8365
Renewal duration in issuingules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8365: Add a renewal duration in the issuing rules
Bug-8365-Add-a-renewal-duration-in-the-issuing-rul.patch (text/plain), 19.39 KB, created by
Jonathan Druart
on 2012-12-10 10:03:54 UTC
(
hide
)
Description:
Bug 8365: Add a renewal duration in the issuing rules
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2012-12-10 10:03:54 UTC
Size:
19.39 KB
patch
obsolete
>From 2824ba832a69daf05958cea888067042649994b0 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Fri, 7 Sep 2012 16:31:44 +0200 >Subject: [PATCH] Bug 8365: Add a renewal duration in the issuing rules > >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) >--- > C4/Circulation.pm | 181 ++++++++------------ > C4/Overdues.pm | 39 ----- > admin/smart-rules.pl | 9 +- > installer/data/mysql/kohastructure.sql | 1 + > installer/data/mysql/updatedatabase.pl | 12 ++ > .../prog/en/modules/admin/smart-rules.tt | 3 + > t/db_dependent/lib/KohaTest/Overdues.pm | 1 - > 7 files changed, 93 insertions(+), 153 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index afe7a6a..4e3499c 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1321,13 +1321,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 >@@ -1371,6 +1376,7 @@ sub GetLoanLength { > # if no rule is set => 21 days (hardcoded) > return { > issuelength => 21, >+ renewalperiod => 21, > lengthunit => 'days', > }; > >@@ -1405,7 +1411,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. >@@ -2427,65 +2433,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 ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); >- if ($resfound) { >- $renewokay = 0; >- $error="on_reserve" >- } >+ my $branchcode = _GetCircControlBranch($item, $borrower); > >+ 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 ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber); >+ if ( $resfound ) { >+ $renewokay = 0; >+ $error = "on_reserve"; >+ } >+ >+ return ( $renewokay, $error ); > } > > =head2 AddRenewal >@@ -2546,7 +2516,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 >@@ -2993,58 +2963,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); > } > } > >diff --git a/C4/Overdues.pm b/C4/Overdues.pm >index ac66c36..fc9e95a 100644 >--- a/C4/Overdues.pm >+++ b/C4/Overdues.pm >@@ -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 >@@ -692,41 +688,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(); >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index c221d71..5d4166d 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -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") { >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index f68c9c7..e06fa9d 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1016,6 +1016,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 >diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl >index 676bb3e..6f77626 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -6134,6 +6134,18 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { > SetVersion ($DBversion); > } > >+$DBversion = '3.09.00.XXX'; >+if (C4::Context->preference("Version") < TransformToNum($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 (Add colum issuingrules.renewalperiod)\n"; >+ SetVersion ($DBversion); >+} >+ > =head1 FUNCTIONS > > =head2 TableExists($table) >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >index 586d72f..d33a243 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >@@ -75,6 +75,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> </th> >@@ -120,6 +121,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> >@@ -166,6 +168,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <td><input name="overduefinescap" size="6" /> </td> > <td><input name="finedays" size="3" /> </td> > <td><input name="renewalsallowed" size="2" /></td> >+ <td><input name="renewalperiod" size="3" /></td> > <td><input name="reservesallowed" size="2" /></td> > <td><input name="rentaldiscount" size="2" /></td> > <td><input type="hidden" name="branch" value="[% current_branch %]"/><input type="submit" value="Add" class="submit" /></td> >diff --git a/t/db_dependent/lib/KohaTest/Overdues.pm b/t/db_dependent/lib/KohaTest/Overdues.pm >index 13eb1f2..07d9d41 100644 >--- a/t/db_dependent/lib/KohaTest/Overdues.pm >+++ b/t/db_dependent/lib/KohaTest/Overdues.pm >@@ -23,7 +23,6 @@ sub methods : Test( 1 ) { > BorType > ReplacementCost > GetFine >- GetIssuingRules > ReplacementCost2 > GetNextIdNotify > NumberNotifyId >-- >1.7.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=13972">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8365
:
12054
|
12374
|
13972
|
14142
|
15300
|
15301
|
15302
|
15303
|
15360
|
15361
|
16120
|
16121
|
16213
|
16214
|
16382
|
16386
|
16396
|
16433
|
16434
|
16435
|
16436