Bug 24718

Summary: Introduce hour-based holds
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: Hold requestsAssignee: Aleisha Amohia <aleisha>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bwsdonna, david, emmi.takkinen, gmcharlt, katrin.fischer, martin.renvoize, mspinney, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20453
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29002
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Get reservedate and expirationdate to use datetime
Bug 24718: Get templates working with HourBasedHolds
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds
Bug 24718: Adding notice to be sent when cancelling holds
Bug 24718: Send notice when cancelling a hold from intranet
Bug 24718: Ensuring tests pass
Bug 24718: (follow-up) Removing old code
Bug 24718: (follow-up) Updated schema file
Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Get reservedate and expirationdate to use datetime
Bug 24718: Get templates working with HourBasedHolds
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds
Bug 24718: Ensuring tests pass
Bug 24718: (follow-up) Removing old code
Bug 24718: (follow-up) Updated schema file
Bug 24718: Get reservedate and expirationdate to use datetime
Bug 24718: Get templates working with HourBasedHolds
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds
Bug 24718: Ensuring tests pass
Bug 24718: (follow-up) Removing old code
Bug 24718: (follow-up) Updated schema file
Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Get reservedate and expirationdate to use datetime
Bug 24718: Get templates working with HourBasedHolds
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds
Bug 24718: Ensuring tests pass
Bug 24718: (follow-up) Removing old code
Bug 24718: (follow-up) Updated schema file
Bug 24718: (follow-up) Update syspref description, fix expdate time when setting waiting
Test failure after patches
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Bug 24718: Updated schema files
Bug 24718: Hour based holds on staff client
Bug 24718: Hour based holds on OPAC
Bug 24718: Holds API fixes for datetime
Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Bug 24718: Hour based holds on staff client
Bug 24718: Hour based holds on OPAC
Bug 24718: Holds API fixes for datetime
Bug 24718: Updated schema files
Bug 24718: Introducing HourBasedHolds system preference
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Bug 24718: Hour based holds on staff client
Bug 24718: Hour based holds on OPAC
Bug 24718: Holds API fixes for datetime
Bug 24718: Updated schema files

Description Aleisha Amohia 2020-02-24 20:04:23 UTC
Some libraries would like to be able to specify the hours which an item will be reserved for (think: 'booking'), for example with 2-hour loans.
Comment 1 Aleisha Amohia 2020-02-24 21:39:08 UTC
Created attachment 99540 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology
Comment 2 Aleisha Amohia 2020-02-24 21:39:13 UTC
Created attachment 99541 [details] [review]
Bug 24718: Get reservedate and expirationdate to use datetime

Sponsored-by: Waikato Institute of Technology
Comment 3 Aleisha Amohia 2020-02-24 21:39:18 UTC
Created attachment 99542 [details] [review]
Bug 24718: Get templates working with HourBasedHolds

Sponsored-by: Waikato Institute of Technology
Comment 4 Aleisha Amohia 2020-02-24 21:39:22 UTC
Created attachment 99543 [details] [review]
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds

Sponsored-by: Waikato Institute of Technology
Comment 5 Aleisha Amohia 2020-02-24 21:39:26 UTC
Created attachment 99544 [details] [review]
Bug 24718: Adding notice to be sent when cancelling holds

Sponsored-by: Waikato Institute of Technology
Comment 6 Aleisha Amohia 2020-02-24 21:39:30 UTC
Created attachment 99545 [details] [review]
Bug 24718: Send notice when cancelling a hold from intranet

Sponsored-by: Waikato Institute of Technology
Comment 7 Katrin Fischer 2020-02-24 21:45:42 UTC
Hi Aleisha, just reading the commit subjects I wonder if the notice for cancelling holds could be a separate feature?
Comment 8 Aleisha Amohia 2020-02-24 21:49:35 UTC
(In reply to Katrin Fischer from comment #7)
> Hi Aleisha, just reading the commit subjects I wonder if the notice for
> cancelling holds could be a separate feature?

Not really, the sending of notices was part of the feature we wrote for this library and depends on a syspref that this feature introduces. This will be explain in the test plan I'm currently writing
Comment 9 Katrin Fischer 2020-02-24 22:08:15 UTC
Ok thx - I was thinking of something like bug 20453.
Comment 10 Aleisha Amohia 2020-02-24 22:11:19 UTC
Hmm I see. I guess could obselete the final two patches here and put them on Bug 24053 so that the notice is always sent, without depending on a syspref
Comment 11 Katrin Fischer 2020-02-24 22:14:38 UTC
If it goes the direction of the other bug, I think not having it necessarily depend on hourly loans would be nice. Maybe we could also do it later. I am adding a see also for now and will let you think it over to what works best here (don't know much about the feature written yet!) :)
Comment 12 Aleisha Amohia 2020-02-24 22:28:40 UTC
The library wanted the ability to specify hours when reserving items for patrons, and be able to cancel those reserves - a process of 'booking' something for a few hours, with the ability to reject (cancel) the booking. The patron would then be notified if the reserve was rejected/cancelled by the library (this part is handled in Bug 20453). The cancel_expired_holds.pl cronjob can be run hourly to work with this feature. 

The library only needed the hour-based holds functionality on the staff side as their patrons can't placed holds via the OPAC (RequestOnOpac disabled) but I have put the functionality on the OPAC side for the option.

Test plan for Bug 24718: hour-based holds

You will need two borrowers (Borrower A and B) and a record with items (Biblio A)

1) Update database, restart memcached
2) Enable the following sysprefs: RequestOnOpac, AllowHoldDateInFuture, OPACAllowHoldDateInFuture, HourBasedHolds
3) Log into the OPAC as Borrower A
4) Find Biblio A
5) Click 'Place a reserve'
6) Click 'Show more options'
7) Try entering dates in the 'starts on' field and 'not needed after' field. Notice you can also enter a time. Leave the start date empty and choose a specific end date/time. Confirm the hold
8) Confirm the hold is saved with the reserve date set to today/now and the expiry date is saved as specified. Notice the time selection is shown.
9) Log into the staff interface
10) Find Biblio A and place a hold for Borrower B
11) Try entering dates in the 'starts on' field and 'expires on' field. Notice you can also enter a time. Specify a future start date. Confirm the hold
12) Confirm your selected date and time shows in the confirmed hold. View all of the pages that show holds and confirm they all show the date and time correctly.

Sponsored-by: Waikato Institute of Technology
Comment 13 Aleisha Amohia 2020-02-24 22:46:19 UTC
Tests coming
Comment 14 Aleisha Amohia 2020-02-25 00:32:50 UTC
Created attachment 99548 [details] [review]
Bug 24718: Ensuring tests pass

Confirm these tests pass before and after applying patches

t/db_dependent/Holds.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Reserves/CancelExpiredReserves.t
Comment 15 Aleisha Amohia 2020-02-25 00:33:16 UTC
(In reply to Aleisha Amohia from comment #12)
> The library wanted the ability to specify hours when reserving items for
> patrons, and be able to cancel those reserves - a process of 'booking'
> something for a few hours, with the ability to reject (cancel) the booking.
> The patron would then be notified if the reserve was rejected/cancelled by
> the library (this part is handled in Bug 20453). The cancel_expired_holds.pl
> cronjob can be run hourly to work with this feature. 
> 
> The library only needed the hour-based holds functionality on the staff side
> as their patrons can't placed holds via the OPAC (RequestOnOpac disabled)
> but I have put the functionality on the OPAC side for the option.
> 
> Test plan for Bug 24718: hour-based holds
> 
> You will need two borrowers (Borrower A and B) and a record with items
> (Biblio A)
> 
> 1) Update database, restart memcached

Remember to update dbix files!

> 2) Enable the following sysprefs: RequestOnOpac, AllowHoldDateInFuture,
> OPACAllowHoldDateInFuture, HourBasedHolds
> 3) Log into the OPAC as Borrower A
> 4) Find Biblio A
> 5) Click 'Place a reserve'
> 6) Click 'Show more options'
> 7) Try entering dates in the 'starts on' field and 'not needed after' field.
> Notice you can also enter a time. Leave the start date empty and choose a
> specific end date/time. Confirm the hold
> 8) Confirm the hold is saved with the reserve date set to today/now and the
> expiry date is saved as specified. Notice the time selection is shown.
> 9) Log into the staff interface
> 10) Find Biblio A and place a hold for Borrower B
> 11) Try entering dates in the 'starts on' field and 'expires on' field.
> Notice you can also enter a time. Specify a future start date. Confirm the
> hold
> 12) Confirm your selected date and time shows in the confirmed hold. View
> all of the pages that show holds and confirm they all show the date and time
> correctly.
> 
> Sponsored-by: Waikato Institute of Technology
Comment 16 David Nind 2020-04-22 09:15:06 UTC
Hi Aleisha.

Everything seems to work as expected according to the test plan.

There is one failed test though (after the patch is applied):

root@40629b80bd10:koha(bz24718)$ prove t/db_dependent/Holds.t
t/db_dependent/Holds.t .. 1/61 
#   Failed test 'holds_placed_today should return a valid borrowernumber'
#   at t/db_dependent/Holds.t line 104.
#          got: '247'
#     expected: '246'
t/db_dependent/Holds.t .. 61/61 # Looks like you failed 1 test of 61.
t/db_dependent/Holds.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/61 subtests 

Test Summary Report
-------------------
t/db_dependent/Holds.t (Wstat: 256 Tests: 61 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=1, Tests=61,  7 wallclock secs ( 0.04 usr  0.00 sys +  5.63 cusr  0.52 csys =  6.19 CPU)
Result: FAIL

In addition, there is also an error happening after the patches are applied when you go the patrons details page and the holds tab:

> DataTables warning: table id=holds-table - Ajax error. For more information 
> about this error, please see http://datatables.net/tn/7

I've reported this error before on bug 24769 - it only seems to happen for me when testing ILS-DI bugs, but is now happening for this bug as well.
Comment 17 Aleisha Amohia 2020-04-22 23:12:56 UTC
Created attachment 103521 [details] [review]
Bug 24718: (follow-up) Removing old code

This solves the JS error
Comment 18 Aleisha Amohia 2020-04-22 23:12:59 UTC
Created attachment 103522 [details] [review]
Bug 24718: (follow-up) Updated schema file

For sandboxes
Comment 19 David Nind 2020-04-23 10:03:53 UTC
Created attachment 103549 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 David Nind 2020-04-23 10:03:56 UTC
Created attachment 103550 [details] [review]
Bug 24718: Get reservedate and expirationdate to use datetime

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2020-04-23 10:04:00 UTC
Created attachment 103551 [details] [review]
Bug 24718: Get templates working with HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 David Nind 2020-04-23 10:04:04 UTC
Created attachment 103552 [details] [review]
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2020-04-23 10:04:08 UTC
Created attachment 103553 [details] [review]
Bug 24718: Ensuring tests pass

Confirm these tests pass before and after applying patches

t/db_dependent/Holds.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Reserves/CancelExpiredReserves.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2020-04-23 10:04:11 UTC
Created attachment 103554 [details] [review]
Bug 24718: (follow-up) Removing old code

This solves the JS error

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 David Nind 2020-04-23 10:04:15 UTC
Created attachment 103555 [details] [review]
Bug 24718: (follow-up) Updated schema file

For sandboxes

Signed-off-by: David Nind <david@davidnind.com>
Comment 26 Aleisha Amohia 2020-06-24 23:34:01 UTC
Created attachment 106270 [details] [review]
Bug 24718: Get reservedate and expirationdate to use datetime

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 Aleisha Amohia 2020-06-24 23:34:05 UTC
Created attachment 106271 [details] [review]
Bug 24718: Get templates working with HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 Aleisha Amohia 2020-06-24 23:34:09 UTC
Created attachment 106272 [details] [review]
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 29 Aleisha Amohia 2020-06-24 23:34:13 UTC
Created attachment 106273 [details] [review]
Bug 24718: Ensuring tests pass

Confirm these tests pass before and after applying patches

t/db_dependent/Holds.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Reserves/CancelExpiredReserves.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 30 Aleisha Amohia 2020-06-24 23:34:16 UTC
Created attachment 106274 [details] [review]
Bug 24718: (follow-up) Removing old code

This solves the JS error

Signed-off-by: David Nind <david@davidnind.com>
Comment 31 Aleisha Amohia 2020-06-24 23:34:20 UTC
Created attachment 106275 [details] [review]
Bug 24718: (follow-up) Updated schema file

For sandboxes

Signed-off-by: David Nind <david@davidnind.com>
Comment 32 Nick Clemens 2020-08-06 18:11:37 UTC
Hi Aleisha,

The comment on the syspref says jobs will be run hourly - probably should say something like:
NOTE: This system preference requires the misc/cronjobs/holds/ cronjobs. Ask your system administrator to schedule them. 

When confirming a hold the max pickup delay is added for days correctly, however, the time part is set to 00:00:00 - I thin it should be preserved from original setting or set to 23:59:59

t/db_dependent/api/v1/holds.t fails - should we accept a simple date and use 23:59:59 if time not specified? I am not sure how it is handled for checkouts
Comment 33 Aleisha Amohia 2020-09-15 01:59:21 UTC
Created attachment 110084 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 34 Aleisha Amohia 2020-09-15 01:59:28 UTC
Created attachment 110085 [details] [review]
Bug 24718: Get reservedate and expirationdate to use datetime

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 35 Aleisha Amohia 2020-09-15 01:59:33 UTC
Created attachment 110086 [details] [review]
Bug 24718: Get templates working with HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 36 Aleisha Amohia 2020-09-15 01:59:38 UTC
Created attachment 110087 [details] [review]
Bug 24718: Get OPAC working with datetimepicker and HourBasedHolds

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 37 Aleisha Amohia 2020-09-15 01:59:43 UTC
Created attachment 110088 [details] [review]
Bug 24718: Ensuring tests pass

Confirm these tests pass before and after applying patches

t/db_dependent/Holds.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Reserves/CancelExpiredReserves.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 38 Aleisha Amohia 2020-09-15 01:59:49 UTC
Created attachment 110089 [details] [review]
Bug 24718: (follow-up) Removing old code

This solves the JS error

Signed-off-by: David Nind <david@davidnind.com>
Comment 39 Aleisha Amohia 2020-09-15 01:59:53 UTC
Created attachment 110090 [details] [review]
Bug 24718: (follow-up) Updated schema file

For sandboxes

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 Aleisha Amohia 2020-09-15 01:59:58 UTC
Created attachment 110091 [details] [review]
Bug 24718: (follow-up) Update syspref description, fix expdate time when setting waiting
Comment 41 Aleisha Amohia 2020-09-15 02:01:32 UTC
(In reply to Nick Clemens from comment #32)
> Hi Aleisha,
> 
> The comment on the syspref says jobs will be run hourly - probably should
> say something like:
> NOTE: This system preference requires the misc/cronjobs/holds/ cronjobs. Ask
> your system administrator to schedule them. 

Fixed 

> 
> When confirming a hold the max pickup delay is added for days correctly,
> however, the time part is set to 00:00:00 - I thin it should be preserved
> from original setting or set to 23:59:59

Fixed (note, this is for when setting a hold as waiting). It now preserves minutes and seconds

> 
> t/db_dependent/api/v1/holds.t fails - should we accept a simple date and use
> 23:59:59 if time not specified? I am not sure how it is handled for checkouts

The same tests in this test file fail for me, before and after the patches, so I can't test this one. Can you please paste the output for what fails and be more specific when you say 'should we accept a simple date'?

Thanks!
Comment 42 Nick Clemens 2020-09-15 11:25:27 UTC
(In reply to Aleisha Amohia from comment #41)
> > t/db_dependent/api/v1/holds.t fails - should we accept a simple date and use
> > 23:59:59 if time not specified? I am not sure how it is handled for checkouts
> 
> The same tests in this test file fail for me, before and after the patches,
> so I can't test this one. Can you please paste the output for what fails and
> be more specific when you say 'should we accept a simple date'?
> 
> Thanks!

I filed bug 26463 - the tests fail for me if AllowHoldPolicyOverride is set, otherwise they pass before these patches.

I will attach a list of failures I see after the patches.

By 'simple date' I meant we should not expect/require the time part of the date, but fall back to a default. Though I am not sure where I saw that as needed

It may be the problem is in api/v1/swagger/definitions/hold.json where hold_date and expiration_date are defined as "format": "date" and may need to be "date-time" with these patches?
Comment 43 Nick Clemens 2020-09-15 11:25:58 UTC
Created attachment 110098 [details]
Test failure after patches
Comment 44 Aleisha Amohia 2020-09-15 23:08:50 UTC
(In reply to Nick Clemens from comment #42)

> It may be the problem is in api/v1/swagger/definitions/hold.json where
> hold_date and expiration_date are defined as "format": "date" and may need
> to be "date-time" with these patches?

Thank you for the suggestion, I'll try this.
Comment 45 Aleisha Amohia 2020-09-16 01:33:18 UTC
Created attachment 110137 [details] [review]
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Comment 46 Aleisha Amohia 2020-09-16 01:34:08 UTC
Nick, I didn't really know what to do other than remove dateonly flags from all the date options. I don't think it worked. Feel free to amend that last patch on my behalf.
Comment 47 Nick Clemens 2020-09-16 10:16:39 UTC
Created attachment 110161 [details] [review]
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Comment 48 Nick Clemens 2020-09-16 10:22:06 UTC
(In reply to Aleisha Amohia from comment #46)
> Nick, I didn't really know what to do other than remove dateonly flags from
> all the date options. I don't think it worked. Feel free to amend that last
> patch on my behalf.

Patch amended. Just needed to set the format to date-time everywhere - still one failure, but it looks like that comes from C4::Reserves::AddReserve - the time part is set based on the HourBasedHolds pref, so we need to mock that or change the assumptions I tihnk
Comment 49 Aleisha Amohia 2020-09-17 22:10:05 UTC
Created attachment 110303 [details] [review]
Bug 24718: (follow-up) Attempting to fix Koha REST holds
Comment 50 Tomás Cohen Arazi 2020-09-17 22:21:47 UTC
Without having looked at the patchset and how the API is to be used, it feels like we should be adding a new (optional) field for either date-time case, or the time portion. Depending on the use we would make of it.

We could also leave the route alone and have a special route for hourly holds /holds/hourly
Comment 51 Martin Renvoize 2021-09-13 10:37:20 UTC
This looks interesting.. could we get a rebase?
Comment 52 Aleisha Amohia 2021-10-20 04:04:34 UTC
Created attachment 126559 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 53 Aleisha Amohia 2021-10-20 04:04:39 UTC
Created attachment 126560 [details] [review]
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Comment 54 Aleisha Amohia 2021-10-20 04:04:44 UTC
Created attachment 126561 [details] [review]
Bug 24718: Updated schema files
Comment 55 Aleisha Amohia 2021-10-20 04:04:48 UTC
Created attachment 126562 [details] [review]
Bug 24718: Hour based holds on staff client

Tests in t/db_dependent/Holds.t
Comment 56 Aleisha Amohia 2021-10-20 04:04:53 UTC
Created attachment 126563 [details] [review]
Bug 24718: Hour based holds on OPAC
Comment 57 Aleisha Amohia 2021-10-20 04:04:58 UTC
Created attachment 126564 [details] [review]
Bug 24718: Holds API fixes for datetime
Comment 58 Aleisha Amohia 2021-10-20 04:10:29 UTC
I have rebased the patches and fixed the API test that was failing.

Note I removed the functionality to place hour-based holds on the OPAC because it should use the new flatpickr plugin which still needs work Bug 29212 . That can be added later.
Comment 59 Aleisha Amohia 2023-03-16 23:23:17 UTC
Created attachment 148293 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 60 Aleisha Amohia 2023-03-16 23:23:21 UTC
Created attachment 148294 [details] [review]
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Comment 61 Aleisha Amohia 2023-03-16 23:23:24 UTC
Created attachment 148295 [details] [review]
Bug 24718: Hour based holds on staff client

Tests in t/db_dependent/Holds.t
Comment 62 Aleisha Amohia 2023-03-16 23:23:27 UTC
Created attachment 148296 [details] [review]
Bug 24718: Hour based holds on OPAC
Comment 63 Aleisha Amohia 2023-03-16 23:23:30 UTC
Created attachment 148297 [details] [review]
Bug 24718: Holds API fixes for datetime
Comment 64 Aleisha Amohia 2023-03-16 23:25:33 UTC
Created attachment 148298 [details] [review]
Bug 24718: Updated schema files
Comment 65 Emmi Takkinen 2023-08-17 08:38:54 UTC
There is a minor merge conflict in t/db_dependent/Holds.t while trying to apply patch "Bug 24718: Hour based holds on staff client". I was able to resolve this, but Koha now dies on Internal Server Error. Looking from locks there are these lines:

Global symbol "$expdate" requires explicit package name (did you forget to declare "my $expdate"?) at ../Koha/C4/Reserves.pm line 216." (and several others)

Type of arg 1 to List::MoreUtils::XS::any must be block or sub {} (not reference constructor) at ../Koha/C4/Reserves.pm line 1848, near "$ignore_borrowers ;" 

Setting this status as "Patch doesn't apply".
Comment 66 Katrin Fischer 2024-01-06 13:29:58 UTC
We now have the base for a booking feature in Koha. It could make sense to build this functionality on top of that now.
Comment 67 Aleisha Amohia 2024-02-27 01:17:21 UTC
Created attachment 162474 [details] [review]
Bug 24718: Introducing HourBasedHolds system preference

Sponsored-by: Waikato Institute of Technology

Signed-off-by: David Nind <david@davidnind.com>
Comment 68 Aleisha Amohia 2024-02-27 01:17:25 UTC
Created attachment 162475 [details] [review]
Bug 24718: Modify reserves.reservedate and expirationdate to use datetime
Comment 69 Aleisha Amohia 2024-02-27 01:17:29 UTC
Created attachment 162476 [details] [review]
Bug 24718: Hour based holds on staff client

Tests in t/db_dependent/Holds.t
Comment 70 Aleisha Amohia 2024-02-27 01:17:33 UTC
Created attachment 162477 [details] [review]
Bug 24718: Hour based holds on OPAC
Comment 71 Aleisha Amohia 2024-02-27 01:17:36 UTC
Created attachment 162478 [details] [review]
Bug 24718: Holds API fixes for datetime
Comment 72 Aleisha Amohia 2024-02-27 01:17:40 UTC
Created attachment 162479 [details] [review]
Bug 24718: Updated schema files
Comment 73 Aleisha Amohia 2024-02-27 01:19:34 UTC
(In reply to Katrin Fischer from comment #66)
> We now have the base for a booking feature in Koha. It could make sense to
> build this functionality on top of that now.

I've rebased this for main because one of our library's uses it in production, bu I absolutely agree. Setting to In Discussion until someone has capacity to take that on.