Bugzilla – Attachment 126562 Details for
Bug 24718
Introduce hour-based holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24718: Hour based holds on staff client
Bug-24718-Hour-based-holds-on-staff-client.patch (text/plain), 21.51 KB, created by
Aleisha Amohia
on 2021-10-20 04:04:48 UTC
(
hide
)
Description:
Bug 24718: Hour based holds on staff client
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2021-10-20 04:04:48 UTC
Size:
21.51 KB
patch
obsolete
>From 07478955c2356923fa51e8979beb1936b66c8cef Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >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 @@ > <td>[% hold.notes | html | html_line_break %]</td> > <td> > [% IF Koha.Preference('AllowHoldDateInFuture') %] >- <input type="text" class="flatpickr" value="[% hold.date | $KohaDates %]" required="required" size="10" name="reservedate" /> >+ [% IF Koha.Preference('HourBasedHolds') %] >+ <input type="text" class="holdst_reservedate" value="[% hold.date | $KohaDates with_hours = 1 %]" required="required" size="15" name="reservedate" /> >+ [% ELSE %] >+ <input type="text" class="flatpickr" value="[% hold.date | $KohaDates %]" required="required" size="15" name="reservedate" /> >+ [% END %] > [% ELSE %] >- [% hold.date | $KohaDates %] >+ [% IF Koha.Preference('HourBasedHolds') %] >+ [% hold.date | $KohaDates with_hours = 1 %] >+ [% ELSE %] >+ [% hold.date | $KohaDates %] >+ [% END %] > [% END %] > </td> > <td> >- <input type="text" class="flatpickr futuredate" value="[% hold.expirationdate | $KohaDates %]" size="10" name="expirationdate" /> >+ [% IF Koha.Preference('HourBasedHolds') %] >+ <input type="text" class="futuredate" value="[% hold.expirationdate | $KohaDates with_hours = 1 %]" size="15" name="expirationdate" /> >+ [% ELSE %] >+ <input type="text" class="flatpickr futuredate" value="[% hold.expirationdate | $KohaDates %]" size="15" name="expirationdate" /> >+ [% END %] > </td> > <td> > [%- 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 @@ > <th><input type="checkbox" class="select_hold" data-id="[% reserveloo.reserve_id | html %]"/></th> > [% END %] > <td><span title="[% reserveloo.waitingdate | html %]">[% reserveloo.waitingdate | $KohaDates %]</span></td> >- <td><span title="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates %]</span></td> >+ [% IF Koha.Preference('HourBasedHolds') %] >+ <td><span title="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates with_hours = 1 %]</span></td> >+ [% ELSE %] >+ <td><span title="[% reserveloo.reservedate | html %]">[% reserveloo.reservedate | $KohaDates %]</span></td> >+ [% END %] > <td> > [% 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 @@ > </ul> > </td> > <td data-order="[% hold.reservedate | html %]"> >- [% 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 %] > </td> > <td>[% hold.reservenotes | html %]</td> > <td> >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 %] > </td> > <td class="hq-sendto">[% Branches.GetName( itemsloo.pickbranch ) | html %]</td> >- <td class="hq-date" data-order="[% itemsloo.reservedate | html %]">[% itemsloo.reservedate | $KohaDates %]</td> >+ <td class="hq-date" data-order="[% itemsloo.reservedate | html %]"> >+ [% IF Koha.Preference('HourBasedHolds') %] >+ [% itemsloo.reservedate | $KohaDates with_hours = 1 %] >+ [% ELSE %] >+ [% itemsloo.reservedate | $KohaDates %] >+ [% END %] >+ </td> > <td class="hq-notes">[% itemsloo.notes | html %]</td> > </tr> > [% END %]</tbody> >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 @@ > <td>[% hold.biblio.author | html %]</td> > <td>[% hold.item.barcode | html %]</td> > <td>[% Branches.GetName( hold.branchcode ) | html %]</td> >- <td data-order="[% hold.reservedate | html %]">[% hold.reservedate | $KohaDates %]</td> >- <td data-order="[% hold.expirationdate | html %]"> >- [% hold.expirationdate | $KohaDates %] >- </td> >+ [% IF Koha.Preference('HourBasedHolds') %] >+ <td data-order="[% hold.reservedate | html %]">[% hold.reservedate | $KohaDates with_hours = 1 %]</td> >+ <td data-order="[% hold.expirationdate | html %]">[% hold.expirationdate | $KohaDates with_hours = 1 %]</td> >+ [% ELSE %] >+ <td data-order="[% hold.reservedate | html %]">[% hold.reservedate | $KohaDates %]</td> >+ <td data-order="[% hold.expirationdate | html %]">[% hold.expirationdate | $KohaDates %]</td> >+ [% END %] > <td data-order="[% hold.waitingdate | html %]"> > [% hold.waitingdate | $KohaDates %] > </td> >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 ) %] > <li> > <label for="from">Hold starts on date:</label> >- <input id="reserve_date" name="reserve_date" id="from" size="10" type="text" > >+ <input id="reserve_date" name="reserve_date" id="from" size="15" type="text" > > <input type="hidden" class="datepickerfrom_hidden" /> > </li> > [% END %] > > <li> > <label for="to">Hold expires on date:</label> >- <input id="expiration_date" name="expiration_date" id="to" size="10" type="text" /> >+ <input id="expiration_date" name="expiration_date" id="to" size="15" type="text" /> > <input type="hidden" class="datepickerto_hidden" /> > </li> > >@@ -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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=126562">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24718
:
99540
|
99541
|
99542
|
99543
|
99544
|
99545
|
99548
|
103521
|
103522
|
103549
|
103550
|
103551
|
103552
|
103553
|
103554
|
103555
|
106270
|
106271
|
106272
|
106273
|
106274
|
106275
|
110084
|
110085
|
110086
|
110087
|
110088
|
110089
|
110090
|
110091
|
110098
|
110137
|
110161
|
110303
|
126559
|
126560
|
126561
|
126562
|
126563
|
126564
|
148293
|
148294
|
148295
|
148296
|
148297
|
148298
|
162474
|
162475
|
162476
|
162477
|
162478
|
162479