Bug 36271 - Bookings should respect circulation rules for max loan periods
Summary: Bookings should respect circulation rules for max loan periods
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 35906
Blocks: 36789
  Show dependency treegraph
 
Reported: 2024-03-07 12:22 UTC by Martin Renvoize
Modified: 2024-05-16 20:10 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 36271 : Add circulation rules for booking (19.05 KB, patch)
2024-03-20 14:40 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 36271 : Add circulation rules for booking (16.85 KB, patch)
2024-04-10 14:08 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 36271 : Add circulation rules for booking (17.19 KB, patch)
2024-04-18 10:07 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 36271 : Add circulation rules for booking (17.18 KB, patch)
2024-04-18 10:11 UTC, Thibaud Guillot
Details | Diff | Splinter Review
Bug 36271: Add circulation rules for booking (17.98 KB, patch)
2024-05-16 09:10 UTC, Thibaud Guillot
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2024-03-07 12:22:45 UTC
Currently, bookings are on/off at the item level, but the booking period is entirely open meaning an item can be booked for any period.

We should refine this to allow libraries to control the maximum length an item can be booked for.

During the original development, I suggested perhaps we should set the maximum to the loan length of the item times the max renewals from the circulation rules.. However, I didn't get much feedback as to whether this was appropriate or whether people wanted a distinct rule for these items or another alternative.
Comment 1 Caroline Cyr La Rose 2024-03-07 15:26:00 UTC
Hi Martin,

When I was testing bookings, I was thinking of a separate parameter for the booking period. However, using the loan length makes sense. I'm not sure about the renewals. Usually, people can renew IF no one else needs the item (i.e. no holds).  But booking for the entire renewal period would remove the option of someone placing a hold on it and getting it after a single loan period, no?

I feel like whatever we choose (loan length or separate rule), someone will want the other option lol! Maybe we could do a separate parameter whose default option is the items loan length?

I'd be curious to know what people using it IRL think. I seem to remember Bywater had libraries who were anxiously awaiting this feature, maybe they would have a more informed opinion than mine.
Comment 2 Thibaud Guillot 2024-03-07 16:11:04 UTC
Hello :) 

I'm actually working on that, for now, I defined a CanItemBeBookedInAdvance function in Bookings to deal with some rules, 4 in fact : 
- booking_total_allowed
- booking_per_item
- booking_per_itemtype ( related to bug 35906 )
- booking_period_length

Does this seem useful to you?
Comment 3 Kristi 2024-03-13 21:27:02 UTC
For us (one of the ByWater libraries), we would picture this as tied in to the circulation rules for item types. 

We have item types set up for the portions of our collection that can be booked. A kit that staff book for story hours, for example, has an item type of Staff Program Support and that item type has its own circ period (1 day plus built in prep time from the booking module) and number of renewals allowed (0).
Comment 4 Martin Renvoize 2024-03-14 09:46:04 UTC
Thanks everyone, this is great feedback.

Really interested to see your work once there's things to look at Thibuad. I'm happy with the direction the itemtype bookable code has taken, though of course we need a signoff there before it can reach the next steps.

I've been looking into the lead-in and trail-out times work on another bug and trying to work through where to place those rules too.. circ level, system preference level etc.

The combination of patron/branch/itemtype could be rather fun to deal with performance-wise with the current datepicker handling so that needs some considerations here whilst we're digging through.

It would be great to see bug 35906 (bookable at itemtype level) tested before we progress further here in my opinion.
Comment 5 Thibaud Guillot 2024-03-20 14:40:07 UTC
Created attachment 163546 [details] [review]
Bug 36271 : Add circulation rules for booking

Welcome to 4 new circulation rules :)

- Bookings allowed (total) / bookings_allowed_total
- Bookings per item (total) / bookings_per_item
- Bookings per itemtype (total) / bookings per itemtype
- Bookings period length (day) / bookings_period_length

Test plan:
1) Apply this patch
2) Set values for these new rules
** Note that rules respect the item setting bookable:
   - bookable to "yes" => bookings per item rule
   - bookable to "follow item type" => bookings per itemtype rule
   - bookable to 'no' => no "Bookings" section on menu
3) Try perform a booking now
4) Define a total to 1, 2 per item and 3 per itemtype with 2 days for
   example for booking period.
5) After 1 an error message showing related to total allowed
6) Set total to 5 and retry on item bookable sets to "Yes"
7) Normally it's works but do it again (to have 3 bookings and an error
   message appears)
8) Do the same thing on item bookable sets to "Follow item type", the
   maximum will be 3 now before see the error.
** Note that if bookings total and booking per item/itemtype is set it
will be the minimum number.
9)You can also try to test with a longer period than bookings period
  length rule, an another error will be displayed.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 6 Thibaud Guillot 2024-03-20 14:42:28 UTC
Hi !

It's a first proposal, I haven't yet written unit tests because (unless I'm mistaken) they're currently KO (Booking.t) 

If you'd like to give me some feedback on what's been done, I'd be happy to hear from you.
Comment 7 Thibaud Guillot 2024-03-20 14:44:13 UTC
Perhaps we could change the patch title to broaden the scope of the rules if you agree
Comment 8 Martin Renvoize 2024-03-20 14:46:49 UTC
Nice one.. I'd just started to look again at this and you posted a patch.. I'll take a look now :)
Comment 9 Thibaud Guillot 2024-04-10 14:08:41 UTC
Created attachment 164627 [details] [review]
Bug 36271 : Add circulation rules for booking

Welcome to 3 new circulation rules :)

- Bookings allowed (total) / bookings_allowed_total
- Bookings per item (total) / bookings_per_item
- Bookings period length (day) / bookings_period_length

Test plan:
1) Apply this patch
2) Set values for these new rules
3) Define a total to 2, 1 per item with 2 days for
   example for booking period.
4) Book an item and retry on the same item, normally an error message is
   showing in the pop up.
6) Book another item, and again another one, on the third an error
   message is showing (always in pop up)
7) You can also try to test with a longer period than bookings period
  length rule, an another error will be displayed.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 10 Laurence Rault 2024-04-11 12:41:09 UTC
Test plan passed OK if one rule is defined for one itemtype

But with several rules : 
All / Mixed materials : 3 bookings, 2 per item, 5 days
All / All : 1 booking, 1 per item, 2 days

If I have on my account :  
3 bookings for mixed materials on two items / two records
I cannot place a booking on an item of Reference type in the same record as above, or in another record : because of maximum booking rule =1
As I have no booked itemtype other than Mixed materials, I should not be blocked for another itemtype.
If I set All / All : 4 bookings, I can add the booking on itemtype Reference

unless I misunderstand, All is not a general quota, but used if no precise rule is defined

=> the counting of booked items should be done per itemtype.
Comment 11 Thibaud Guillot 2024-04-18 10:07:55 UTC
Created attachment 165061 [details] [review]
Bug 36271 : Add circulation rules for booking

Welcome to 3 new circulation rules :)

- Bookings allowed (total) / bookings_allowed_total
- Bookings per item (total) / bookings_per_item
- Bookings period length (day) / bookings_period_length

Test plan:
1) Apply this patch
2) Set values for these new rules
3) Define a total to 2, 1 per item with 2 days for
   example for booking period.
4) Book an item and retry on the same item, normally an error message is
   showing in the pop up.
6) Book another item, and again another one, on the third an error
   message is showing (always in pop up)
7) You can also try to test with a longer period than bookings period
  length rule, an another error will be displayed.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 12 Thibaud Guillot 2024-04-18 10:11:44 UTC
Created attachment 165063 [details] [review]
Bug 36271 : Add circulation rules for booking

Welcome to 3 new circulation rules :)

- Bookings allowed (total) / bookings_allowed_total
- Bookings per item (total) / bookings_per_item
- Bookings period length (day) / bookings_period_length

Test plan:
1) Apply this patch
2) Set values for these new rules
3) Define a total to 2, 1 per item with 2 days for
   example for booking period.
4) Book an item and retry on the same item, normally an error message is
   showing in the pop up.
6) Book another item, and again another one, on the third an error
   message is showing (always in pop up)
7) You can also try to test with a longer period than bookings period
  length rule, an another error will be displayed.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 13 Fridolin Somers 2024-04-25 07:48:54 UTC
> Bug 36271 : xxx
Please fix commit message, no space before semicolon.
Comment 14 Thibaud Guillot 2024-05-16 09:10:58 UTC
Created attachment 166809 [details] [review]
Bug 36271: Add circulation rules for booking

Welcome to 3 new circulation rules :)

- Bookings allowed (total) / bookings_allowed_total
- Bookings per item (total) / bookings_per_item
- Bookings period length (day) / bookings_period_length

Test plan:
1) Apply this patch
2) Set values for these new rules
3) Define a total to 2, 1 per item with 2 days for
   example for booking period.
4) Book an item and retry on the same item, normally an error message is
   showing in the pop up.
6) Book another item, and again another one, on the third an error
   message is showing (always in pop up)
7) You can also try to test with a longer period than bookings period
  length rule, an another error will be displayed.

Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS)
Comment 15 David Nind 2024-05-16 20:10:30 UTC
Dependent bug 35906 no longer applies 8-(..