From 07478955c2356923fa51e8979beb1936b66c8cef Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Wed, 20 Oct 2021 15:47:47 +1300 Subject: [PATCH] Bug 24718: Hour based holds on staff client Tests in t/db_dependent/Holds.t --- C4/HoldsQueue.pm | 4 +- C4/Reserves.pm | 34 ++++++++-- Koha/Hold.pm | 2 +- Koha/Item.pm | 2 +- .../intranet-tmpl/prog/en/includes/holds_table.inc | 18 +++++- .../prog/en/includes/waiting_holds.inc | 6 +- .../prog/en/modules/circ/pendingreserves.tt | 6 +- .../prog/en/modules/circ/view_holdsqueue.tt | 8 ++- .../prog/en/modules/members/holdshistory.tt | 11 ++-- .../prog/en/modules/reserve/request.tt | 19 +++++- svc/holds | 25 ++++++-- t/db_dependent/Holds.t | 75 ++++++++++++++++++++-- 12 files changed, 177 insertions(+), 33 deletions(-) diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm index b4d2b026f2..3e620a5b89 100644 --- a/C4/HoldsQueue.pm +++ b/C4/HoldsQueue.pm @@ -231,7 +231,7 @@ sub GetBibsWithPendingHoldRequests { FROM reserves WHERE found IS NULL AND priority > 0 - AND reservedate <= CURRENT_DATE() + AND reservedate <= CURRENT_TIMESTAMP() AND suspend = 0 "; my $sth = $dbh->prepare($bib_query); @@ -275,7 +275,7 @@ sub GetPendingHoldRequestsForBib { WHERE biblionumber = ? AND found IS NULL AND priority > 0 - AND reservedate <= CURRENT_DATE() + AND reservedate <= CURRENT_TIMESTAMP() AND suspend = 0 ORDER BY priority"; my $sth = $dbh->prepare($request_query); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f2ee463979..61958ca74c 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -194,10 +194,26 @@ sub AddReserve { my $itemtype = $params->{itemtype}; my $non_priority = $params->{non_priority}; - $resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' }) - or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' }); - - $expdate = output_pref({ str => $expdate, dateonly => 1, dateformat => 'iso' }); + if ( C4::Context->preference('HourBasedHolds') ){ + if ( $resdate ){ + $resdate = dt_from_string( $resdate ); + } else { + $resdate = dt_from_string(); + } + if ( $expdate ){ + $expdate = dt_from_string( $expdate ); + } + } else { + # add hour and minute components + if ( $resdate ){ + $resdate = dt_from_string( $resdate )->set({ hour => 00, minute => 00, second => 00 }); + } else { + $resdate = dt_from_string(); + } + if ( $expdate ){ + $expdate = dt_from_string( $expdate )->set({ hour => 23, minute => 59, second => 59 }); + } + } # if we have an item selectionned, and the pickup branch is the same as the holdingbranch # of the document, we force the value $priority and $found . @@ -470,9 +486,13 @@ sub CanItemBeReserved { } } + my $dtf = Koha::Database->new->schema->storage->datetime_parser; + my $fromdate = dt_from_string->set({ hour => 00, minute => 00, second => 00 }); + my $todate = dt_from_string->set({ hour => 23, minute => 59, second => 59 }); + my $today_holds = Koha::Holds->search({ borrowernumber => $borrowernumber, - reservedate => dt_from_string->date + reservedate => [ -and => { '>=' => $dtf->format_datetime($fromdate) }, { '<=' => $dtf->format_datetime($todate) } ], }); if (!$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '' @@ -947,7 +967,7 @@ sub CancelExpiredReserves { my $expireWaiting = C4::Context->preference('ExpireReservesMaxPickUpDelay'); my $dtf = Koha::Database->new->schema->storage->datetime_parser; - my $params = { expirationdate => { '<', $dtf->format_date($today) } }; + my $params = { expirationdate => { '<', $dtf->format_datetime($today) } }; $params->{found} = [ { '!=', 'W' }, undef ] unless $expireWaiting; # FIXME To move to Koha::Holds->search_expired (?) @@ -2221,7 +2241,7 @@ sub CalculatePriority { $sql.= ' AND ( reservedate <= ? )'; } else { - $sql.= ' AND ( reservedate < NOW() )'; + $sql.= ' AND ( reservedate <= NOW() )'; } my $dbh = C4::Context->dbh(); my @row = $dbh->selectrow_array( diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 26c41d7924..38a039a8b3 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -214,7 +214,7 @@ sub set_waiting { # If patron's requested expiration date is prior to the # calculated one, we keep the patron's one. my $cmp = $requested_expiration ? DateTime->compare($requested_expiration, $expirationdate) : 0; - $values->{expirationdate} = $cmp == -1 ? $requested_expiration->ymd : $expirationdate->ymd; + $values->{expirationdate} = $cmp == -1 ? $requested_expiration : $expirationdate; $self->set($values)->store(); diff --git a/Koha/Item.pm b/Koha/Item.pm index 86d82f8637..46e24cc1f7 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -796,7 +796,7 @@ sub current_holds { itemnumber => $self->itemnumber, suspend => 0, -or => [ - reservedate => { '<=' => $dtf->format_date(dt_from_string) }, + reservedate => { '<=' => $dtf->format_datetime(dt_from_string) }, waitingdate => { '!=' => undef }, ], }; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index 516b04f322..d37882d801 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -108,13 +108,25 @@ [% hold.notes | html | html_line_break %] [% IF Koha.Preference('AllowHoldDateInFuture') %] - + [% IF Koha.Preference('HourBasedHolds') %] + + [% ELSE %] + + [% END %] [% ELSE %] - [% hold.date | $KohaDates %] + [% IF Koha.Preference('HourBasedHolds') %] + [% hold.date | $KohaDates with_hours = 1 %] + [% ELSE %] + [% hold.date | $KohaDates %] + [% END %] [% END %] - + [% IF Koha.Preference('HourBasedHolds') %] + + [% ELSE %] + + [% END %] [%- IF ( hold.found ) -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc index 27e7fb7724..9bcab200a4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/waiting_holds.inc @@ -26,7 +26,11 @@ [% END %] [% reserveloo.waitingdate | $KohaDates %] - [% reserveloo.reservedate | $KohaDates %] + [% IF Koha.Preference('HourBasedHolds') %] + [% reserveloo.reservedate | $KohaDates with_hours = 1 %] + [% ELSE %] + [% reserveloo.reservedate | $KohaDates %] + [% END %] [% INCLUDE 'biblio-title.inc' biblio=reserveloo.biblio link = 1 %] [% UNLESS ( item_level_itypes ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt index af7c47b588..3de2d0ca00 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt @@ -177,7 +177,11 @@ - [% hold.reservedate | $KohaDates %] in [% Branches.GetName ( hold.branchcode ) | html %] + [% IF Koha.Preference('HourBasedHolds') %] + [% hold.reservedate | $KohaDates with_hours = 1 %] in [% Branches.GetName ( hold.branchcode ) | html %] + [% ELSE %] + [% hold.reservedate | $KohaDates %] in [% Branches.GetName ( hold.branchcode ) | html %] + [% END %] [% hold.reservenotes | html %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt index 564ab5f54f..879a86edaa 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt @@ -191,7 +191,13 @@ [% END %] [% Branches.GetName( itemsloo.pickbranch ) | html %] - [% itemsloo.reservedate | $KohaDates %] + + [% IF Koha.Preference('HourBasedHolds') %] + [% itemsloo.reservedate | $KohaDates with_hours = 1 %] + [% ELSE %] + [% itemsloo.reservedate | $KohaDates %] + [% END %] + [% itemsloo.notes | html %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt index bd2aad334f..fce8b7f63f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt @@ -75,10 +75,13 @@ [% hold.biblio.author | html %] [% hold.item.barcode | html %] [% Branches.GetName( hold.branchcode ) | html %] - [% hold.reservedate | $KohaDates %] - - [% hold.expirationdate | $KohaDates %] - + [% IF Koha.Preference('HourBasedHolds') %] + [% hold.reservedate | $KohaDates with_hours = 1 %] + [% hold.expirationdate | $KohaDates with_hours = 1 %] + [% ELSE %] + [% hold.reservedate | $KohaDates %] + [% hold.expirationdate | $KohaDates %] + [% END %] [% hold.waitingdate | $KohaDates %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 8e9bf1fd4f..60b0cf06e1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -492,14 +492,14 @@ [% IF ( reserve_in_future ) %]
  • - +
  • [% END %]
  • - +
  • @@ -1155,6 +1155,10 @@ var startPicker = $("#reserve_date").flatpickr({ + [% IF Koha.Preference('HourBasedHolds') %] + enableTime: true, + dateFormat: flatpickr_dateformat_string + " " + flatpickr_timeformat_string, + [% END %] minDate: new Date().fp_incr(1), /* Require that "Hold starts on date" be in the future */ onChange: function(selectedDates, dateStr, instance) { altFormat = instance.formatDate(selectedDates[0], "Y-m-d"); @@ -1167,6 +1171,10 @@ }); var endPicker = $("#expiration_date").flatpickr({ + [% IF Koha.Preference('HourBasedHolds') %] + enableTime: true, + dateFormat: flatpickr_dateformat_string + " " + flatpickr_timeformat_string, + [% END %] minDate: new Date().fp_incr(1), // Require that "Hold expires on date" be in the future onChange: function(selectedDates, dateStr, instance) { altFormat = instance.formatDate(selectedDates[0], "Y-m-d"); @@ -1177,6 +1185,13 @@ }, }); + [% IF Koha.Preference('HourBasedHolds') %] + $(".holdst_reservedate, .futuredate").flatpickr({ + enableTime: true, + dateFormat: flatpickr_dateformat_string + " " + flatpickr_timeformat_string, + }); + [% END %] + var my_table = $("#requestspecific").dataTable($.extend(true, {}, dataTablesDefaults, { 'bPaginate': false, "sDom": '<"top pager"ilf>t', diff --git a/svc/holds b/svc/holds index 77c260ff27..e70b7bbeb6 100755 --- a/svc/holds +++ b/svc/holds @@ -114,19 +114,32 @@ while ( my $h = $holds_rs->next() ) { waiting_here => $h->branch()->branchcode() eq $branch, priority => $h->priority(), itemtype_limit => $itemtype_limit, - reservedate_formatted => $h->reservedate() ? output_pref( - { dt => dt_from_string( $h->reservedate() ), dateonly => 1 } - ) - : q{}, suspend_until_formatted => $h->suspend_until() ? output_pref( { dt => dt_from_string( $h->suspend_until() ), dateonly => 1 } ) : q{}, - expirationdate_formatted => $h->expirationdate() ? output_pref( + }; + + if ( C4::Context->preference('HourBasedHolds') ){ + $hold->{reservedate_formatted} = $h->reservedate() ? output_pref( + { dt => dt_from_string( $h->reservedate() ) } + ) + : q{}, + $hold->{expirationdate_formatted} = $h->expirationdate() ? output_pref( + { dt => dt_from_string( $h->expirationdate() ) } + ) + : q{}, + } else { + $hold->{reservedate_formatted} = $h->reservedate() ? output_pref( + { dt => dt_from_string( $h->reservedate() ), dateonly => 1 } + ) + : q{}, + $hold->{expirationdate_formatted} = $h->expirationdate() ? output_pref( { dt => dt_from_string( $h->expirationdate() ), dateonly => 1 } ) : q{}, - }; + } + $hold->{transfered} = 0; $hold->{not_transfered} = 0; diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index b62bc123bd..a5d7c57636 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 72; +use Test::More tests => 73; use MARC::Record; use C4::Biblio; @@ -102,7 +102,7 @@ my $reservedate = $first_hold->reservedate; my $borrowernumber = $first_hold->borrowernumber; my $branch_1code = $first_hold->branchcode; my $reserve_id = $first_hold->reserve_id; -is( $reservedate, output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }), "holds_placed_today should return a valid reserve date"); +is( dt_from_string($reservedate), dt_from_string, "holds_placed_today should return a valid reserve date"); is( $borrowernumber, $borrowernumbers[0], "holds_placed_today should return a valid borrowernumber"); is( $branch_1code, $branch_1, "holds_placed_today should return a valid branchcode"); ok($reserve_id, "Test holds_placed_today()"); @@ -922,7 +922,7 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { is_deeply( CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), { status => 'tooManyReservesToday', limit => 2 }, - 'Patron cannot a third item with 2 reserves daily cap' + 'Patron cannot reserve a third item with 2 reserves daily cap' ); # Update last hold so reservedate is in the past, so 2 holds, but different day @@ -1658,10 +1658,77 @@ subtest 'ModReserve can only update expirationdate for found holds' => sub { $hold = Koha::Holds->find($reserve_id); - is( $hold->expirationdate, '1981-06-10', + is( dt_from_string( $hold->expirationdate )->ymd, '1981-06-10', 'Found hold expiration date updated correctly' ); is( $hold->priority, '0', 'Found hold priority was not updated' ); $schema->storage->txn_rollback; }; + +subtest 'HourBasedHolds' => sub { + plan tests => 4; + $schema->storage->txn_begin; + + my $category = $builder->build({ source => 'Category' }); + my $branch = $builder->build({ source => 'Branch' })->{ branchcode }; + my $biblio = $builder->build_sample_biblio( { itemtype => 'DUMMY' } ); + my $itemnumber = $builder->build_sample_item( + { library => $branch, biblionumber => $biblio->biblionumber } ) + ->itemnumber; + + my $borrowernumber = Koha::Patron->new( + { + firstname => 'firstname', + surname => 'surname', + categorycode => $category->{categorycode}, + branchcode => $branch, + } + )->store->borrowernumber; + + t::lib::Mocks::mock_preference('HourBasedHolds', 1); + + my $reservation_date = DateTime->now->set( hour => 7, minute => 30 ); + my $expiration_date = DateTime->now->set( hour => 19, minute => 30 ); + + my $reserve_id = AddReserve( + { + branchcode => $branch, + borrowernumber => $borrowernumber, + biblionumber => $biblio->biblionumber, + priority => + C4::Reserves::CalculatePriority( $biblio->biblionumber ), + itemnumber => $itemnumber, + reservation_date => $reservation_date, + expiration_date => $expiration_date, + } + ); + + my $hold = Koha::Holds->find($reserve_id); + is( dt_from_string( $hold->reservedate )->hour, 07, "Hold reserve date is set correctly with hours when HourBasedHolds enabled" ); + is( dt_from_string( $hold->expirationdate )->hour, 19, "Hold expiration date is set correctly with hours when HourBasedHolds enabled" ); + + t::lib::Mocks::mock_preference('HourBasedHolds', 0); + + $reservation_date = DateTime->now->ymd; + $expiration_date = DateTime->now->ymd; + + $reserve_id = AddReserve( + { + branchcode => $branch, + borrowernumber => $borrowernumber, + biblionumber => $biblio->biblionumber, + priority => + C4::Reserves::CalculatePriority( $biblio->biblionumber ), + itemnumber => $itemnumber, + reservation_date => $reservation_date, + expiration_date => $expiration_date, + } + ); + + $hold = Koha::Holds->find($reserve_id); + is( dt_from_string( $hold->reservedate )->hour, 00, "Hold reserve date is set correctly when HourBasedHolds disabled" ); + is( dt_from_string( $hold->expirationdate )->hour, 23, "Hold expiration date is set correctly when HourBasedHolds disabled" ); + + $schema->storage->txn_rollback; +}; -- 2.11.0