From 9fcd0eb036d156aa389f9a73b0a708214a6c23a5 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Sat, 22 Sep 2018 15:46:37 +0000 Subject: [PATCH] Bug 8367: Template fixes + fixes to C4/Reserves.pm and Koha/Hold.pm Fixed waitingreserves.tt template so the borrowernumber is successfully passed to waitingreserves.pl upon cancelling a hold, so users can now successfully cancel holds from the waitingreserves.pl interface. Removed check for ReserveMaxPickupdateDelay in Koha/Hold.pm as this syspref is removed in the previous patch. Instead the expirationdate is calculated the same way as the lastpickupdatedate that is retrieving the holdpickupwait value for the appropriate issuingrule and then adding that onto the waiting date. Same test plan as main patch Sponsored-By: Brimbank Library, Australia --- C4/Circulation.pm | 4 +- C4/Letters.pm | 12 +- C4/Reserves.pm | 132 +++++++++++++++++++-- Koha/Hold.pm | 32 ++++- Koha/IssuingRules.pm | 2 +- admin/smart-rules.pl | 1 + circ/returns.pl | 1 + circ/waitingreserves.pl | 60 ++++++---- .../prog/en/modules/admin/smart-rules.tt | 1 + .../prog/en/modules/help/admin/smart-rules.tt | 32 ++--- .../prog/en/modules/members/moremember.tt | 1 + koha-tmpl/intranet-tmpl/prog/js/holds.js | 3 + 12 files changed, 221 insertions(+), 60 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9961d380778..44f94e7fec7 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2005,7 +2005,6 @@ sub AddReturn { } $validTransfert = 1; } - # fix up the accounts..... if ( $item->itemlost ) { $messages->{'WasLost'} = 1; @@ -2061,8 +2060,9 @@ sub AddReturn { my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->itemnumber, undef, $lookahead ) unless ( $item->withdrawn ); if ($resfound) { - $resrec->{'ResFound'} = $resfound; + $resrec->{'ResFound'} = $resfound; $messages->{'ResFound'} = $resrec; + C4::Reserves::ModReserveStatus($item->{'itemnumber'}, 'W'); } # Record the fact that this book was returned. diff --git a/C4/Letters.pm b/C4/Letters.pm index bdb213a49e8..cbf0e7a40ff 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -786,15 +786,15 @@ sub _parseletter { # Work on a local copy of $values_in (passed by reference) to avoid side effects # in callers ( by changing / formatting values ) - my $values = $values_in ? { %$values_in } : {}; + my $values = $values_in ? { %$values_in } : {}; if ( $table eq 'borrowers' && $values->{'dateexpiry'} ){ $values->{'dateexpiry'} = output_pref({ dt => dt_from_string( $values->{'dateexpiry'} ), dateonly => 1 }); } - if ( $table eq 'reserves' && $values->{'waitingdate'} ) { - $values->{'waitingdate'} = output_pref({ dt => dt_from_string( $values->{'waitingdate'} ), dateonly => 1 }); - } +# if ( $table eq 'reserves' && $values->{'waitingdate'}) { +# $values->{'waitingdate'} = output_pref({ dt => dt_from_string( $values->{'waitingdate'} ), dateonly => 1 }); +# } if ($letter->{content} && $letter->{content} =~ /<>/) { my $todaysdate = output_pref( DateTime->now() ); @@ -811,7 +811,9 @@ sub _parseletter { my $av = Koha::AuthorisedValues->search({ category => 'ROADTYPE', authorised_value => $val }); $val = $av->count ? $av->next->lib : ''; } - +warn $table; +warn $field; +warn $val; # Dates replacement my $replacedby = defined ($val) ? $val : ''; if ( $replacedby diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f2801d83ebb..716ef622142 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -120,7 +120,6 @@ BEGIN { &CanItemBeReserved &CanReserveBeCanceledFromOpac &CancelExpiredReserves - &AutoUnsuspendReserves &IsAvailableForItemLevelRequest @@ -194,12 +193,11 @@ sub AddReserve { #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 ); + my $patron = Koha::Patrons->find( {borrowernumber => $borrowernumber} ); + $lastpickupdate = GetLastPickupDate( $reserve, $item, $patron ); } - # Don't add itemtype limit if specific item is selected $itemtype = undef if $checkitem; - # updates take place here my $hold = Koha::Hold->new( { @@ -219,7 +217,6 @@ sub AddReserve { } )->store(); $hold->set_waiting() if $found eq 'W'; - logaction( 'HOLDS', 'CREATE', $hold->id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); @@ -666,6 +663,104 @@ sub GetReserveStatus { return ''; # empty string here will remove need for checking undef, or less log lines } + +=head2 GetLastPickupDate + + my $lastpickupdate = GetLastPickupDate($reserve, $item); + my $lastpickupdate = GetLastPickupDate($reserve, $item, $borrower); + my $lastpickupdate = GetLastPickupDate(undef, $item); + + Gets the last pickup date from the issuingrules for the given reserves-row and sets the + $reserve->{lastpickupdate}.-value. + + If the reserves-row is not passed, function tries to figure it out from the item-row. + Calculating the last pickup date respects Calendar holidays and skips to the next open day. + If the issuingrule's holdspickupwait is 0 or less, means that the lastpickupdate-feature + is disabled for this kind of material. + + @PARAM1 koha.reserves-row + @PARAM2 koha.items-row, If the reserve is not given, an item must be given to be + able to find a reservation + @PARAM3 koha.borrowers-row, OPTIONAL + RETURNS DateTime, depicting the last pickup date. + OR undef if there is an error or if the issuingrules disable this feature for this material. + +=cut + +sub GetLastPickupDate { + my ($reserve, $item, $borrower) = @_; + ##Verify parameters + if ( not defined $reserve and not defined $item ) { + warn "C4::Reserves::GetMaxPickupDate(), is called without a reserve and an item"; + return; + } + if ( defined $reserve and not defined $item ) { + $item = C4::Items::GetItem( $reserve->{itemnumber} ); + } + + unless ( defined $borrower) { + my $borrower = Koha::Patrons->find( $reserve->{borrowernumber} ); + $borrower = $borrower->unblessed; + } + + unless ( defined $reserve ) { + my $reserve = GetReservesFromItemnumber( $item->{itemnumber} ); + } + my $date = $reserve->{waitingdate}; + unless ( $date ) { #It is possible that a reserve is just caught and it doesn't have a waitingdate yet. + $date = DateTime->now( time_zone => C4::Context->tz() ); #So default to NOW() + } + else { + $date = (ref $reserve->{waitingdate} eq 'DateTime') ? $reserve->{waitingdate} : dt_from_string($reserve->{waitingdate}); + } + ##Get churning the LastPickupDate + + # Get the controlbranch + my $controlbranch = GetReservesControlBranch( $item, $borrower ); + my $hbr = C4::Context->preference('HomeOrHoldingBranch') || "homebranch"; + my $branchcode = "*"; + if ( $controlbranch eq "ItemHomeLibrary" ) { + $branchcode = $item->{$hbr}; + } elsif ( $controlbranch eq "PatronLibrary" ) { + $branchcode = $borrower->branchcode; + } + warn 'getlastpickup'; + warn $branchcode; + warn $borrower->{'categorycode'}; + warn $item->{itype}; + my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule({ + branchcode => $branchcode, + categorycode => $borrower->{'categorycode'}, + itemtype => $item->{itype}, + }); + + my $reservebranch; + if ($reserve->{'branchcode'}) { + $reservebranch = $reserve->{'branchcode'}; + } else { + $reservebranch = $reserve->branchcode; + } + if ( defined($issuingrule) && defined $issuingrule->holdspickupwait && $issuingrule->holdspickupwait > 0 ) { #If holdspickupwait is <= 0, it means this feature is disabled for this type of material. + $date->add( days => $issuingrule->holdspickupwait ); + my $calendar = Koha::Calendar->new( branchcode => $reservebranch ); + my $is_holiday = $calendar->is_holiday( $date ); + while ( $is_holiday ) { + $date->add( days => 1 ); + $is_holiday = $calendar->is_holiday( $date ); + } + $reserve->{lastpickupdate} = $date->ymd(); + return $date; + } + #Without explicitly setting the return value undef, the lastpickupdate-column is + # set as 0000-00-00 instead of NULL in DBD::MySQL. + #This causes this hold to always expire any lastpickupdate check, + ## effectively canceling it as soon as cancel_expired_holds.pl is ran. + ##This is exactly the opposite of disabling the autoexpire feature. + ##So please let me explicitly return undef, because Perl cant always handle everything. + delete $reserve->{lastpickupdate}; + return undef; +} + =head2 CheckReserves ($status, $matched_reserve, $possible_reserves) = &CheckReserves($itemnumber); @@ -1011,7 +1106,7 @@ sub ModReserveFill { =head2 ModReserveStatus - &ModReserveStatus($itemnumber, $newstatus); + &ModReserveStatus($itemnumber, $newstatus, $holdtype); Update the reserve status for the active (priority=0) reserve. @@ -1022,12 +1117,12 @@ $newstatus is the new status. =cut sub ModReserveStatus { - #first : check if we have a reservation for this item . my ($itemnumber, $newstatus) = @_; my $dbh = C4::Context->dbh; my $now = dt_from_string; + my $reserve = $dbh->selectrow_hashref(q{ SELECT * FROM reserves @@ -1035,10 +1130,13 @@ sub ModReserveStatus { AND found IS NULL AND priority = 0 }, {}, $itemnumber); - return unless $reserve; - my $lastpickupdate = GetLastPickupDate( $reserve ); + return unless $reserve; + my $borrower = Koha::Patrons->find( $reserve->{borrowernumber} ); + $borrower = $borrower->unblessed; + my $item = C4::Items::GetItem( $reserve->{itemnumber} ); + my $lastpickupdate = GetLastPickupDate( $reserve, $item, $borrower ); my $query = q{ UPDATE reserves SET found = ?, @@ -1100,6 +1198,20 @@ sub ModReserveAffect { $hold->itemnumber($itemnumber); $hold->set_waiting($transferToDo); + my $item = C4::Items::GetItem( $itemnumber ); + my $borrower = Koha::Patrons->find( $borrowernumber ); + $borrower = $borrower->unblessed; + my $lastpickupdate = GetLastPickupDate( $hold, $item, $borrower ); + warn $lastpickupdate; + my $query = " + UPDATE reserves + SET lastpickupdate = ? + WHERE borrowernumber = ? + AND itemnumber = ? + "; + $sth = $dbh->prepare($query); + $sth->execute( $lastpickupdate, $borrowernumber,$itemnumber); + _koha_notify_reserve( $hold->reserve_id ) if ( !$transferToDo && !$already_on_shelf ); @@ -2057,7 +2169,7 @@ sub ReserveSlip { return unless $hold; my $reserve = $hold->unblessed; - +warn Dumper($reserve); return C4::Letters::GetPreparedLetter ( module => 'circulation', letter_code => 'HOLD_SLIP', diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 30188ae69e1..b509fbd4313 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -33,6 +33,7 @@ use Koha::Items; use Koha::Libraries; use Koha::Old::Holds; use Koha::Calendar; +use Koha::IssuingRules; use Koha::Exceptions::Hold; @@ -181,10 +182,37 @@ sub set_waiting { my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); my $expirationdate = $today->clone; - $expirationdate->add(days => $max_pickup_delay); + + my $patron = Koha::Patrons->find( {borrowernumber => $self->borrowernumber} ); + my $item = Koha::Items->find( $self->itemnumber() ); + my $hold = Koha::Holds->find( $self->reserve_id ); +# my $lastpickupdate = C4::Reserves->GetLastPickupDate( $hold, $item, $patron); + + # Get the controlbranch + my $controlbranch = C4::Reserves->GetReservesControlBranch( $item, $patron ); + my $hbr = C4::Context->preference('HomeOrHoldingBranch') || "homebranch"; + my $branchcode = "*"; + if ( $controlbranch eq "ItemHomeLibrary" ) { + $branchcode = $item->{$hbr}; + } elsif ( $controlbranch eq "PatronLibrary" ) { + $branchcode = $patron->branchcode; + } + + warn $branchcode; + warn $patron->categorycode; + warn $item->itype; + my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule({ + branchcode => $branchcode, + categorycode => $patron->categorycode, + itemtype => $item->itype, + }); + + if ( defined $issuingrule->holdspickupwait && $issuingrule->holdspickupwait > 0) { + $expirationdate->add(days => $issuingrule->holdspickupwait); + } if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { - $expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay ); + $expirationdate = $calendar->days_forward( dt_from_string(), $issuingrule->{holdspickupwait} ); } # If patron's requested expiration date is prior to the diff --git a/Koha/IssuingRules.pm b/Koha/IssuingRules.pm index 608b4edeedb..a9459d0543f 100644 --- a/Koha/IssuingRules.pm +++ b/Koha/IssuingRules.pm @@ -22,7 +22,7 @@ use Modern::Perl; use Koha::Database; use Koha::Caches; - +use Data::Dumper; use Koha::IssuingRule; use base qw(Koha::Objects); diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 5b1da40ed3e..6bfb7af61c4 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -278,6 +278,7 @@ elsif ($op eq 'add') { chargeperiod_charge_at => $chargeperiod_charge_at, renewalsallowed => $renewalsallowed, renewalperiod => $renewalperiod, + holdspickupwait => $holdspickupwait, norenewalbefore => $norenewalbefore, auto_renew => $auto_renew, no_auto_renewal_after => $no_auto_renewal_after, diff --git a/circ/returns.pl b/circ/returns.pl index 657229a10e1..5f01c5959e4 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -424,6 +424,7 @@ if ( $messages->{'ResFound'}) { my $diffBranchSend = !$branchCheck ? $reserve->{branchcode} : undef; ModReserveAffect( $reserve->{itemnumber}, $reserve->{borrowernumber}, $diffBranchSend, $reserve->{reserve_id} ); + ModReserveStatus($reserve->{itemnumber}, 'W'); my ( $messages, $nextreservinfo ) = GetOtherReserves($reserve->{itemnumber}); my $patron = Koha::Patrons->find( $nextreservinfo ); diff --git a/circ/waitingreserves.pl b/circ/waitingreserves.pl index b91a338f665..3e54bbef97d 100755 --- a/circ/waitingreserves.pl +++ b/circ/waitingreserves.pl @@ -27,11 +27,11 @@ use C4::Circulation; use C4::Members; use C4::Biblio; use C4::Items; -use Date::Calc qw( - Today - Add_Delta_Days - Date_to_Days -); +#use Date::Calc qw( +# Today +# Add_Delta_Days +# Date_to_Days +#); use C4::Reserves; use C4::Koha; use Koha::DateUtils; @@ -39,6 +39,7 @@ use Koha::BiblioFrameworks; use Koha::Items; use Koha::ItemTypes; use Koha::Patrons; +use Data::Dumper; my $input = new CGI; @@ -87,23 +88,22 @@ my $holds = Koha::Holds->waiting->search({ priority => 0, ( $all_branches ? () : # get reserves for the branch we are logged into, or for all branches -my $today = Date_to_Days(&Today); +my $today = dt_from_string; +my $max_pickup_delay = C4::Context->preference('ReservesMaxPickUpDelay'); while ( my $hold = $holds->next ) { next unless ($hold->waitingdate && $hold->waitingdate ne '0000-00-00'); - my $item = $hold->item; my $patron = $hold->borrower; my $biblio = $item->biblio; my $holdingbranch = $item->holdingbranch; my $homebranch = $item->homebranch; - my %getreserv = ( title => $biblio->title, itemnumber => $item->itemnumber, waitingdate => $hold->waitingdate, reservedate => $hold->reservedate, - borrowernum => $patron->borrowernumber, + borrowernumber => $patron->borrowernumber, biblionumber => $biblio->biblionumber, barcode => $item->barcode, homebranch => $homebranch, @@ -118,21 +118,34 @@ while ( my $hold = $holds->next ) { ); my $itemtype = Koha::ItemTypes->find( $item->effective_itemtype ); - my ( $expire_year, $expire_month, $expire_day ) = split (/-/, $hold->expirationdate); - my $calcDate = Date_to_Days( $expire_year, $expire_month, $expire_day ); - - if ($today > $calcDate) { - if ($cancelall) { - my $res = cancel( $hold->item->itemnumber, $hold->borrowernumber, $hold->item->holdingbranch, $hold->item->homebranch, !$transfer_when_cancel_all ); - push @cancel_result, $res if $res; - next; - } else { - push @over_loop, $hold; + $getreserv{'itemtype'} = $itemtype->description; # FIXME Should not it be translated_description? + $getreserv{'subtitle'} = GetRecordValue( + 'subtitle', + GetMarcBiblio({ biblionumber => $biblio->biblionumber }), + $biblio->frameworkcode); + if ( $homebranch ne $holdingbranch ) { + $getreserv{'dotransfer'} = 1; + } + + $getreserv{patron} = $patron; + warn $getreserv{'waitingdate'}; + warn $getreserv{'lastpickupdate'}; + if ( $getreserv{'waitingdate'} ) { + my $lastpickupdate = dt_from_string($getreserv{'lastpickupdate'}); + if ( DateTime->compare( $today, $lastpickupdate ) == 1 ) { + if ($cancelall) { + my $res = cancel( $item->itemnumber, $patron->borrowernumber, $item->holdingbranch, $homebranch, !$transfer_when_cancel_all ); + push @cancel_result, $res if $res; + next; + } else { + push @overloop, \%getreserv; + $overcount++; + } + }else{ + push @reservloop, \%getreserv; + $reservcount++; } - }else{ - push @reserve_loop, $hold; } - } $template->param(cancel_result => \@cancel_result) if @cancel_result; @@ -144,7 +157,7 @@ $template->param( overcount => scalar @over_loop, show_date => output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), tab => $tab, - show_date => format_date(C4::Dates->today('iso')), + show_date => $today, ); # Checking if there is a Fast Cataloging Framework @@ -166,7 +179,6 @@ sub cancel { my $transfer = $fbr ne $tbr; # XXX && !$nextreservinfo; return if $transfer && $skip_transfers; - my ( $messages, $nextreservinfo ) = ModReserveCancelAll( $item, $borrowernumber ); # if the document is not in his homebranch location and there is not reservation after, we transfer it 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 42af59359b9..30d0ba9894b 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 @@ -196,6 +196,7 @@ [% rule.suspension_chargeperiod | html %] [% rule.renewalsallowed | html %] [% rule.renewalperiod | html %] + [% rule.holdspickupwait | html %] [% rule.norenewalbefore | html %] [% IF ( rule.auto_renew ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/smart-rules.tt index 72a4b916523..1452e510a3a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/help/admin/smart-rules.tt @@ -7,23 +7,23 @@

The rules are applied from most specific to less specific, using the first found in this order:

The CircControl and HomeOrHoldingBranch also come in to play when figuring out which circulation rule to follow.

If you are a single library system choose your branch name before creating rules (sometimes having only rules for the 'all libraries' option can cause issues with holds)

@@ -37,9 +37,9 @@

From the matrix you can choose any combination of patron categories and item types to apply the rules to

-
  • Once your policy is set, you can unset it by clicking the 'Unset' link to the right of the rule
  • +
  • Once your policy is set, you can unset it by clicking the 'Unset' link to the right of the rule
  • Checkouts Per Patron

    diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index 86d26a586f3..227c690914a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -750,6 +750,7 @@ Hold date + Last pickup date Title Call number Barcode diff --git a/koha-tmpl/intranet-tmpl/prog/js/holds.js b/koha-tmpl/intranet-tmpl/prog/js/holds.js index 3c0d77dc48b..64bc49d2c12 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/holds.js +++ b/koha-tmpl/intranet-tmpl/prog/js/holds.js @@ -18,6 +18,9 @@ $(document).ready(function() { "data": { _: "reservedate_formatted", "sort": "reservedate" } }, { + "mDataProp": "lastpickupdate_formatted" + }, + { "mDataProp": function ( oObj ) { title = "