[Bug 42608] New: Hold-shelf expiration over-extends when ExcludeHolidaysFromMaxPickUpDelay is on and useDaysMode=Dayweek
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Bug ID: 42608 Summary: Hold-shelf expiration over-extends when ExcludeHolidaysFromMaxPickUpDelay is on and useDaysMode=Dayweek Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com When ExcludeHolidaysFromMaxPickUpDelay is on and useDaysMode is set to Dayweek, the pickup expiration date set for a hold by Koha::Hold::set_waiting is inflated by a whole week whenever the pickup window crosses a single (one-off) closure on a normally-open weekday -- for example a Bank Holiday Monday. Steps to reproduce ================== System preferences: ReservesMaxPickUpDelay = 6 ExcludeHolidaysFromMaxPickUpDelay = ON ExpireReservesOnHolidays = ON useDaysMode = Dayweek Branch calendar: - Sundays marked as a weekly closed day. - A single (one-off) holiday on a Monday two weeks out -- for example 2026-05-04 (the early May Bank Holiday). Trap a hold to waiting at that branch on Tuesday 2026-04-28. Actual result ============= expirationdate is set to 2026-05-12 -- a 14-day pickup window. Expected result =============== expirationdate should be 2026-05-06 -- a 6-open-day pickup window. The intent of ExcludeHolidaysFromMaxPickUpDelay is to give the patron the full ReservesMaxPickUpDelay count of *open* days to come and collect the item, by extending the window by one calendar day for every closed day that falls inside it. useDaysMode (Dayweek / Datedue / Calendar / Days) is a loan-due-date concept -- the patron picks no weekday for a hold pickup; staff trap the item, and the only thing that should influence the expiration date is how many open days the patron should get to collect. Wider example table =================== Same configuration as above. Sunday 3 May and Monday 4 May 2026 are closed at the branch. Comparison of current (buggy) vs expected expiration dates for each weekday a hold could be trapped that week: Hold trapped on Current (buggy) Expected ----------------- ----------------- ----------------- Mon 27 Apr 2026 Mon 11 May 2026 Tue 5 May 2026 Tue 28 Apr 2026 Tue 12 May 2026 Wed 6 May 2026 Wed 29 Apr 2026 Wed 13 May 2026 Thu 7 May 2026 Thu 30 Apr 2026 Thu 14 May 2026 Fri 8 May 2026 Fri 1 May 2026 Fri 15 May 2026 Sat 9 May 2026 In each case the expected expiration is `waitingdate + ReservesMaxPickUpDelay` open days, with Sun 3 May and Mon 4 May skipped (not counted toward the 6-day window). The current buggy expirations are all 14 calendar days from the trap date -- a whole extra week. Reproduction snippet ==================== use Koha::Calendar; use Koha::DateUtils qw(dt_from_string); # Test branch: Sundays weekly closed, single holiday 2026-05-04. my $cal = Koha::Calendar->new( branchcode => 'BR', days_mode => 'Dayweek', ); # Bug: prints 2026-05-12 (14 days). # Expected: 2026-05-06 (6 open days). say $cal->days_forward( dt_from_string('2026-04-28'), 6 )->ymd; Notes ===== Loan due dates calculated by Koha::Calendar::addDays under useDaysMode=Dayweek behave correctly -- the bug only surfaces on hold pickup expiration dates. A fix should not regress the loan due-date behaviour, which is the original purpose of the Dayweek mode (keeping a Friday-due loan due on a Friday over closures). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jackie.usher@westsussex.gov | |.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |OpenFifth list of Sponsors| |<https://openfifth.co.uk/> Patch complexity|--- |Small patch Sponsorship status|--- |Sponsored Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199157&action=edit Bug 42608: Stop hold pickup expiration from week-jumping over closures Koha::Hold::set_waiting calls Koha::Calendar::days_forward to derive the holds-shelf expiration date when ExcludeHolidaysFromMaxPickUpDelay is on. days_forward iterated next_open_days($dt, 1) ReservesMaxPickUpDelay times, and next_open_days consults get_push_amt for every closed day it encounters. Under useDaysMode=Dayweek, get_push_amt returns 7 for any closure that is not a permanently closed weekday -- so an intermediate Bank Holiday Monday inside the pickup window pushed the cursor a whole week forward instead of one day, inflating the expiration by seven days. Dayweek is a loan-due-date concept: it exists to keep a Friday-due loan due on a Friday across closures. A patron does not pick a weekday for a hold-shelf collection; staff set the expiration when they trap the item, and the only thing that should drive it is how many open days the patron should have to come and collect. days_forward is now decoupled from useDaysMode and always counts forward one open day at a time, ignoring closures without consuming them from the count. next_open_days, prev_open_days, get_push_amt and addDays are unchanged -- loan due-date math keeps the existing Dayweek behaviour. Test plan: 1. Inside KTD, run the updated unit tests: prove t/db_dependent/Calendar.t prove t/db_dependent/Hold.t Confirm both pass, including the new Bug 42608 sub-tests. 2. Manual reproduction: - Set sysprefs: useDaysMode=Dayweek, ReservesMaxPickUpDelay=6, ExcludeHolidaysFromMaxPickUpDelay=ON, ExpireReservesOnHolidays=ON. - On a branch, mark Sundays as a weekly closure and add a single holiday on Monday 2026-05-04. - Trap five holds to waiting at that branch with waitingdates 2026-04-27 through 2026-05-01. - Without the patch: expirationdates are 2026-05-11, 2026-05-12, 2026-05-13, 2026-05-14, 2026-05-15 (all +14 calendar days). - With the patch: expirationdates are 2026-05-05, 2026-05-06, 2026-05-07, 2026-05-08, 2026-05-09 (giving the patron 6 open days in every case, skipping Sun 3 May and Mon 4 May). 3. Verify the existing Calendar mode behaviour is unchanged: - Switch useDaysMode to Calendar and repeat step 2 -- the same expected expirationdates should apply. 4. Verify loan due-date behaviour is unchanged: - With useDaysMode=Dayweek, issue an item with a loan period that places its due date on Mon 2026-05-04. Confirm the due date rolls forward by a whole week (the original Dayweek intent), proving this patch did not regress addDays/addDuration. Sponsored-by: OpenFifth <https://openfifth.co.uk/> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |andrew@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199157|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42608 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 205589 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205589&action=edit Bug 42608: Stop hold pickup expiration from week-jumping over closures Koha::Hold::set_waiting calls Koha::Calendar::days_forward to derive the holds-shelf expiration date when ExcludeHolidaysFromMaxPickUpDelay is on. days_forward iterated next_open_days($dt, 1) ReservesMaxPickUpDelay times, and next_open_days consults get_push_amt for every closed day it encounters. Under useDaysMode=Dayweek, get_push_amt returns 7 for any closure that is not a permanently closed weekday -- so an intermediate Bank Holiday Monday inside the pickup window pushed the cursor a whole week forward instead of one day, inflating the expiration by seven days. Dayweek is a loan-due-date concept: it exists to keep a Friday-due loan due on a Friday across closures. A patron does not pick a weekday for a hold-shelf collection; staff set the expiration when they trap the item, and the only thing that should drive it is how many open days the patron should have to come and collect. days_forward is now decoupled from useDaysMode and always counts forward one open day at a time, ignoring closures without consuming them from the count. next_open_days, prev_open_days, get_push_amt and addDays are unchanged -- loan due-date math keeps the existing Dayweek behaviour. Test plan: 1. Inside KTD, run the updated unit tests: prove t/db_dependent/Calendar.t prove t/db_dependent/Hold.t Confirm both pass, including the new Bug 42608 sub-tests. 2. Manual reproduction: - Set sysprefs: useDaysMode=Dayweek, ReservesMaxPickUpDelay=6, ExcludeHolidaysFromMaxPickUpDelay=ON, ExpireReservesOnHolidays=ON. - On a branch, mark Sundays as a weekly closure and add a single holiday on Monday 2026-05-04. - Trap five holds to waiting at that branch with waitingdates 2026-04-27 through 2026-05-01. - Without the patch: expirationdates are 2026-05-11, 2026-05-12, 2026-05-13, 2026-05-14, 2026-05-15 (all +14 calendar days). - With the patch: expirationdates are 2026-05-05, 2026-05-06, 2026-05-07, 2026-05-08, 2026-05-09 (giving the patron 6 open days in every case, skipping Sun 3 May and Mon 4 May). 3. Verify the existing Calendar mode behaviour is unchanged: - Switch useDaysMode to Calendar and repeat step 2 -- the same expected expirationdates should apply. 4. Verify loan due-date behaviour is unchanged: - With useDaysMode=Dayweek, issue an item with a loan period that places its due date on Mon 2026-05-04. Confirm the due date rolls forward by a whole week (the original Dayweek intent), proving this patch did not regress addDays/addDuration. Sponsored-by: OpenFifth <https://openfifth.co.uk/> Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk> -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org