Description
Pedro Amorim
2025-08-13 11:25:45 UTC
Created attachment 185358 [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. 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) Created attachment 185359 [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 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 Adding Martin as QA here for when he's available. 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> How hard would it be to add a small test here? (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. 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> (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.. Created attachment 186159 [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. 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. Nice work everyone! Pushed to main for 25.11 Nice work everyone! Pushed to 25.05.x Pushed to 24.11.x for 24.11.10 |