[Bug 36271] New: Bookings should respect circulation rules for max loan periods
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Bug ID: 36271 Summary: Bookings should respect circulation rules for max loan periods Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com 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. -- 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=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |35248 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35248 [Bug 35248] Bookings needs unit tests -- 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=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de, | |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=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |thibaud.guillot@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #1 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #2 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- 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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Kristi <kkrueger@cuyahogalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kkrueger@cuyahogalibrary.or | |g --- Comment #3 from Kristi <kkrueger@cuyahogalibrary.org> --- 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). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Bug 36271 depends on bug 35248, which changed state. Bug 35248 Summary: Bookings needs unit tests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35248 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Depends on|35248 |35906 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35248 [Bug 35248] Bookings needs unit tests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35906 [Bug 35906] Add bookable option on itemtypes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #5 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 163546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163546&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #6 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #7 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Perhaps we could change the patch title to broaden the scope of the rules if you agree -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice one.. I'd just started to look again at this and you posted a patch.. I'll take a look now :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163546|0 |1 is obsolete| | --- Comment #9 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 164627 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164627&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Laurence Rault <laurence.rault@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |laurence.rault@biblibre.com Status|Needs Signoff |Failed QA --- Comment #10 from Laurence Rault <laurence.rault@biblibre.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164627|0 |1 is obsolete| | --- Comment #11 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 165061 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165061&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165061|0 |1 is obsolete| | --- Comment #12 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 165063 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165063&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> ---
Bug 36271 : xxx Please fix commit message, no space before semicolon.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36789 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36789 [Bug 36789] Transform a booking into checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165063|0 |1 is obsolete| | --- Comment #14 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166809 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166809&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #15 from David Nind <david@davidnind.com> --- Dependent bug 35906 no longer applies 8-(.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #16 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166907&action=edit Bug 36271: (follow-up) Add switch for join table according to item-level_itypes syspref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #17 from David Nind <david@davidnind.com> --- The patches no longer apply 8-(.. git bz apply 36271 Bug 36271 Depends on bug 35906 (Signed Off) Follow? [(y)es, (n)o] y Bug 35906 - Add bookable option on itemtypes 166866 - Bug 35906: Add bookable option on itemtype 166867 - Bug 35906: Add unit test 166868 - Bug 35906: Remove preference and add override handling 166869 - Bug 35906: Schema dump 166870 - Bug 35906: (QA follow-up) Fix item-type schema 166871 - Bug 35906: (follow-up) rebased it Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35906: Add bookable option on itemtype Applying: Bug 35906: Add unit test Applying: Bug 35906: Remove preference and add override handling Applying: Bug 35906: Schema dump Applying: Bug 35906: (QA follow-up) Fix item-type schema Applying: Bug 35906: (follow-up) rebased it Bug 36271 - Bookings should respect circulation rules for max loan periods 166809 - Bug 36271: Add circulation rules for booking 166907 - Bug 36271: (follow-up) Add switch for join table according to item-level_itypes syspref Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36271: Add circulation rules for booking Using index info to reconstruct a base tree... M Koha/Booking.pm M Koha/REST/V1/Bookings.pm M admin/smart-rules.pl M koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt A koha-tmpl/intranet-tmpl/prog/js/place_booking_modal.js Falling back to patching base and 3-way merge... CONFLICT (modify/delete): koha-tmpl/intranet-tmpl/prog/js/place_booking_modal.js deleted in HEAD and modified in Bug 36271: Add circulation rules for booking. Version Bug 36271: Add circulation rules for booking of koha-tmpl/intranet-tmpl/prog/js/place_booking_modal.js left in tree. Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Auto-merging admin/smart-rules.pl Auto-merging Koha/REST/V1/Bookings.pm Auto-merging Koha/Booking.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 36271: Add circulation rules for booking -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166809|0 |1 is obsolete| | Attachment #166907|0 |1 is obsolete| | --- Comment #18 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166930&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #19 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166931&action=edit Bug 36271: (follow-up) Add switch for join table according to item-level_itypes syspref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166930|0 |1 is obsolete| | Attachment #166931|0 |1 is obsolete| | --- Comment #20 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166932&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #21 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166933&action=edit Bug 36271: (follow-up) Add switch for join table according to item-level_itypes syspref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #22 from David Nind <david@davidnind.com> --- The patches now apply, however there is an error when restarting everything - see [1]. I think I managed tow ork around this, but what I did may be incorrect. I tested using koha-testing-docker (KTD). I have changed the status to Failed QA: 1. Have set the rules as per the test plan. 2. I can make one booking, then when I go to make another for the same patron, I get a failure error (as expected, as Bookings per item (total) = 1). 3. I changed the rules so that they are 2, 2, and 2 - with the expectation that the same patron could make 2 bookings for the item, at different times. I get "Failure" in a red box at the top of the dialogue box if I try to make two bookings. 4. I have not changed any other settings, or made any changes to the item type: . Under the items section for the record where you change it to bookable it says "Item type bookable: No" . I don't know whether that causes an issue or not . If you go to change the bookable setting, then if the item type says it isn't bookable, then you shouldn't be able to override and make an individual item bookable(?) Not sure if this is the way it should work, or I have misunderstood. . Probably not really understanding what the interactions should be here: . Set as bookable at the item type level, override at the individual record/item level? . Set as bookable at the item type level only? Then having bookable showing at the individual record/item level doesn't make sense to me. . Maybe some more explanation is required in the commit message and test plan of the intent may help . (I haven't read through the bug, and related bugs, history) 5. I'm not a fan of generic "Failure" messages. If there are a set of rules (as in this case), then I think any error message should indicate the reason for the failure so you can correct things. In this case, I don't know what the reason for the failure is. [1] Error when restarting everything ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ After the patch is applied, I get this error message when restarting everything (apply patch, updatedatabase, flush_memcached, restart_all): restart_all Restarting Apache httpd web server: apache2. Restarting Koha ILS: koha-commonStopping Plack daemon for kohadev:. Stopping Z39.50/SRU daemon for kohadev:. Stopping Koha worker daemon for kohadev (default):. Stopping Koha worker daemon for kohadev (long_tasks):. Stopping Koha indexing daemon for kohadev:. Successfully stopped all services for instance koha-common:. Starting Plack daemon for kohadev:. Starting Z39.50/SRU daemon for kohadev:syntax error at /kohadevbox/koha/Koha/Booking.pm line 118, near "if if" Compilation failed in require at /kohadevbox/koha/Koha/Bookings.pm line 23. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Bookings.pm line 23. Compilation failed in require at /kohadevbox/koha/Koha/Biblio.pm line 46. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Biblio.pm line 46. Compilation failed in require at /kohadevbox/koha/Koha/Biblios.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Biblios.pm line 25. Compilation failed in require at /kohadevbox/koha/Koha/Libraries.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Libraries.pm line 25. Compilation failed in require at /kohadevbox/koha/Koha/Object/Limit/Library.pm line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Object/Limit/Library.pm line 22. Compilation failed in require at /kohadevbox/koha/Koha/AuthorisedValue.pm line 26. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/AuthorisedValue.pm line 26. Compilation failed in require at /kohadevbox/koha/Koha/AuthorisedValues.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/AuthorisedValues.pm line 25. Compilation failed in require at /kohadevbox/koha/C4/Koha.pm line 27. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Koha.pm line 27. Compilation failed in require at /kohadevbox/koha/C4/Biblio.pm line 88. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Biblio.pm line 88. Compilation failed in require at /kohadevbox/koha/Koha/Z3950Responder.pm line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Z3950Responder.pm line 22. Compilation failed in require at /kohadevbox/koha/misc/z3950_responder.pl line 27. BEGIN failed--compilation aborted at /kohadevbox/koha/misc/z3950_responder.pl line 27. failed! Starting Koha worker daemon for kohadev (default):. Starting Koha worker daemon for kohadev (long_tasks):. Starting Koha indexing daemon for kohadev:. At line 118 of Koha/Booking.pm there is an "if if" - if I remove one of the ifs, then everything restarts properly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166932|0 |1 is obsolete| | Attachment #166933|0 |1 is obsolete| | --- Comment #23 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166952 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166952&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #24 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to David Nind from comment #22)
The patches now apply, however there is an error when restarting everything - see [1]. I think I managed tow ork around this, but what I did may be incorrect.
I tested using koha-testing-docker (KTD).
I have changed the status to Failed QA:
1. Have set the rules as per the test plan.
2. I can make one booking, then when I go to make another for the same patron, I get a failure error (as expected, as Bookings per item (total) = 1).
3. I changed the rules so that they are 2, 2, and 2 - with the expectation that the same patron could make 2 bookings for the item, at different times. I get "Failure" in a red box at the top of the dialogue box if I try to make two bookings.
4. I have not changed any other settings, or made any changes to the item type: . Under the items section for the record where you change it to bookable it says "Item type bookable: No" . I don't know whether that causes an issue or not . If you go to change the bookable setting, then if the item type says it isn't bookable, then you shouldn't be able to override and make an individual item bookable(?) Not sure if this is the way it should work, or I have misunderstood. . Probably not really understanding what the interactions should be here: . Set as bookable at the item type level, override at the individual record/item level? . Set as bookable at the item type level only? Then having bookable showing at the individual record/item level doesn't make sense to me. . Maybe some more explanation is required in the commit message and test plan of the intent may help . (I haven't read through the bug, and related bugs, history)
5. I'm not a fan of generic "Failure" messages. If there are a set of rules (as in this case), then I think any error message should indicate the reason for the failure so you can correct things. In this case, I don't know what the reason for the failure is.
[1] Error when restarting everything ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
After the patch is applied, I get this error message when restarting everything (apply patch, updatedatabase, flush_memcached, restart_all):
restart_all Restarting Apache httpd web server: apache2. Restarting Koha ILS: koha-commonStopping Plack daemon for kohadev:. Stopping Z39.50/SRU daemon for kohadev:. Stopping Koha worker daemon for kohadev (default):. Stopping Koha worker daemon for kohadev (long_tasks):. Stopping Koha indexing daemon for kohadev:. Successfully stopped all services for instance koha-common:. Starting Plack daemon for kohadev:. Starting Z39.50/SRU daemon for kohadev:syntax error at /kohadevbox/koha/Koha/Booking.pm line 118, near "if if" Compilation failed in require at /kohadevbox/koha/Koha/Bookings.pm line 23. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Bookings.pm line 23. Compilation failed in require at /kohadevbox/koha/Koha/Biblio.pm line 46. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Biblio.pm line 46. Compilation failed in require at /kohadevbox/koha/Koha/Biblios.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Biblios.pm line 25. Compilation failed in require at /kohadevbox/koha/Koha/Libraries.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Libraries.pm line 25. Compilation failed in require at /kohadevbox/koha/Koha/Object/Limit/Library.pm line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Object/Limit/Library.pm line 22. Compilation failed in require at /kohadevbox/koha/Koha/AuthorisedValue.pm line 26. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/AuthorisedValue.pm line 26. Compilation failed in require at /kohadevbox/koha/Koha/AuthorisedValues.pm line 25. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/AuthorisedValues.pm line 25. Compilation failed in require at /kohadevbox/koha/C4/Koha.pm line 27. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Koha.pm line 27. Compilation failed in require at /kohadevbox/koha/C4/Biblio.pm line 88. BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Biblio.pm line 88. Compilation failed in require at /kohadevbox/koha/Koha/Z3950Responder.pm line 22. BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/Z3950Responder.pm line 22. Compilation failed in require at /kohadevbox/koha/misc/z3950_responder.pl line 27. BEGIN failed--compilation aborted at /kohadevbox/koha/misc/z3950_responder.pl line 27. failed! Starting Koha worker daemon for kohadev (default):. Starting Koha worker daemon for kohadev (long_tasks):. Starting Koha indexing daemon for kohadev:.
At line 118 of Koha/Booking.pm there is an "if if" - if I remove one of the ifs, then everything restarts properly.
Hello David, Thanks for your feedback. I remove the typo issue ("if if") and rebase my commit into one unique commit by the way. For the failure message its the original behavior when errors comes but not necessary from circulation rules. I'll look into the details of why it's happening -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166952|0 |1 is obsolete| | --- Comment #25 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166957&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #26 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- I miss some js code on my last rebase (due to place_booking.js modal renaming). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166957|0 |1 is obsolete| | --- Comment #27 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 166962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=166962&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|36789 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36789 [Bug 36789] Transform a booking into checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=37354 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Bookings should respect |Bookings should have |circulation rules for max |circulation rules |loan periods | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've renamed this bug to highlight the change in scope. I'm really interested in the direction here and I think we should continue to refine the rule options in this bug. However, I'm also keen to submit my original proposal to respect existing rules for loan length and renewals. I've cloned the bug to bug 37354 and will submit there so we don't end up with conflicting alternative patches here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|martin.renvoize@ptfs-europe |thibaud.guillot@biblibre.co |.com |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #29 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Thanks that's a good thing ! I will take a look at the bug you mentioned ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |paul.derscheid@lmscloud.de --- Comment #30 from Paul Derscheid <paul.derscheid@lmscloud.de> --- I think this needs a rebase: error: sha1 information is lacking or useless (Koha/Items.pm). error: could not build fake ancestor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #31 from Paul Derscheid <paul.derscheid@lmscloud.de> --- I'm getting: Missing right curly or square bracket at /kohadevbox/koha/Koha/Item.pm line 2686, at end of line syntax error at /kohadevbox/koha/Koha/Item.pm line 2686, at EOF when running reset_all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #32 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Take a look at commit 7608e0ae6d36ad849851e975202b53ca6d674229 and its changes to Koha/Item.pm Seems something went off there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #166962|0 |1 is obsolete| | --- Comment #33 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- Created attachment 170197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170197&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #34 from Thibaud Guillot <thibaud.guillot@biblibre.com> --- (In reply to Paul Derscheid from comment #31)
I'm getting:
Missing right curly or square bracket at /kohadevbox/koha/Koha/Item.pm line 2686, at end of line syntax error at /kohadevbox/koha/Koha/Item.pm line 2686, at EOF
when running reset_all.
Thanks to you Paul.. I missed up the last rebase ;) that's fixed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Olivier Hubert <olivier.hubert@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |olivier.hubert@inlibro.com --- Comment #35 from Olivier Hubert <olivier.hubert@inlibro.com> --- The patch does not apply on the current main branch: Bug 36271 - Bookings should have circulation rules 170197 - Bug 36271: Add circulation rules for booking Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36271: Add circulation rules for booking error: sha1 information is lacking or useless (Koha/Item.pm). error: could not build fake ancestor Patch failed at 0001 Bug 36271: Add circulation rules for booking -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170197|0 |1 is obsolete| | --- Comment #36 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- Created attachment 172866 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172866&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172866|0 |1 is obsolete| | --- Comment #37 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 172924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172924&action=edit 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) Signed-off-by: esther <esther@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Esther Melander <esther.melander@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |esther.melander@bywatersolu | |tions.com 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=36271 Marie <marie.hedbom@musikverket.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marie.hedbom@musikverket.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|35906 |37354 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35906 [Bug 35906] Add bookable option on itemtypes https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37354 [Bug 37354] Bookings should respect circulation rules for max loan periods -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #38 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Comment on attachment 172924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172924 Bug 36271: Add circulation rules for booking Review of attachment 172924: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36271&attachment=172924) ----------------------------------------------------------------- ::: Koha/Booking.pm @@ +53,5 @@
+ { status => tooLongBookingPeriod, limit => $limit }, if the borrower has exceeded their maximum booking period. + { status => bookingPeriodNotValid }, if booking period is not valid (undef or equal to 0). +=cut + +sub can_be_booked_in_advance {
Whilst this is great for the serverside logic, the rest of the bookings flow on the client allows for upfront notification as soon as possible regarding bookability. We should update the bookings modal to react to the choices made as soon as possible as exemplified by recent bugs. You are 100% correct to take some of the other circulation rules into account, age-related restrictions for example, but these should really have been reported as independent bugs and clear code submitted to block at the API level and also expose quickly in the Modal as soon as the patron is selected for example. @@ +91,5 @@
+ my $bookings_period_length = $rights->{bookings_period_length} || 0; + + return { status => 'noBookingsAllowed' } if defined($bookings_allowed_total) && $bookings_allowed_total == 0; + + my $bookings_per_item_count = Koha::Bookings->search( { patron_id => $patron->borrowernumber, item_id => $item->itemnumber } )->count();
Given that bookings are time bound and not a replenishing queue.. should this rule also be time bound or clarified somehow. What does it mean? * "Active" bookings waiting to be fulfilled? * Bookings allowed in a year, a quarter, a term? ::: Koha/CirculationRules.pm @@ +227,5 @@
+ }, + bookings_per_item => { + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, + bookings_period_length => {
With bug 37354 now implemented using existing loan rules to construct durations we probably don't need this one.. or we need to reconsider it concerning the handling introduced in the aforementioned bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Bug 36271 depends on bug 37354, which changed state. Bug 37354 Summary: Bookings should respect circulation rules for max loan periods https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37354 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #39 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Comment on attachment 172924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172924 Bug 36271: Add circulation rules for booking Review of attachment 172924: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36271&attachment=172924) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js @@ +1084,5 @@
});
posting.fail(function (data) { + var error = data.responseJSON.error; + var errorMessage = error ? error : "Failure";
"Failure" will not be extracted to be added to .po. It has to be enclosed in the right i18n function. @@ +1089,3 @@
$("#booking_result").replaceWith( '<div id="booking_result" class="alert alert-danger">' + + _(errorMessage) +
If my notes about the i18n process are correct, _("") wouldn't work in a .js file. And __("") should be kept. https://wiki.koha-community.org/wiki/User:Victor_Grousset_-_tuxayo/Translati... Was the change made after witnessing _("") failing and __("") working? Or is that just a typo? If the above occurrence of "Failure" is enclosed in the right i18n function. Then that function can't be used again on the same string. It might work because passing a translated string won't match a source (english) string in the .po. And it won't change the string which is already translated. Even if it works,it's not how the i18n functions should be used. The root issue is data.responseJSON.error coming from the server. And mixing it with "Failure". But before handling having a default message. As it stands, _(errorMessage) will not cause the various detailed error messages sent by the server to be extracted to -messages-js.po. Having separate .po files means we can't do a trick by counting on the server side code to make then available anyway. You can look for the 3 other occurrences of data.responseJSON.error and they implement i18n. I don't know if there is still a better way but at least it should work. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #40 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Comment on attachment 172924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172924 Bug 36271: Add circulation rules for booking Review of attachment 172924: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36271&attachment=172924) ----------------------------------------------------------------- ::: Koha/REST/V1/Bookings.pm @@ +104,5 @@
+ 'noBookingsAllowed' => 'Bookings are not allowed according to circulation rules', + 'tooManyBookings' => sprintf('Patron has reached the maximum of booking according to circulation rules : %s maximum (%s)', $limit, $rule), + 'tooLongBookingPeriod' => sprintf('Booking period exceed booking period limit according to circulation rules : %s day(s)', $limit), + 'bookingPeriodNotValid' => sprintf('Booking period must be valid'), + 'ageRestricted' => "Age restricted",
It's not translatable and it can't be due to being in a .pm. https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML2:_HTML_inside_Pe... A way (unless there something better) would be to pass the tokens (noBookingsAllowed/tooManyBookings/etc) directly and some variables that will be needed to construct the error message client side. Here the rule wouldn't have been translatable either, so that will also allow that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172924|0 |1 is obsolete| | --- Comment #41 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- Created attachment 184855 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184855&action=edit 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 / bookings_period_length By branches, you can set quotas either generally or by item. Note that 'Booking period length' add a way to limit the booking period, pre or post process is used (I think :) ) only for staff usage to prepare items, here it's a way of restricting the possible length of time during which you can book. Test plan: 1) Apply this patch 2) Set values for these new rules 3) Try to perform some bookings with different rules values (also on the OPAC side) Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community |https://bugs.koha-community |.org/bugzilla3/show_bug.cgi |.org/bugzilla3/show_bug.cgi |?id=37354 |?id=33738 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #42 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- (In reply to Martin Renvoize (ashimema) from comment #38)
Comment on attachment 172924 [details] [review] Bug 36271: Add circulation rules for booking
Review of attachment 172924 [details] [review]: -----------------------------------------------------------------
::: Koha/Booking.pm @@ +53,5 @@
+ { status => tooLongBookingPeriod, limit => $limit }, if the borrower has exceeded their maximum booking period. + { status => bookingPeriodNotValid }, if booking period is not valid (undef or equal to 0). +=cut + +sub can_be_booked_in_advance {
Whilst this is great for the serverside logic, the rest of the bookings flow on the client allows for upfront notification as soon as possible regarding bookability.
We should update the bookings modal to react to the choices made as soon as possible as exemplified by recent bugs.
You are 100% correct to take some of the other circulation rules into account, age-related restrictions for example, but these should really have been reported as independent bugs and clear code submitted to block at the API level and also expose quickly in the Modal as soon as the patron is selected for example.
@@ +91,5 @@
+ my $bookings_period_length = $rights->{bookings_period_length} || 0; + + return { status => 'noBookingsAllowed' } if defined($bookings_allowed_total) && $bookings_allowed_total == 0; + + my $bookings_per_item_count = Koha::Bookings->search( { patron_id => $patron->borrowernumber, item_id => $item->itemnumber } )->count();
Given that bookings are time bound and not a replenishing queue.. should this rule also be time bound or clarified somehow.
What does it mean? * "Active" bookings waiting to be fulfilled? * Bookings allowed in a year, a quarter, a term?
::: Koha/CirculationRules.pm @@ +227,5 @@
+ }, + bookings_per_item => { + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, + bookings_period_length => {
With bug 37354 now implemented using existing loan rules to construct durations we probably don't need this one.. or we need to reconsider it concerning the handling introduced in the aforementioned bug.
Hi Martin and thanks for your feedback, sorry for the delay in my reply :) I've tried to clarify the “length of booking period” rule, I think it might be interesting to have this rule to limit patrons in their bookings. I've added a dependency to the OPAC patch precisely for this kind of test and easier integration. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #43 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- (In reply to Victor Grousset/tuxayo from comment #40)
Comment on attachment 172924 [details] [review] Bug 36271: Add circulation rules for booking
Review of attachment 172924 [details] [review]: -----------------------------------------------------------------
::: Koha/REST/V1/Bookings.pm @@ +104,5 @@
+ 'noBookingsAllowed' => 'Bookings are not allowed according to circulation rules', + 'tooManyBookings' => sprintf('Patron has reached the maximum of booking according to circulation rules : %s maximum (%s)', $limit, $rule), + 'tooLongBookingPeriod' => sprintf('Booking period exceed booking period limit according to circulation rules : %s day(s)', $limit), + 'bookingPeriodNotValid' => sprintf('Booking period must be valid'), + 'ageRestricted' => "Age restricted",
It's not translatable and it can't be due to being in a .pm. https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML2: _HTML_inside_Perl_scripts
A way (unless there something better) would be to pass the tokens (noBookingsAllowed/tooManyBookings/etc) directly and some variables that will be needed to construct the error message client side.
Here the rule wouldn't have been translatable either, so that will also allow that.
Hi Victor and thanks too for your feedback ;) I've tweaked the code so that the translations and the way it's done are okay ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Circulation| |Bookings function| | --- Comment #44 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, this no longer applies: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36271: Add circulation rules for booking error: sha1 information is lacking or useless (Koha/REST/V1/Bookings.pm). error: could not build fake ancestor Patch failed at 0001 Bug 36271: Add circulation rules for booking hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-36271-Add-circulation-rules-for-booking-2vhey347.patch I wonder how well this development fits with the other ongoing work on bookings? Would be good to have a short assessment from one of the others more involved with this code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Fiona Borthwick <fiona.borthwick@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fiona.borthwick@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Aude Charillon <aude.charillon@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aude.charillon@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=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|37354 |33738 Status|Patch doesn't apply |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33738 [Bug 33738] Add bookings to the OPAC https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37354 [Bug 37354] Bookings should respect circulation rules for max loan periods -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184855|0 |1 is obsolete| | --- Comment #45 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- Created attachment 204001 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204001&action=edit 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 / bookings_period_length By branches, you can set quotas either generally or by item. Note that 'Booking period length' add a way to limit the booking period, pre or post process is used (I think :) ) only for staff usage to prepare items, here it's a way of restricting the possible length of time during which you can book. Test plan: 1) Apply this patch 2) Set values for these new rules 3) Try to perform some bookings with different rules values (also on the OPAC side) Sponsored by: Association de Gestion des Œuvres Sociales d'Inria (AGOS) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36271 --- Comment #46 from Thibaud Guillot (thibaud_g) <thibaud.guillot@biblibre.com> --- Created attachment 204002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204002&action=edit Bug 36271: (QA follow-up) Tidy code and fix POD heading mismatch on _type -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org