@@ -, +, @@ granular level - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. - a new function C4::Reserves::GetLastPickupDate() to calculate the proper date from Calendar. - the ReservesMaxPickUpDelay syspref. - the ExpireReservesOnHolidays syspref, because it became obsolete due to the changes in handling the last pickup date with Koha Calendar. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO figure out how to rename existing letter placeholders <> with <> Otherwise existing reserves won't get UPDATEd with the new lastpickupdate-values. ****** ****** ****** ****** (circ/circulation.pl, circ/waitingreserves.pl, members/moremember.pl) equal to the waiting date + the "holds pickup wait" defined moved to the next open library day. ****** ****** ****** ****** somewhere. the (current date + the 'Holds pickup wait' in your issuing rules) in days. ** ** ** ** ** ** ** ** ** ** ** ** location single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. should now be increased by four days. ** ** ** ** ** ** ** ** ** ** ** ** somewhere. lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. --- C4/Reserves.pm | 75 +++++++- admin/smart-rules.pl | 1 + circ/waitingreserves.pl | 2 + installer/data/mysql/kohastructure.sql | 3 + installer/data/mysql/sysprefs.sql | 4 +- installer/data/mysql/updatedatabase.pl | 30 +++ koha-tmpl/intranet-tmpl/prog/en/js/holds.js | 148 +++++++++++++++ .../en/modules/admin/preferences/circulation.pref | 9 +- .../prog/en/modules/admin/smart-rules.tt | 3 + .../prog/en/modules/circ/circulation.tt | 1 + .../prog/en/modules/circ/waitingreserves.tt | 4 +- .../prog/en/modules/help/admin/smart-rules.tt | 165 ++++++++++++++++ .../prog/en/modules/help/circ/waitingreserves.tt | 11 ++ .../opac-tmpl/bootstrap/en/modules/opac-user.tt | 5 +- svc/holds | 7 +- t/db_dependent/Holds.t | 210 ++++++++++++++++++++- 16 files changed, 651 insertions(+), 27 deletions(-) create mode 100644 koha-tmpl/intranet-tmpl/prog/en/js/holds.js create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/smart-rules.tt create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/help/circ/waitingreserves.tt --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -186,12 +186,16 @@ sub AddReserve { # Make room in reserves for this before those of a later reserve date $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority ); } + my ($waitingdate, $lastpickupdate); - my $waitingdate; - + my $item = C4::Items::GetItem( $checkitem ); # If the reserv had the waiting status, we had the value of the resdate if ( $found eq 'W' ) { $waitingdate = $resdate; + + #The reserve-object doesn't exist yet in DB, so we must supply what information we have to GetLastPickupDate() so it can do it's work. + my $reserve = {borrowernumber => $borrowernumber, waitingdate => $waitingdate, branchcode => $branch}; + $lastpickupdate = GetLastPickupDate( $reserve, $item ); } # Don't add itemtype limit if specific item is selected @@ -211,6 +215,7 @@ sub AddReserve { waitingdate => $waitingdate, expirationdate => $expdate, itemtype => $itemtype, + lastpickupdate => $lastpickupdate, } )->store(); $hold->set_waiting() if $found eq 'W'; @@ -1004,9 +1009,29 @@ sub ModReserveStatus { my ($itemnumber, $newstatus) = @_; my $dbh = C4::Context->dbh; - my $query = "UPDATE reserves SET found = ?, waitingdate = NOW() WHERE itemnumber = ? AND found IS NULL AND priority = 0"; + my $now = dt_from_string; + my $reserve = $dbh->selectrow_hashref(q{ + SELECT * + FROM reserves + WHERE itemnumber = ? + AND found IS NULL + AND priority = 0 + }, {}, $itemnumber); + return unless $reserve; + + my $lastpickupdate = GetLastPickupDate( $reserve ); + + my $query = q{ + UPDATE reserves + SET found = ?, + waitingdate = ?, + lastpickupdate = ? + WHERE itemnumber = ? + AND found IS NULL + AND priority = 0 + }; my $sth_set = $dbh->prepare($query); - $sth_set->execute( $newstatus, $itemnumber ); + $sth_set->execute( $newstatus, $now, $lastpickupdate, $itemnumber ); if ( C4::Context->preference("ReturnToShelvingCart") && $newstatus ) { CartToShelf( $itemnumber ); @@ -1503,6 +1528,7 @@ sub _Findgroupreserve { reserves.borrowernumber AS borrowernumber, reserves.reservedate AS reservedate, reserves.branchcode AS branchcode, + reserves.lastpickupdate AS lastpickupdate, reserves.cancellationdate AS cancellationdate, reserves.found AS found, reserves.reservenotes AS reservenotes, @@ -1538,6 +1564,7 @@ sub _Findgroupreserve { reserves.borrowernumber AS borrowernumber, reserves.reservedate AS reservedate, reserves.branchcode AS branchcode, + reserves.lastpickupdate AS lastpickupdate, reserves.cancellationdate AS cancellationdate, reserves.found AS found, reserves.reservenotes AS reservenotes, @@ -1573,6 +1600,7 @@ sub _Findgroupreserve { reserves.reservedate AS reservedate, reserves.waitingdate AS waitingdate, reserves.branchcode AS branchcode, + reserves.lastpickupdate AS lastpickupdate, reserves.cancellationdate AS cancellationdate, reserves.found AS found, reserves.reservenotes AS reservenotes, @@ -1795,6 +1823,42 @@ sub MoveReserve { } } +=head MoveWaitingdate + + #Move waitingdate two months and fifteen days forward. + my $dateDuration = DateTime::Duration->new( months => 2, days => 15 ); + $reserve = MoveWaitingdate( $reserve, $dateDuration); + + #Move waitingdate one year and eleven days backwards. + my $dateDuration = DateTime::Duration->new( years => -1, days => -11 ); + $reserve = MoveWaitingdate( $reserve, $dateDuration); + +Moves the waitingdate and updates the lastpickupdate to match. +If waitingdate is not defined, uses today. +Is intended to be used from automated tests, because under normal library +operations there should be NO REASON to move the waitingdate. + +@PARAM1 koha.reserves-row, with waitingdate set. +@PARAM2 DateTime::Duration, with the desired offset. +RETURNS koha.reserve-row, with keys waitingdate and lastpickupdate updated. +=cut +sub MoveWaitingdate { + my ($reserve, $dateDuration) = @_; + + my $dt = dt_from_string( $reserve->{waitingdate} ); + $dt->add_duration( $dateDuration ); + $reserve->{waitingdate} = $dt->ymd(); + + GetLastPickupDate( $reserve ); #Update the $reserve->{lastpickupdate} + + #UPDATE the DB part + my $dbh = C4::Context->dbh(); + my $sth = $dbh->prepare( "UPDATE reserves SET waitingdate=?, lastpickupdate=? WHERE reserve_id=?" ); + $sth->execute( $reserve->{waitingdate}, $reserve->{lastpickupdate}, $reserve->{reserve_id} ); + + return $reserve; +} + =head2 MergeHolds MergeHolds($dbh,$to_biblio, $from_biblio); @@ -1891,7 +1955,8 @@ sub RevertWaitingStatus { SET priority = 1, found = NULL, - waitingdate = NULL + waitingdate = NULL, + lastpickupdate = NULL, WHERE reserve_id = ? "; --- a/admin/smart-rules.pl +++ a/admin/smart-rules.pl @@ -147,6 +147,7 @@ elsif ($op eq 'add') { $no_auto_renewal_after_hard_limit = output_pref( { dt => $no_auto_renewal_after_hard_limit, dateonly => 1, dateformat => 'iso' } ) if ( $no_auto_renewal_after_hard_limit ); my $reservesallowed = $input->param('reservesallowed'); my $holds_per_record = $input->param('holds_per_record'); + my $holdspickupwait = $input->param('holdspickupwait'); my $onshelfholds = $input->param('onshelfholds') || 0; $maxissueqty =~ s/\s//g; $maxissueqty = undef if $maxissueqty !~ /^\d+/; --- a/circ/waitingreserves.pl +++ a/circ/waitingreserves.pl @@ -115,6 +115,7 @@ while ( my $hold = $holds->next ) { borrowername => $patron->surname, # FIXME Let's send $patron to the template borrowerfirstname => $patron->firstname, borrowerphone => $patron->phone, + lastpickupdate => $hold->lastpickupdate, ); my $itemtype = Koha::ItemTypes->find( $item->effective_itemtype ); @@ -158,6 +159,7 @@ $template->param( show_date => output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), ReservesMaxPickUpDelay => $max_pickup_delay, tab => $tab, + show_date => format_date(C4::Dates->today('iso')), ); # Checking if there is a Fast Cataloging Framework --- a/installer/data/mysql/kohastructure.sql +++ a/installer/data/mysql/kohastructure.sql @@ -860,6 +860,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules `no_auto_renewal_after_hard_limit` date default NULL, -- no auto renewal allowed after a given date `reservesallowed` smallint(6) NOT NULL default "0", -- how many holds are allowed `holds_per_record` SMALLINT(6) NOT NULL DEFAULT 1, -- How many holds a patron can have on a given bib + `holdspickupwait` int(11) default NULL, -- How many open library days a hold can wait in the pickup shelf until it becomes problematic `branchcode` varchar(10) NOT NULL default '', -- the branch this rule is for (branches.branchcode) overduefinescap decimal(28,6) default NULL, -- the maximum amount of an overdue fine cap_fine_to_replacement_price BOOLEAN NOT NULL DEFAULT '0', -- cap the fine based on item's replacement price @@ -1882,6 +1883,7 @@ CREATE TABLE `reserves` ( -- information related to holds/reserves in Koha `suspend` BOOLEAN NOT NULL DEFAULT 0, `suspend_until` DATETIME NULL DEFAULT NULL, `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- If record level hold, the optional itemtype of the item the patron is requesting + `lastpickupdate` date NULL DEFAULT NULL, -- the last day this hold is available for pickup, until it becomes problematic PRIMARY KEY (`reserve_id`), KEY priorityfoundidx (priority,found), KEY `borrowernumber` (`borrowernumber`), @@ -1921,6 +1923,7 @@ CREATE TABLE `old_reserves` ( -- this table holds all holds/reserves that have b `suspend` BOOLEAN NOT NULL DEFAULT 0, -- in this hold suspended (1 for yes, 0 for no) `suspend_until` DATETIME NULL DEFAULT NULL, -- the date this hold is suspended until (NULL for infinitely) `itemtype` VARCHAR(10) NULL DEFAULT NULL, -- If record level hold, the optional itemtype of the item the patron is requesting + `lastpickupdate` date NULL DEFAULT NULL, -- the last day this hold is available for pickup, until it becomes problematic PRIMARY KEY (`reserve_id`), KEY `old_reserves_borrowernumber` (`borrowernumber`), KEY `old_reserves_biblionumber` (`biblionumber`), --- a/installer/data/mysql/sysprefs.sql +++ a/installer/data/mysql/sysprefs.sql @@ -161,11 +161,10 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'), ('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'), ('ExpireReservesMaxPickUpDelayCharge','0',NULL,'If ExpireReservesMaxPickUpDelay is enabled, and this field has a non-zero value, than a borrower whose waiting hold has expired will be charged this amount.','free'), -('ExpireReservesOnHolidays', '1', NULL, 'If false, reserves at a library will not be canceled on days the library is not open.', 'YesNo'), ('ExcludeHolidaysFromMaxPickUpDelay', '0', NULL, 'If ON, reserves max pickup delay takes into accountthe closed days.', 'YesNo'), ('ExportCircHistory', 0, NULL, "Display the export circulation options", 'YesNo' ), ('ExportRemoveFields','',NULL,'List of fields for non export in circulation.pl (separated by a space)','Free'), -('ExtendedPatronAttributes','1',NULL,'Use extended patron IDs and attributes','YesNo'), +('ExtendedPatronAttributes','0',NULL,'Use extended patron IDs and attributes','YesNo'), ('FacetLabelTruncationLength','20',NULL,'Specify the facet max length in OPAC','Integer'), ('FacetMaxCount','20',NULL,'Specify the max facet count for each category','Integer'), ('FailedLoginAttempts','','','Number of login attempts before lockout the patron account','Integer'), @@ -470,7 +469,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('RequestOnOpac','1',NULL,'If ON, globally enables patron holds on OPAC','YesNo'), ('RequireStrongPassword','1','','Require a strong login password for staff and patrons','YesNo'), ('ReservesControlBranch','PatronLibrary','ItemHomeLibrary|PatronLibrary','Branch checked for members reservations rights','Choice'), -('ReservesMaxPickUpDelay','7','','Define the Maximum delay to pick up an item on hold','Integer'), ('ReservesNeedReturns','1','','If ON, a hold placed on an item available in this library must be checked-in, otherwise, a hold on a specific item, that is in the library & available is considered available','YesNo'), ('RESTdefaultPageSize','20','','Default page size for endpoints listing objects','Integer'), ('RESTOAuth2ClientCredentials','0',NULL,'If enabled, the OAuth2 client credentials flow is enabled for the REST API.','YesNo'), --- a/installer/data/mysql/updatedatabase.pl +++ a/installer/data/mysql/updatedatabase.pl @@ -38,6 +38,8 @@ use Getopt::Long; # Koha modules use C4::Context; use C4::Installer; +use C4::Reserves; +use DateTime::Duration; use Koha::Database; use Koha; use Koha::DateUtils; @@ -16474,6 +16476,34 @@ if( CheckVersion( $DBversion ) ) { print "Upgrade to $DBversion done (Bug 17602 - Integrate support for OneClickdigital/Recorded Books API)\n"; } +$DBversion = "18.06.00.XXX"; +if ( CheckVersion($DBversion) ) { + my $maxpickupdelay = C4::Context->preference('ReservesMaxPickUpDelay') || 0; #MaxPickupDelay + $dbh->do(q{ DELETE FROM systempreferences WHERE variable='ReservesMaxPickUpDelay'; }); + $dbh->do(q{ DELETE FROM systempreferences WHERE variable='ExpireReservesOnHolidays'; }); + # //DELETE FROM systempreferences WHERE variable='ExpireReservesMaxPickUpDelay'; #This syspref is not needed and would be better suited to be calculated from the holdspickupwait + # //ExpireReservesMaxPickUpDelayCharge #This could be added as a column to the issuing rules. + $dbh->do(q{ ALTER TABLE issuingrules ADD COLUMN holdspickupwait INT(11) NULL default NULL AFTER reservesallowed; }); + $dbh->do(q{ ALTER TABLE reserves ADD COLUMN lastpickupdate DATE NULL default NULL AFTER suspend_until; }); + $dbh->do(q{ ALTER TABLE old_reserves ADD COLUMN lastpickupdate DATE NULL default NULL AFTER suspend_until; }); + my $sth = $dbh->prepare(q{ + UPDATE issuingrules SET holdspickupwait = ? + }); + $sth->execute( $maxpickupdelay ) if $maxpickupdelay; #Don't want to accidentally nullify all! + + ##Populate the lastpickupdate-column from existing 'ReservesMaxPickUpDelay' + print "Populating the new lastpickupdate-column for all waiting holds. This might take a while.\n"; + $sth = $dbh->prepare(q{ SELECT * FROM reserves WHERE found = 'W'; }); + $sth->execute( ); + my $dtdur = DateTime::Duration->new( days => 0 ); + + while ( my $res = $sth->fetchrow_hashref ) { + C4::Reserves::MoveWaitingdate( $res, $dtdur ); #We call MoveWaitingdate with a 0 duration to simply recalculate the lastpickupdate and store the new values to DB. + } + print "Upgrade to $DBversion done (8367: Add colum issuingrules.holdspickupwait and reserves.lastpickupdate. Populates introduced columns from the expiring ReservesMaxPickUpDelay. Deletes the ReservesMaxPickUpDelay and ExpireReservesOnHolidays -sysprefs)\n"; + SetVersion($DBversion); +} + # SEE bug 13068 # if there is anything in the atomicupdate, read and execute it. --- a/koha-tmpl/intranet-tmpl/prog/en/js/holds.js +++ a/koha-tmpl/intranet-tmpl/prog/en/js/holds.js @@ -0,0 +1,148 @@ +$(document).ready(function() { + var holdsTable; + + // Don't load holds table unless it is clicked on + $("#holds-tab").on( "click", function(){ load_holds_table() } ); + + // If the holds tab is preselected on load, we need to load the table + if ( $("#holds-tab").parent().hasClass('ui-state-active') ) { load_holds_table() } + + function load_holds_table() { + if ( ! holdsTable ) { + holdsTable = $("#holds-table").dataTable({ + "bAutoWidth": false, + "sDom": "rt", + "aoColumns": [ + { + "mDataProp": "reservedate_formatted" + }, + { + "mDataProp": "lastpickupdate_formatted" + }, + { + "mDataProp": function ( oObj ) { + title = "" + + oObj.title; + + $.each(oObj.subtitle, function( index, value ) { + title += " " + value.subfield; + }); + + title += ""; + + if ( oObj.author ) { + title += " " + BY.replace( "_AUTHOR_", oObj.author ); + } + + if ( oObj.itemnotes ) { + var span_class = ""; + if ( $.datepicker.formatDate('yy-mm-dd', new Date(oObj.issuedate) ) == ymd ) { + span_class = "circ-hlt"; + } + title += " - " + oObj.itemnotes + "" + } + + return title; + } + }, + { + "mDataProp": function( oObj ) { + return oObj.itemcallnumber || ""; + } + }, + { + "mDataProp": function( oObj ) { + var data = ""; + + if ( oObj.suspend == 1 ) { + data += "

" + HOLD_IS_SUSPENDED; + if ( oObj.suspend_until ) { + data += " " + UNTIL.format( oObj.suspend_until_formatted ); + } + data += "

"; + } + + if ( oObj.barcode ) { + data += ""; + if ( oObj.found == "W" ) { + + if ( oObj.waiting_here ) { + data += ITEM_IS_WAITING_HERE; + } else { + data += ITEM_IS_WAITING; + data += " " + AT.format( oObj.waiting_at ); + } + + } else if ( oObj.transferred ) { + data += ITEM_IS_IN_TRANSIT.format( oObj.from_branch, oObj.date_sent ); + } else if ( oObj.not_transferred ) { + data += NOT_TRANSFERRED_YET.format( oObj.not_transferred_by ); + } + data += ""; + + data += " " + + oObj.barcode + + ""; + } + + return data; + } + }, + { + "mDataProp": function( oObj ) { + return oObj.branchcode || ""; + } + }, + { "mDataProp": "expirationdate_formatted" }, + { + "mDataProp": function( oObj ) { + if ( oObj.priority && parseInt( oObj.priority ) && parseInt( oObj.priority ) > 0 ) { + return oObj.priority; + } else { + return ""; + } + } + }, + { + "bSortable": false, + "mDataProp": function( oObj ) { + return "" + + "" + + "" + + ""; + } + } + ], + "bPaginate": false, + "bProcessing": true, + "bServerSide": false, + "sAjaxSource": '/cgi-bin/koha/svc/holds', + "fnServerData": function ( sSource, aoData, fnCallback ) { + aoData.push( { "name": "borrowernumber", "value": borrowernumber } ); + + $.getJSON( sSource, aoData, function (json) { + fnCallback(json) + } ); + }, + }); + + if ( $("#holds-table").length ) { + $("#holds-table_processing").position({ + of: $( "#holds-table" ), + collision: "none" + }); + } + } + } +}); --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -589,18 +589,13 @@ Circulation: choices: ItemHomeLibrary: "item's home library" PatronLibrary: "patron's home library" - - to see if the patron can place a hold on the item. - - - - Mark a hold as problematic if it has been waiting for more than - - pref: ReservesMaxPickUpDelay - class: integer - - days. + - to see if the patron can place a hold on the item. - - pref: ExpireReservesMaxPickUpDelay choices: yes: Allow no: "Don't allow" - - "holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay" + - "holds to expire automatically if they have not been picked by within the time period specified in hold pickup delay defined in the issuing rules" - - If using ExpireReservesMaxPickUpDelay, charge a borrower who allows his or her waiting hold to expire a fee of - pref: ExpireReservesMaxPickUpDelayCharge --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -83,6 +83,7 @@ Suspension charging interval Renewals allowed (count) Renewal period + Holds wait for pickup (day) No renewal before Automatic renewal No automatic renewal after @@ -276,6 +277,7 @@ + [% ELSE %] --- a/svc/holds +++ a/svc/holds @@ -48,7 +48,7 @@ my $branch = C4::Context->userenv->{'branch'}; my $schema = Koha::Database->new()->schema(); my @sort_columns = - qw/reservedate title itemcallnumber barcode expirationdate priority/; + qw/reservedate lastpickupdate title itemcallnumber barcode expirationdate priority/; my $borrowernumber = $input->param('borrowernumber'); my $offset = $input->param('iDisplayStart'); @@ -93,6 +93,7 @@ while ( my $h = $holds_rs->next() ) { branchcode => $h->branch()->branchname(), branches => $libraries, reservedate => $h->reservedate(), + lastpickupdate => $h->lastpickupdate(), expirationdate => $h->expirationdate(), suspend => $h->suspend(), suspend_until => $h->suspend_until(), @@ -111,6 +112,10 @@ while ( my $h = $holds_rs->next() ) { { dt => dt_from_string( $h->reservedate() ), dateonly => 1 } ) : q{}, + lastpickupdate_formatted => $h->lastpickupdate() ? output_pref( + { dt => dt_from_string( $h->lastpickupdate() ), dateonly => 1 } + ) + : q{}, suspend_until_formatted => $h->suspend_until() ? output_pref( { dt => dt_from_string( $h->suspend_until() ), dateonly => 1 } ) --- a/t/db_dependent/Holds.t +++ a/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 56; +use Test::More tests => 59; use MARC::Record; use Koha::Patrons; use C4::Items; @@ -382,9 +382,135 @@ is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, "CanItemBeReserved should return 'cannotReserveFromOtherBranches'"); ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem( - { homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $bibnum); -is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'OK', - "CanItemBeReserved should return 'OK'"); + { homebranch => 'CPL', holdingbranch => 'CPL', itype => 'CAN' } , $bibnum); +is(CanItemBeReserved($borrowernumbers[0], $itemnumber), 'OK', + "CanItemBeReserved should returns 'OK'"); + +##Setting duration variables +my $now = DateTime->now(); +my $minus4days = DateTime::Duration->new(days => -4); +my $minus1days = DateTime::Duration->new(days => -1); +my $plus1days = DateTime::Duration->new(days => 1); +my $plus4days = DateTime::Duration->new(days => 4); +##Setting some test prerequisites testing environment +C4::Context->set_preference( 'ExpireReservesMaxPickUpDelay', 1 ); +setSimpleCircPolicy(); +setCalendars(); +#Running more tests +testGetLastPickupDate(); +testMoveWaitingdate(); +testCancelExpiredReserves(); +C4::Context->set_preference( 'ExpireReservesMaxPickUpDelay', 0 ); + +## Environment should be the following +## Holidays: days from today; -2,-3,-4 +sub testCancelExpiredReserves { + $dbh->do('UPDATE issuingrules SET holdspickupwait = 1'); #Make holds problematize after 1 day + + $reserves = $dbh->selectall_arrayref('SELECT * FROM reserves WHERE found IS NULL', { Slice => {} }); + $reserve = $reserves->[0]; + #Catch this hold and make it Waiting for pickup today. + C4::Reserves::ModReserveAffect( $reserve->{itemnumber}, $reserve->{borrowernumber} ); + $reserve = C4::Reserves::GetReserve( $reserve->{reserve_id} ); #UPDATE DB changes to local scope. + + CancelExpiredReserves(); + my $count = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE reserve_id = ?", undef, $reserve->{reserve_id} ); + is( $count, 1, "Waiting reserve with lastpickupdate for ".$reserve->{lastpickupdate}." not canceled" ); + + C4::Reserves::MoveWaitingdate( $reserve, DateTime::Duration->new(days => -4) ); + CancelExpiredReserves(); + $count = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE reserve_id = ?", undef, $reserve->{reserve_id} ); + is( $count, 0, "Waiting reserve with lastpickupdate for ".$reserve->{lastpickupdate}." totally canceled" ); + + # Test expirationdate + $reserve = $reserves->[1]; + $dbh->do("UPDATE reserves SET expirationdate = DATE_SUB( NOW(), INTERVAL 1 DAY ) WHERE reserve_id = ?", undef, $reserve->{reserve_id} ); + CancelExpiredReserves(); + $count = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE reserve_id = ?", undef, $reserve->{reserve_id} ); + is( $count, 0, "Reserve with manual expiration date canceled correctly" ); + + #This test verifies that reserves with holdspickupwait disabled are not ćanceled! + $dbh->do('UPDATE issuingrules SET holdspickupwait = 0'); #Make holds never problematize + $reserve = $reserves->[2]; + #Catch this hold and make it Waiting for pickup today. + C4::Reserves::ModReserveAffect( $reserve->{itemnumber}, $reserve->{borrowernumber} ); + $reserve = C4::Reserves::GetReserve( $reserve->{reserve_id} ); #UPDATE DB changes to local scope. + #Move the caught reserve 4 days to past and try to cancel it. + C4::Reserves::MoveWaitingdate( $reserve, $minus4days ); + CancelExpiredReserves(); + $count = $dbh->selectrow_array("SELECT COUNT(*) FROM reserves WHERE reserve_id = ?", undef, $reserve->{reserve_id} ); + is( $count, 1, "CancelExpiredReserves(): not canceling lastpickupdate-less hold." ); +} + +## Environment should be the following +## Holidays: days from today; -2,-3,-4 +sub testMoveWaitingdate { + $dbh->do('UPDATE issuingrules SET holdspickupwait = 1'); #Make holds problematize after 1 day + + $reserves = $dbh->selectall_arrayref('SELECT * FROM reserves WHERE found IS NULL', { Slice => {} }); #Get reserves not waiting for pickup + $reserve = $reserves->[0]; + + C4::Reserves::ModReserveAffect( $reserve->{itemnumber}, $reserve->{borrowernumber} ); #Catch the reserve and put it to wait for pickup, now we get a waitingdate generated. + + C4::Reserves::MoveWaitingdate( $reserve, $minus1days ); + $reserve = C4::Reserves::GetReserve( $reserve_id ); #UPDATE DB changes to local scope. Actually MoveWaitingdate already updates changes to DB, but just making sure it does. + is( ($reserve->{waitingdate} eq $now->clone()->add_duration($minus1days)->ymd() && + $reserve->{lastpickupdate} eq $now->ymd()), + 1, "MoveWaitingdate(): Moving to past"); + C4::Reserves::MoveWaitingdate( $reserve, $plus1days ); + + C4::Reserves::MoveWaitingdate( $reserve, $plus4days ); + $reserve = C4::Reserves::GetReserve( $reserve_id ); + is( ($reserve->{waitingdate} eq $now->clone()->add_duration($plus4days)->ymd() && + $reserve->{lastpickupdate} eq $now->clone()->add_duration($plus4days)->add_duration($plus1days)->ymd()), + 1, "MoveWaitingdate(): Moving to future"); + C4::Reserves::MoveWaitingdate( $reserve, $minus4days ); +} + +## Environment should be the following +## Holidays: days from today; -2,-3,-4 +sub testGetLastPickupDate { + $dbh->do('UPDATE issuingrules SET holdspickupwait = 1'); #Make holds problematize after 1 day + + my $now = DateTime->now(); + my $minus4days = DateTime::Duration->new(days => -4); + my $minus1days = DateTime::Duration->new(days => -1); + my $plus1days = DateTime::Duration->new(days => 1); + my $plus4days = DateTime::Duration->new(days => 4); + + $reserves = $dbh->selectall_arrayref('SELECT * FROM reserves', { Slice => {} }); #Get reserves not waiting for pickup + $reserve = $reserves->[0]; + + $reserve->{waitingdate} = $now->clone()->add_duration($minus4days)->ymd(); + my $lastpickupdate = C4::Reserves::GetLastPickupDate( $reserve )->ymd(); + $reserve = C4::Reserves::GetReserve( $reserve_id ); #UPDATE DB changes to local scope + is( $lastpickupdate, $now->clone()->add_duration($minus1days)->ymd(), + "GetLastPickupDate(): Calendar finds the next open day for lastpickupdate."); + + $reserve->{waitingdate} = $now->clone()->add_duration($minus1days)->ymd(); + $lastpickupdate = C4::Reserves::GetLastPickupDate( $reserve )->ymd(); + is( $lastpickupdate, $now->ymd(), + "GetLastPickupDate(): Not using Calendar"); + + $reserve->{waitingdate} = $now->clone()->add_duration($plus4days)->ymd(); + $lastpickupdate = C4::Reserves::GetLastPickupDate( $reserve )->ymd(); + is( $lastpickupdate, $now->clone()->add_duration($plus4days)->add_duration($plus1days)->ymd(), + "GetLastPickupDate(): Moving to future"); + + #This test catches moving lastpickupdate for each holiday, instead of just moving the last date to an open library day + $dbh->do('UPDATE issuingrules SET holdspickupwait = 4'); #Make holds problematize after 4 days + $reserve->{waitingdate} = $now->clone()->add_duration($minus4days)->ymd(); + $lastpickupdate = C4::Reserves::GetLastPickupDate( $reserve )->ymd(); + is( $lastpickupdate, $now->ymd(), + "GetLastPickupDate(): Moving lastpickupdate over holidays, but not affected by them"); + + #This test verifies that this feature is disabled and an undef is returned + $dbh->do('UPDATE issuingrules SET holdspickupwait = 0'); #Make holds never problematize + $reserve->{waitingdate} = $now->clone()->add_duration($minus4days)->ymd(); + $lastpickupdate = C4::Reserves::GetLastPickupDate( $reserve ); + is( $reserve->{lastpickupdate}, undef, + "GetLastPickupDate(): holdspickupwait disabled"); +} # Bug 12632 t::lib::Mocks::mock_preference( 'item-level_itypes', 1 ); @@ -513,7 +639,7 @@ subtest 'Pickup location availability tests' => sub { # Helper method to set up a Biblio. sub create_helper_biblio { - my $itemtype = shift; + my $itemtype = $_[0] ? $_[0] : 'BK'; my $bib = MARC::Record->new(); my $title = 'Silence in the library'; $bib->append_fields( @@ -523,3 +649,77 @@ sub create_helper_biblio { ); return ($bibnum, $title, $bibitemnum) = AddBiblio($bib, ''); } + +sub setSimpleCircPolicy { + $dbh->do('DELETE FROM issuingrules'); + $dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, + maxissueqty, issuelength, lengthunit, + renewalsallowed, renewalperiod, + norenewalbefore, auto_renew, + fine, chargeperiod, holdspickupwait) + VALUES (?, ?, ?, ?, + ?, ?, ?, + ?, ?, + ?, ?, + ?, ?, ? + ) + }, + {}, + '*', '*', '*', 25, + 20, 14, 'days', + 1, 7, + '', 0, + .10, 1,1 + ); +} + +###Set C4::Calendar and Koha::Calendar holidays for +# today -2 days +# today -3 days +# today -4 days +# +## Koha::Calendar for caching purposes (supposedly) doesn't work from the DB in this script +## So we must set the cache for Koha::calnder as well as the DB modifications for C4::Calendar. +## When making date comparisons with Koha::Calendar, using DateTime::Set, DateTime-objects +## need to match by the nanosecond and time_zone. +sub setCalendars { + + ##Set the C4::Calendar + my $now = DateTime->now(time_zone => C4::Context->tz())->truncate(to => 'day'); + my $c4calendar = C4::Calendar->new(branchcode => $reserve->{branchcode}); + $now->add_duration( DateTime::Duration->new(days => -2) ); + $c4calendar->insert_single_holiday( + day => $now->day(), + month => $now->month(), + year => $now->year(), + title => 'Test', + description => 'Test', + ); + $now->add_duration( DateTime::Duration->new(days => -1) ); + $c4calendar->insert_single_holiday( + day => $now->day(), + month => $now->month(), + year => $now->year(), + title => 'Test', + description => 'Test', + ); + $now->add_duration( DateTime::Duration->new(days => -1) ); + $c4calendar->insert_single_holiday( + day => $now->day(), + month => $now->month(), + year => $now->year(), + title => 'Test', + description => 'Test', + ); + + #Set the Koha::Calendar + my $kohaCalendar = Koha::Calendar->new(branchcode => $reserve->{branchcode}); + $now = DateTime->now(time_zone => C4::Context->tz())->truncate(to => 'day'); + $now->add_duration( DateTime::Duration->new(days => -2) ); + $kohaCalendar->add_holiday( $now ); + $now->add_duration( DateTime::Duration->new(days => -1) ); + $kohaCalendar->add_holiday( $now ); + $now->add_duration( DateTime::Duration->new(days => -1) ); + $kohaCalendar->add_holiday( $now ); +} --