[Bug 40435] New: CanBookBeRenewed should respect future holds or even more
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Bug ID: 40435 Summary: CanBookBeRenewed should respect future holds or even more Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com This is a follow-up of bug 37651. -- 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=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | Status|NEW |ASSIGNED -- 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=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CanBookBeRenewed should |CanBookBeRenewed should |respect future holds or |respect future holds |even more | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |37651 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37651 [Bug 37651] biblio->current_holds and item->current_holds do not respect ConfirmFutureHolds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CanBookBeRenewed should |Allow CanBookBeRenewed to |respect future holds |consider future holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=40435 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 190449 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190449&action=edit Bug 40435: Database revision Test plan: [1] Run install or upgrade (updatedatabase.pl). Verify that pref was added to database. * perl -MC4::Context -e'print C4::Context->preference('FutureHoldsBlockRenewals'),"\n"' * You should see a zero printed. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 190450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190450&action=edit Bug 40435: New preference FutureHoldsBlockRenewals Test plan: Change this pref value under Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 190451 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190451&action=edit Bug 40435: Add to HEA Grouping all future hold prefs here too. Test plan: prove t/db_dependent/UsageStats.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 190452 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190452&action=edit Bug 40435: Module changes CanBookBeRenewed will by default pass the skip_future_holds flag to item->current_holds so that renewals are not blocked by future holds. The new pref FutureHoldsBlockRenewals allows you to change that historical behavior. Test plan: [1] Set pref ConfirmFutureHolds=3, FutureHoldsBlockRenewals=Allow, OPACAllowHoldDateInFuture=Allow [2] Checkout some item to patron A. [3] Place hold on same item for patron B, reserve date tomorrow. [4] Try to renew item with patron A on account page. Should be marked as Not renewable. [5] Set pref FutureHoldsBlockRenewals=Dont [6] Repeat step 4. Renew should be allowed. [7] prove t/db_dependent/Holds.t t/db_dependent/Reserves.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 190453 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190453&action=edit Bug 40435: Add tests Test plan: Run t/db_dependent/Circulation.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- At this point, I have chosen to set the period for blocking renewals equal to the period of ConfirmFutureHolds (given that you enable the new pref). This seems to make the most sense now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> 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=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190449|0 |1 is obsolete| | --- Comment #7 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190520 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190520&action=edit Bug 40435: Database revision Test plan: [1] Run install or upgrade (updatedatabase.pl). Verify that pref was added to database. * perl -MC4::Context -e'print C4::Context->preference('FutureHoldsBlockRenewals'),"\n"' * You should see a zero printed. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190450|0 |1 is obsolete| | --- Comment #8 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190521 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190521&action=edit Bug 40435: New preference FutureHoldsBlockRenewals Test plan: Change this pref value under Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190451|0 |1 is obsolete| | --- Comment #9 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190522 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190522&action=edit Bug 40435: Add to HEA Grouping all future hold prefs here too. Test plan: prove t/db_dependent/UsageStats.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190452|0 |1 is obsolete| | --- Comment #10 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190523 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190523&action=edit Bug 40435: Module changes CanBookBeRenewed will by default pass the skip_future_holds flag to item->current_holds so that renewals are not blocked by future holds. The new pref FutureHoldsBlockRenewals allows you to change that historical behavior. Test plan: [1] Set pref ConfirmFutureHolds=3, FutureHoldsBlockRenewals=Allow, OPACAllowHoldDateInFuture=Allow [2] Checkout some item to patron A. [3] Place hold on same item for patron B, reserve date tomorrow. [4] Try to renew item with patron A on account page. Should be marked as Not renewable. [5] Set pref FutureHoldsBlockRenewals=Dont [6] Repeat step 4. Renew should be allowed. [7] prove t/db_dependent/Holds.t t/db_dependent/Reserves.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190453|0 |1 is obsolete| | --- Comment #11 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190524 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190524&action=edit Bug 40435: Add tests Test plan: Run t/db_dependent/Circulation.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens (kidclamp) from comment #11)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Great! Thx -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190520|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=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190521|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=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190522|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=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190523|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=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190524|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=40435 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190671 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190671&action=edit Bug 40435: Database revision Test plan: [1] Run install or upgrade (updatedatabase.pl). Verify that pref was added to database. * perl -MC4::Context -e'print C4::Context->preference('FutureHoldsBlockRenewals'),"\n"' * You should see a zero printed. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190672 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190672&action=edit Bug 40435: New preference FutureHoldsBlockRenewals Test plan: Change this pref value under Administration. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190673 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190673&action=edit Bug 40435: Add to HEA Grouping all future hold prefs here too. Test plan: prove t/db_dependent/UsageStats.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #16 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190674&action=edit Bug 40435: Module changes CanBookBeRenewed will by default pass the skip_future_holds flag to item->current_holds so that renewals are not blocked by future holds. The new pref FutureHoldsBlockRenewals allows you to change that historical behavior. Test plan: [1] Set pref ConfirmFutureHolds=3, FutureHoldsBlockRenewals=Allow, OPACAllowHoldDateInFuture=Allow [2] Checkout some item to patron A. [3] Place hold on same item for patron B, reserve date tomorrow. [4] Try to renew item with patron A on account page. Should be marked as Not renewable. [5] Set pref FutureHoldsBlockRenewals=Dont [6] Repeat step 4. Renew should be allowed. [7] prove t/db_dependent/Holds.t t/db_dependent/Reserves.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190675&action=edit Bug 40435: Add tests Test plan: Run t/db_dependent/Circulation.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #18 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190676&action=edit Bug 40435: (follow-up) Fix DateTime mutation in test The test had a bug where $future_date was being mutated by the add() method on line 1975, causing line 1982 to use 4 days instead of 2 days. This meant the test wasn't properly verifying that FutureHoldsBlockRenewals=0 prevents blocking for holds within the ConfirmFutureHolds window. Test plan: 1. Run: prove t/db_dependent/Circulation.t :: "CanBookBeRenewed | future holds" 2. Verify all tests pass 3. Confirm the test now properly validates the intended behavior Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 --- Comment #19 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190677&action=edit Bug 40435: (follow-up) Change preference type to YesNo Change FutureHoldsBlockRenewals from type 'Integer' to 'YesNo' for consistency with other boolean preferences in Koha (e.g., AllowHoldDateInFuture, OPACAllowHoldDateInFuture). While the preference functionally works as Integer due to the .pref file defining the choices, YesNo is the established convention for binary system preferences. Test plan: 1. Apply this patch 2. Run the database update 3. Verify the preference appears correctly in system preferences 4. Verify the preference still functions properly with tests: prove t/db_dependent/Circulation.t :: "CanBookBeRenewed | future holds" Signed-off-by: Martin Renvoize <martin.renvoize@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=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #20 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Sponsored Marcel? Even if just by your own organisation? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize (ashimema) from comment #20)
Sponsored Marcel? Even if just by your own organisation?
Thx for QAing. No need for a sponsor line here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 --- Comment #22 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | Text to go in the| |Historically, renewals are release notes| |not blocked for future | |holds (which are only | |possible when enabled). | |This report adds a new | |preference | |FutureHoldsBlockRenewals to | |override that behavior. If | |you enable it, renewals are | |blocked by future holds | |when they are not further | |away than the threshhold | |used for Holds to Pull in | |preference | |ConfirmFutureHolds. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40435 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #23 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- This bug won't be backported to the 25.11.x branch as it has been deemed an enhancement or new feature. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org