From 8ad00bc02ff23e0dad6661da9ff6b11745cda6f7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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) Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer Test comments on second patch. Signed-off-by: Jared Camins-Esakov --- C4/Circulation.pm | 179 +++++++----------- C4/Overdues.pm | 39 ---- admin/smart-rules.pl | 9 +- installer/data/mysql/kohastructure.sql | 1 + installer/data/mysql/updatedatabase.pl | 13 ++ .../prog/en/modules/admin/smart-rules.tt | 3 + t/db_dependent/lib/KohaTest/Overdues.pm | 1 - 7 files changed, 94 insertions(+), 151 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index bb5b718233..425e16ca3e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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); } } diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 80771ae47f..bc1c7a77db 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 @@ -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(); diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index c221d7107a..5d4166ddaa 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 15eb4d7f1b..7b5da3fe50 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -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 diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 5c7b3230ce..747c0d8b54 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -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 <> 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); } 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 bf7242eba3..0326cf6a5e 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 @@ -149,6 +149,7 @@ for="tobranch">Clone these rules to:   @@ -205,6 +206,7 @@ for="tobranch">Clone these rules to: [% rule.finedays %] [% rule.renewalsallowed %] + [% rule.renewalperiod %] [% rule.reservesallowed %] [% rule.rentaldiscount %] Edit @@ -253,6 +255,7 @@ for="tobranch">Clone these rules to: + diff --git a/t/db_dependent/lib/KohaTest/Overdues.pm b/t/db_dependent/lib/KohaTest/Overdues.pm index 13eb1f23dd..07d9d41318 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 -- 2.39.5