From 6358a1464a918d3b82a21e26fc04c072b8f2aeb6 Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Thu, 26 Nov 2015 11:00:22 +0100 Subject: [PATCH] Bug 15261: Check if checkout/reserve request overlaps existing reserves When checking out or placing hold, we should check if an existing reserve whose period overlap exists. - A user places a hold from opac whose requested period overlaps with an existing reserve period => prevent reserve - A librarian places a hold from staff whose requested period overlaps with an existing reserve period => Warn librarian (Ask for confirmation) - A librarian makes a checkout from staff whose requested period overlaps with an existing reserve period => Warn librarian (Ask for confirmation) Test plan: Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod 1 (staff side): - Place a hold on title (which has only one items) level with start date and expiration date. - Place another hold (also title level) with period overlaping this reserve. - Check you are warned about an existing reserve 2 (staff side): - Place a hold on title (which has more than one items) level with start date and expiration date. - Place another hold (also title level) with period overlaping this reserve. - Check you are NOT warned about an existing reserve. Because it remains at least one item not reserved. 3 (staff side): - Place a hold on item level with start date and expiration date. - Place another hold on item level with period overlaping this reserve. - Check you are warned about an existing reserve. 4 (opac side): - Do the same than for staff side. Instead of a warn, reserve is prevented. 5: - Place a hold on title (which has only one items) level with start date and expiration date. - Try to checkout the unique item from this title with period overlaping the reserve period. - Check you are warned about an existing reserve 6: - Place a hold on title (which has more than one items) level with start date and expiration date. - Checkout an item from this title with period overlaping the reserve period. - Check you are NOT warned about an existing reserve. 7: - Place a hold on item level with start date and expiration date. - Checkout this item period overlaping the reserve period. - Check you are warned about an existing reserve 8: - Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}), - Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30 days}), - Check that you cannot renew the checkout. Signed-off-by: Laurence Rault --- C4/Circulation.pm | 25 ++++++- C4/ILSDI/Services.pm | 15 +++- C4/Reserves.pm | 58 +++++++++++++- Koha/DateUtils.pm | 37 +++++++++ .../data/mysql/atomicupdate/bug-15261.pl | 18 +++++ installer/data/mysql/mandatory/sysprefs.sql | 2 + .../admin/preferences/circulation.pref | 12 +++ .../prog/en/modules/circ/circulation.tt | 2 + .../prog/en/modules/reserve/placerequest.tt | 75 +++++++++++++++++++ .../bootstrap/en/modules/opac-user.tt | 2 + opac/opac-reserve.pl | 15 +++- reserve/placerequest.pl | 73 +++++++++++++++++- svc/renew | 2 +- 13 files changed, 326 insertions(+), 10 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug-15261.pl create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0a8df34263..a24740292d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1273,6 +1273,29 @@ sub CanBookBeIssued { } } } + + if ( C4::Context->preference("PreventCheckoutOnSameReservePeriod") ) { + my $now = dt_from_string(); + my $reserves_on_same_period = C4::Reserves::ReservesOnSamePeriod( + $item_object->biblionumber, $item_object->itemnumber, $now->ymd, + $duedate->ymd + ); + if ($reserves_on_same_period) { + my $reserve = $reserves_on_same_period->[0]; + if ( $reserve->{borrowernumber} ne $patron->borrowernumber ) { + my $patron = Koha::Patrons->find( $reserve->{borrowernumber} ); + $needsconfirmation{RESERVED} = 1; + $needsconfirmation{resfirstname} = $patron->firstname; + $needsconfirmation{ressurname} = $patron->surname; + $needsconfirmation{rescardnumber} = $patron->cardnumber; + $needsconfirmation{resborrowernumber} = $patron->borrowernumber; + $needsconfirmation{resbranchcode} = $patron->branchcode; + $needsconfirmation{resreservedate} = $reserve->{reservedate}; + $needsconfirmation{reserve_id} = $reserve->{reserve_id}; + } + } + } + } ## CHECK FOR BOOKINGS @@ -3177,7 +3200,7 @@ already renewed the loan. =cut sub CanBookBeRenewed { - my ( $patron, $issue, $override_limit, $cron ) = @_; + my ( $patron, $issue, $override_limit, $cron, $date_due ) = @_; my $auto_renew = "no"; my $soonest; diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 597a9ddd1d..b528e78a46 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -24,7 +24,8 @@ use C4::Members; use C4::Items qw( get_hostitemnumbers_of ); use C4::Circulation qw( CanBookBeRenewed barcodedecode CanBookBeIssued AddRenewal ); use C4::Accounts; -use C4::Reserves qw( CanBookBeReserved IsAvailableForItemLevelRequest CalculatePriority AddReserve CanItemBeReserved ); +use C4::Reserves + qw( CanBookBeReserved IsAvailableForItemLevelRequest CalculatePriority AddReserve CanItemBeReserved ReservesOnSamePeriod); use C4::Context; use C4::Auth; use CGI qw ( -utf8 ); @@ -816,6 +817,12 @@ sub HoldTitle { my $resdate = $cgi->param('start_date'); my $expdate = $cgi->param('expiry_date'); + if ( C4::Context->preference('PreventReservesOnSamePeriod') + && ReservesOnSamePeriod( $biblionumber, undef, $resdate, $expdate ) ) + { + return { code => 'overlap' }; + } + # Add the reserve # $branch, $borrowernumber, $biblionumber, # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, @@ -912,6 +919,12 @@ sub HoldItem { my $resdate = $cgi->param('start_date'); my $expdate = $cgi->param('expiry_date'); + if ( C4::Context->preference('PreventReservesOnSamePeriod') + && ReservesOnSamePeriod( $biblionumber, $itemnumber, $resdate, $expdate ) ) + { + return { code => 'overlap' }; + } + # Add the reserve my $priority = C4::Reserves::CalculatePriority($biblionumber); AddReserve( diff --git a/C4/Reserves.pm b/C4/Reserves.pm index b559c8b9b1..5a3e988ae1 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -40,7 +40,7 @@ use Koha::Calendar; use Koha::Cache::Memory::Lite; use Koha::CirculationRules; use Koha::Database; -use Koha::DateUtils qw( dt_from_string output_pref ); +use Koha::DateUtils qw( dt_from_string output_pref date_ranges_overlap ); use Koha::Holds; use Koha::ItemTypes; use Koha::Items; @@ -139,6 +139,10 @@ BEGIN { GetMaxPatronHoldsForRecord MergeHolds + + RevertWaitingStatus + + ReservesOnSamePeriod ); } @@ -2424,6 +2428,58 @@ sub GetMaxPatronHoldsForRecord { return $max; } +=head2 ReservesOnSamePeriod + + my $reserve = ReservesOnSamePeriod( $biblionumber, $itemnumber, $resdate, $expdate); + + Return the reserve that match the period ($resdate => $expdate), + undef if no reserve match. + +=cut + +sub ReservesOnSamePeriod { + my ( $biblionumber, $itemnumber, $resdate, $expdate ) = @_; + + unless ( $resdate && $expdate ) { + return; + } + + my @reserves = Koha::Holds->search( { biblionumber => $biblionumber } )->as_list; + + $resdate = output_pref( { str => $resdate, dateonly => 1, dateformat => 'iso' } ); + $expdate = output_pref( { str => $expdate, dateonly => 1, dateformat => 'iso' } ); + + my @reserves_overlaps; + foreach my $reserve (@reserves) { + + unless ( $reserve->reservedate && $reserve->expirationdate ) { + next; + } + + if ( + date_ranges_overlap( + $resdate, $expdate, + $reserve->reservedate, + $reserve->expirationdate + ) + ) + { + + # If reserve is item level and the requested periods overlap. + if ( $itemnumber && $reserve->itemnumber && $reserve->itemnumber == $itemnumber ) { + return [ $reserve->unblessed ]; + } + push @reserves_overlaps, $reserve->unblessed; + } + } + + if ( @reserves_overlaps >= Koha::Items->search( { biblionumber => $biblionumber } )->count() ) { + return \@reserves_overlaps; + } + + return; +} + =head1 AUTHOR Koha Development Team diff --git a/Koha/DateUtils.pm b/Koha/DateUtils.pm index fed83d5b6e..bb10d3c767 100644 --- a/Koha/DateUtils.pm +++ b/Koha/DateUtils.pm @@ -33,6 +33,7 @@ BEGIN { output_pref format_sqldatetime flatpickr_date_format + date_ranges_overlap ); } @@ -393,4 +394,40 @@ sub flatpickr_date_format { }->{$arg}; } +=head2 date_ranges_overlap + + $bool = date_ranges_overlap($start1, $end1, $start2, $end2); + + Tells if first range ($start1 => $end1) overlaps + the second one ($start2 => $end2) + +=cut + +sub date_ranges_overlap { + my ( $start1, $end1, $start2, $end2 ) = @_; + + $start1 = dt_from_string( $start1, 'iso' ); + $end1 = dt_from_string( $end1, 'iso' ); + $start2 = dt_from_string( $start2, 'iso' ); + $end2 = dt_from_string( $end2, 'iso' ); + + if ( + # Start of range 2 is in the range 1. + ( DateTime->compare( $start2, $start1 ) >= 0 && DateTime->compare( $start2, $end1 ) <= 0 ) + || + + # End of range 2 is in the range 1. + ( DateTime->compare( $end2, $start1 ) >= 0 && DateTime->compare( $end2, $end1 ) <= 0 ) + || + + # Range 2 start before and end after range 1. + ( DateTime->compare( $start2, $start1 ) < 0 && DateTime->compare( $end2, $end1 ) > 0 ) + ) + { + return 1; + } + + return; +} + 1; diff --git a/installer/data/mysql/atomicupdate/bug-15261.pl b/installer/data/mysql/atomicupdate/bug-15261.pl new file mode 100755 index 0000000000..dc1e528d2c --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-15261.pl @@ -0,0 +1,18 @@ +use Modern::Perl; + +return { + bug_number => '15261', + description => 'Add sysprefs PreventCheckoutOnSameReservePeriod and PreventReservesOnSamePeriod', + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do( + q{ + INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES + ('PreventReservesOnSamePeriod', '0', 'Prevent to hold a document if a reserve on same period exists', NULL, 'YesNo'), + ('PreventCheckoutOnSameReservePeriod', '0', 'Prevent to checkout a document if a reserve on same period exists', NULL, 'YesNo') + } + ); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 0d77d5ba94..401669cea7 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -648,7 +648,9 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('PreservationNotForLoanDefaultTrainIn', '', '', 'Not for loan to apply to items removed from the preservation waiting list', 'TextArea'), ('PreservationNotForLoanWaitingListIn', '', '', 'Not for loan to apply to items added to the preservation waiting list', 'TextArea'), ('PreserveSerialNotes','1','','When a new "Expected" issue is generated, should it be prefilled with last created issue notes?','YesNo'), +('PreventCheckoutOnSameReservePeriod','0','','Prevent to checkout a document if a reserve on same period exists','YesNo'), ('PreventWithdrawingItemsStatus', '', '', 'Prevent the withdrawing of items based on certain statuses' , 'multiple'), +('PreventReservesOnSamePeriod','0','','Prevent to hold a document if a reserve on same period exists','YesNo'), ('previousIssuesDefaultSortOrder','asc','asc|desc','Specify the sort order of Previous Issues on the circulation page','Choice'), ('PrintNoticesMaxLines','0','','If greater than 0, sets the maximum number of lines an overdue notice will print. If the number of items is greater than this number, the notice will end with a warning asking the borrower to check their online account for a full list of overdue items.','Integer'), ('PrivacyPolicyConsent','','Enforced|Permissive|Disabled','Data privacy policy consent in the OPAC', 'Choice'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 82070552d6..f9082faa8d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -653,6 +653,12 @@ Circulation: class: integer - to fill holds. - "(list of not for loan authorized values separated with a pipe '|')" + - + - pref: PreventCheckoutOnSameReservePeriod + choices: + 1: Do + 0: "Don't" + - If yes, checkouts periods can't overlap with a reserve period. - - pref: HoldsAutoFill choices: @@ -1123,6 +1129,12 @@ Circulation: 1: "Do" 0: "Don't" - set holds cancelled via SIP as cancellation requests. + - + - pref: PreventReservesOnSamePeriod + choices: + 1: Do + 0: "Don't" + - If yes, Reserves periods for the same document can't overlap. Patron restrictions: - - pref: PatronRestrictionTypes diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index 92e438c86e..a4136029ce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -346,6 +346,7 @@ [% IF ( RESERVED ) %]

+

[% END %] @@ -354,6 +355,7 @@


+

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt new file mode 100644 index 0000000000..3b275d9cc5 --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt @@ -0,0 +1,75 @@ +[% USE JSON.Escape %] +[% USE raw %] +[% SET footerjs = 1 %] +[% INCLUDE 'doc-head-open.inc' %] +Confirm holds › Holds › Circulation › Koha +[% INCLUDE 'doc-head-close.inc' %] + + + +[% WRAPPER 'header.inc' %] + [% INCLUDE 'circ-search.inc' %] +[% END %] + +[% WRAPPER 'sub-header.inc' %] + [% WRAPPER breadcrumbs %] + [% WRAPPER breadcrumb_item %] + Catalog + [% END %] + [% WRAPPER breadcrumb_item %] + Confirm holds + [% END %] + [% END %] +[% END %] + +
+
+ +
+
+ +[% MACRO jsinclude BLOCK %] + +[% END %] + +[% INCLUDE 'intranet-bottom.inc' %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt index 27d287c20b..337beb08ef 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt @@ -104,6 +104,8 @@ There are items available in the library, please visit to check them out [% CASE 'not_placed' %] Error when placing hold, please report this to the library + [% CASE 'overlaps' %] + Hold overlaps with existing holds [% CASE %] Error: [% fail | html %] [% END %] diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index d35ae935f3..a8144c24a2 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -24,9 +24,10 @@ use CGI qw ( -utf8 ); use C4::Auth qw( get_template_and_user ); use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); use C4::Circulation qw( GetBranchItemRule ); -use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ); -use C4::Biblio qw( GetBiblioData GetFrameworkCode ); -use C4::Output qw( output_html_with_http_headers ); +use C4::Reserves + qw( CanItemBeReserved CanBookBeReserved AddReserve IsAvailableForItemLevelRequest GetReserveFee ReservesOnSamePeriod ); +use C4::Biblio qw( GetBiblioData GetFrameworkCode ); +use C4::Output qw( output_html_with_http_headers ); use C4::Context; use C4::Members; use C4::Overdues; @@ -267,6 +268,14 @@ if ( $op eq 'cud-place_reserve' ) { # Inserts a null into the 'itemnumber' field of 'reserves' table. $itemNum = undef; } + + if ( $canreserve && C4::Context->preference("PreventReservesOnSamePeriod") ) { + if ( C4::Reserves::ReservesOnSamePeriod( $biblioNum, $itemNum, $startdate, $patron_expiration_date ) ) { + $canreserve = 0; + push @failed_holds, 'overlaps'; + } + } + my $notes = $query->param( 'notes_' . $biblioNum ) || ''; my $item_group_id = $query->param("item_group_id_$biblioNum") || undef; diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 9c8838c47c..b1d0c64e5b 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -22,10 +22,13 @@ use Modern::Perl; +use List::MoreUtils qw( none ); + use CGI qw ( -utf8 ); use URI; use C4::Reserves qw( CanItemBeReserved AddReserve CanBookBeReserved ); -use C4::Auth qw( checkauth ); +use C4::Auth qw( get_template_and_user ); +use C4::Output qw( output_html_with_http_headers ); use Koha::Items; use Koha::Patrons; @@ -33,7 +36,14 @@ use Koha::HoldGroup; my $input = CGI->new(); -checkauth( $input, 0, { reserveforothers => 'place_holds' }, 'intranet' ); +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { + template_name => 'reserve/placerequest.tt', + query => $input, + type => 'intranet', + flagsrequired => { reserveforothers => 'place_holds' }, + } +); my @reqbib = $input->multi_param('reqbib'); my @biblionumbers = $input->multi_param('biblionumber'); @@ -54,11 +64,15 @@ my $multi_holds = $input->param('multi_holds'); my $hold_group_param = $input->param('hold_group') || undef; my $hold_group; +my $confirm = $input->param('confirm'); +my @confirm_biblionumbers = $input->multi_param('confirm_biblionumbers'); + my $patron = Koha::Patrons->find($borrowernumber); my $holds_to_place_count = $input->param('holds_to_place_count') || 1; -my %bibinfos = (); +my %bibinfos = (); +my $multi_hold = @holdable_bibs > 1; foreach my $bibnum (@holdable_bibs) { my %bibinfo = (); $bibinfo{title} = $input->param("title_$bibnum"); @@ -73,7 +87,60 @@ if ( $op eq 'cud-placerequest' && $patron ) { $hold_group = Koha::HoldGroup->new->store; } + if ( C4::Context->preference('PreventReservesOnSamePeriod') && !$confirm ) { + my %overlap_reserves; + foreach my $biblionumber ( keys %bibinfos ) { + next if exists $overlap_reserves{$biblionumber}; + + if (@checkitems) { + for my $checkitem (@checkitems) { + my $item = Koha::Items->find($checkitem); + next if $biblionumber ne $item->biblionumber; + + my $overlapping_reserves = + C4::Reserves::ReservesOnSamePeriod( $biblionumber, $checkitem, $startdate, $expirationdate ); + if ( $overlapping_reserves && 0 < scalar @$overlapping_reserves ) { + $overlap_reserves{$biblionumber} = { + title => $multi_hold ? $bibinfos{$biblionumber}->{title} : $title, + }; + } + } + } else { + my $overlapping_reserves = + C4::Reserves::ReservesOnSamePeriod( $biblionumber, undef, $startdate, $expirationdate ); + if ( $overlapping_reserves && 0 < scalar @$overlapping_reserves ) { + $overlap_reserves{$biblionumber} = { + title => $multi_hold ? $bibinfos{$biblionumber}->{title} : $title, + }; + } + } + } + + if ( scalar keys %overlap_reserves ) { + my %params = $input->Vars; + delete $params{csrf_token}; + my @params; + foreach my $name ( sort keys %params ) { + my @values = split( "\0", $params{$name} ); + foreach my $value (@values) { + push @params, { name => $name, value => $value }; + } + } + $template->param( params => \@params ); + + $template->param( + borrowernumber => $borrowernumber, + biblionumbers => \@biblionumbers, + overlap_reserves => \%overlap_reserves, + ); + + output_html_with_http_headers $input, $cookie, $template->output; + exit; + } + } + foreach my $biblionumber ( keys %bibinfos ) { + next if ( $confirm && none { $_ eq $biblionumber } @confirm_biblionumbers ); my $can_override = C4::Context->preference('AllowHoldPolicyOverride'); if (@checkitems) { diff --git a/svc/renew b/svc/renew index 86c8b1548a..e02f9de72b 100755 --- a/svc/renew +++ b/svc/renew @@ -64,7 +64,7 @@ my $item = Koha::Items->find($itemnumber); my $confirmations; ( $data->{renew_okay}, $data->{error} ) = - CanBookBeRenewed( $patron, $item->checkout, $override_limit ); + CanBookBeRenewed( $patron, $item->checkout, $override_limit, 0, $date_due ); # If we're allowing reserved items to be renewed... if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride') ) { -- 2.39.5