Bug 40644 - Bookings biblio checks erroneously if multiple check-outs and bookings exist
Summary: Bookings biblio checks erroneously if multiple check-outs and bookings exist
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Pedro Amorim
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 29002
Blocks:
  Show dependency treegraph
 
Reported: 2025-08-13 11:25 UTC by Pedro Amorim
Modified: 2025-10-16 05:41 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.04,24.11.10
Circulation function: Bookings


Attachments
Bug 40644: Only count checkouts that have not yet been counted as bookings (1.68 KB, patch)
2025-08-13 11:32 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40644: Only count checkouts that have not yet been counted as bookings (1.85 KB, patch)
2025-08-13 11:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40644: Only count checkouts that have not yet been counted as bookings (1.85 KB, patch)
2025-08-13 11:43 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40644: Only count checkouts that have not yet been counted as bookings (1.92 KB, patch)
2025-08-14 18:09 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 40644: Only count checkouts that have not yet been counted as bookings (2.01 KB, patch)
2025-09-04 09:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 40644: Add tests (3.68 KB, patch)
2025-09-04 14:26 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 40644: Add tests (3.51 KB, patch)
2025-09-04 14:29 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2025-08-13 11:25:45 UTC

    
Comment 1 Pedro Amorim 2025-08-13 11:32:21 UTC Comment hidden (obsolete)
Comment 2 Pedro Amorim 2025-08-13 11:43:00 UTC Comment hidden (obsolete)
Comment 3 Pedro Amorim 2025-08-13 11:43:50 UTC
Created attachment 185360 [details] [review]
Bug 40644: Only count checkouts that have not yet been counted as bookings

Test plan:
1) Have a record with 8 items.
2) Place a booking on 6 of the items (6 bookings total) for a full month
   (e.g. August)
3) Check-out all 6 of those bookings
4) Attempt to place a new booking for one of the 2 free items for end of month, e.g. (Aug 26 -> Aug 28).
5) It will fail with error "Booking conflicts". This is because the code
   is adding 6 checkouts with 6 bookings = 12 which is higher than 8
bookable items.
6) With this patch, check-outs that refer to an existing booking are
   excluded, so the check becomes if 8 - 6 > 0 (which passes the check) rather than 8 - 12 > 0 (which fails the check)

Run existing tests:
prove t/db_dependent/Koha/Item.t
prove t/db_dependent/Koha/Biblio.t
prove t/db_dependent/Koha/Booking.t
Comment 4 Pedro Amorim 2025-08-13 11:44:31 UTC
Adding Martin as QA here for when he's available.
Comment 5 Andrew Fuerste-Henry 2025-08-14 18:09:32 UTC
Created attachment 185417 [details] [review]
Bug 40644: Only count checkouts that have not yet been counted as bookings

Test plan:
1) Have a record with 8 items.
2) Place a booking on 6 of the items (6 bookings total) for a full month
   (e.g. August)
3) Check-out all 6 of those bookings
4) Attempt to place a new booking for one of the 2 free items for end of month, e.g. (Aug 26 -> Aug 28).
5) It will fail with error "Booking conflicts". This is because the code
   is adding 6 checkouts with 6 bookings = 12 which is higher than 8
bookable items.
6) With this patch, check-outs that refer to an existing booking are
   excluded, so the check becomes if 8 - 6 > 0 (which passes the check) rather than 8 - 12 > 0 (which fails the check)

Run existing tests:
prove t/db_dependent/Koha/Item.t
prove t/db_dependent/Koha/Biblio.t
prove t/db_dependent/Koha/Booking.t

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Comment 6 Marcel de Rooy 2025-08-29 09:29:23 UTC
How hard would it be to add a small test here?
Comment 7 Pedro Amorim 2025-09-01 09:13:06 UTC
(In reply to Marcel de Rooy from comment #6)
> How hard would it be to add a small test here?

I believe it shouldn't be too hard but unfortunately I had to jump on to other things as soon as I found the fix here. I'm happy to note this down to write some tests whenever I'm able to.
Comment 8 Marcel de Rooy 2025-09-04 09:18:58 UTC
Created attachment 186129 [details] [review]
Bug 40644: Only count checkouts that have not yet been counted as bookings

Test plan:
1) Have a record with 8 items.
2) Place a booking on 6 of the items (6 bookings total) for a full month
   (e.g. August)
3) Check-out all 6 of those bookings
4) Attempt to place a new booking for one of the 2 free items for end of month, e.g. (Aug 26 -> Aug 28).
5) It will fail with error "Booking conflicts". This is because the code
   is adding 6 checkouts with 6 bookings = 12 which is higher than 8
bookable items.
6) With this patch, check-outs that refer to an existing booking are
   excluded, so the check becomes if 8 - 6 > 0 (which passes the check) rather than 8 - 12 > 0 (which fails the check)

Run existing tests:
prove t/db_dependent/Koha/Item.t
prove t/db_dependent/Koha/Biblio.t
prove t/db_dependent/Koha/Booking.t

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2025-09-04 09:19:38 UTC
(In reply to Pedro Amorim from comment #7)
> I believe it shouldn't be too hard but unfortunately I had to jump on to
> other things as soon as I found the fix here. I'm happy to note this down to
> write some tests whenever I'm able to.

Wont insist for this one line. Passing as an exception to the rule..
Comment 10 Pedro Amorim 2025-09-04 14:26:49 UTC Comment hidden (obsolete)
Comment 11 Pedro Amorim 2025-09-04 14:29:05 UTC
Created attachment 186161 [details] [review]
Bug 40644: Add tests

If you apply this patch before the other one:

prove t/db_dependent/Koha/Biblio.t

Will fail. Apply both patches. The above now passes.

This highlighted something. Following the test plan I can't reproduce
this anymore. In order to reproduce, the check-outs need to happen
before the bookings, otherwise when the check-outs happen on an existing
booking it'll 'complete' or 'cancel' the booking thus not causing the
error.
Comment 12 Lucas Gass (lukeg) 2025-09-05 14:06:54 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 13 Paul Derscheid 2025-09-20 08:59:10 UTC
Nice work everyone!

Pushed to 25.05.x
Comment 14 Fridolin Somers 2025-10-16 05:38:57 UTC
Pushed to 24.11.x for 24.11.10