Summary: | Holds placed when all libraries are closed do not get added to holds queue if HoldsQueueSkipClosed and RealTimeHoldsQueue are enabled | ||
---|---|---|---|
Product: | Koha | Reporter: | Emily Lamancusa <emily.lamancusa> |
Component: | Hold requests | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | Pushed to stable --- | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | bugzilla, daniel.gaghan, fridolin.somers, gmcharlt, Laura.escamilla, lisette.scheer, michaela.sieber, nick, pedro.amorim, rcoert, sbrown, tomascohen, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34676 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32328 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36817 |
||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This patch set adds a new option "--unallocated" to the build_holds_queue.pl cronjob.
This option prevents deletion of the existing queue, and looks for new unassigned holds that may be mapped to available items.
There are two intended uses for the option, depending on whether the 'RealTimeHoldsQueue' (RTHQ) system preference is enabled or not.
Without RTHQ libraries who want a more static holds queue during the day can run an hourly 'unallocated' cronjob. This will add new holds to the queue as they come in, but allow libraries longer to fill the holds in their existing queue before they move on. The recommendation would then be a nightly full run to rebuild the queue entirely.
With RTHQ, libraries could run a nightly 'unallocated' cron to select holds for libraries that were not open on the previous day, and to select holds that have been unsuspended by another cronjob.
Current setups will continue to function as before with no change, but libraries may wish to review their options after this upgrade.
|
Version(s) released in: |
24.05.00,23.11.06
|
Attachments: |
Bug 32565: WIP
Bug 32565: WIP Bug 32565: WIP - Minor fixes to have WIP working Bug 32565: Add unallocated option to holds queue Bug 32565: Unit tests Bug 32565: Add unallocated option to holds queue Bug 32565: Unit tests Bug 32565: QA Follow-up: Fix tests Bug 32565: Remove unallocated param from cron script Bug 32565: Add unallocated option to holds queue Bug 32565: Unit tests Bug 32565: QA Follow-up: Fix tests Bug 32565: Add unallocated option to holds queue Bug 32565: Unit tests Bug 32565: (QA follow-up) Fix tests Bug 32565: (follow-up) Tidy Bug 32565: Add unallocated option to holds queue Bug 32565: Unit tests Bug 32565: (QA follow-up) Fix tests Bug 32565: (follow-up) Tidy |
Description
Emily Lamancusa
2023-01-04 15:32:05 UTC
I think the solution to this one may be server configuration - there is a need to run the holds queue cron daily/nightly to check for holds that may not have had an open library when placed Could the script be modified to only check for missing holds that haven't been allocated yet? Otherwise it will delete the whole table, mess with all the timestamps etc which would be a real set back now that we have RT. (In reply to Katrin Fischer from comment #2) > Could the script be modified to only check for missing holds that haven't > been allocated yet? Otherwise it will delete the whole table, mess with all > the timestamps etc which would be a real set back now that we have RT. I second this - in addition to the reasons Katrin mentioned, we've found (at least in testing) that the real-time holds queue also distributes the holds more equitably among our libraries than the cron job, so we'd like to preserve that! Running a modified holds queue cron would also be more in keeping with the goals of Bug 31995. We saw this recently at PCCLD, so +1 to fixing this. Created attachment 147659 [details] [review] Bug 32565: WIP Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected This will need unit tests Not taking this one right now, just had an idea to test, so if someone else wants to pickup unit tests please do Created attachment 161231 [details] [review] Bug 32565: WIP Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected This will need unit tests Created attachment 161232 [details] [review] Bug 32565: WIP - Minor fixes to have WIP working This is my understanding of the suggested solution: In order to address the problem reported in the original post, one would have to configure the cronjob entry "build_holds_queue.pl --unallocated" on top of the sys pref configurations stated in the original post (HoldsQueueSkipClosed = "open" and RealTimeHoldsQueue = "enabled"). In other words, the patch alone does not prevent the issue, it aims to fix the issue after it has occurred, when the cron runs by allocating the unallocated holds. Please correct me if I'm wrong. Observations with the patch: - My understanding is that RealTimeHoldsQueue and the build_holds_queue.pl cronjob are/were designed to be mutually exclusive, or at least for the most part, i.e. either use one of the other (?). This patch enforces the relationship between the syspref+cronjob, i.e. one would be expected to use both RealTimeHoldsQueue and the build_holds_queue.pl cronjob (at least when HoldsQueueSkipClosed is set to "open"). I think this is confusing. I think we should have one way or the other but, honestly/ideally, just one standard way of doing this. - With all of the above said, and having tested the patch, I don't think it does what it's designed to do. To recreate, clean k-t-d: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 At this point I would have expected the unallocated hold to have been placed, but it isn't. 13) Lastly, run the cron script without 'unallocated': perl misc/cronjobs/holds/build_holds_queue.pl --force 14) Notice both holds are now in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 My understanding is that the downside of running the cron with --force while having the syspref configured as above, is that the whole queue is rebuilt and some items previously allocated may be swapped around (?), and this is why having the cron with --force plus the sysprefs is not recommended. Correct me if I'm wrong please. I don't have a proposed fix of my own as I don't think I understand any of this well enough to provide one efficiently. Trying to help untangle this, in hope of not making it worse :) Problem: When all libraries owning a requested item are closed, we are not able to allocate a hold. Past: In the past and without RTHoldsQueue the holdsqueue was rebuilt completely. As a result, the unallocated holds would get allocated. Present: We now have RTHoldsQueue and try to avoid running the build_holdsqueue additionally, so we need a way to deal with the unallocated holds without still rebuilding the table regularly. I think having a switch on the build_holdsqueue that does prevent it deleting the table and makes it only add the so far unallocated would be OK. What could be alternatives? * Having a separate script: not ideal really as it would be too similar really. * Have some other way of triggering re-allocation - I don't think we have an option there apart from cron? (In reply to Katrin Fischer from comment #10) > * Have some other way of triggering re-allocation - I don't think we have an > option there apart from cron? Discussed this with Martin, an option would perhaps have it be a "scheduled job" (think background jobs, but scheduled to run at a specific given time instead of whenever possible), but I don't think we're there yet. Brainstorm idea, but if a hold is placed today and no library is open today, why not have RTHoldsQueue check tomorrow? and the day after that, and so on, until it finds a suitable one? (within a sensible limit) That's what would happen anyway through the cron, right? Maybe we'd have to add a way of flagging the hold that it can't be satisfied immediately as the library is closed but it could show the hold as "pre-allocated". This may not be a great idea, but let me know what the problems are with it as I lack the hold queues workflow knowledge required to see it. Pre-allocation will require new logic that we currently not have and has some issues that we could encounter. Allocation right now can be random (StaticHoldsQueueWeight), static (RandomizeHoldsQueueWeight) or driven by the transport cost matrix (UseTransportCostMatrix). Only "open" libraries will be looked at with HoldsQueueSkipClosed. So we have a complex system. When pre-allocating, we could do open libraries first, but when all are closed, should we take the duration of the closing into account? It could be just a holiday closure or it could be library that is closed for months. What if the calendar entries were changed after pre-allocating (closing period is extended)? The hold could get stuck in limbo basically forever, while it sits on the shelf in a library close by. I believe pre-allocating is not good, as we don't have a way to "review" those. What do you mean by have RTHoldsQueue check tomorrow? RTHoldsQueue means that every time a hold/item/biblio is added or updated we trigger a 'review' of the holds queue entry for the bibliographic record concerned. It's triggered by an action - so we'd still need to locate the unallocated and trigger this step for these holds. Created attachment 161393 [details] [review] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Created attachment 161394 [details] [review] Bug 32565: Unit tests This is similar to another bug we filed 3467 Pedro, if you want to pursue something different here I am happy to move my patches to that bug. Otherwise we could combine the two Created attachment 161679 [details] [review] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 161680 [details] [review] Bug 32565: Unit tests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> (In reply to Nick Clemens from comment #15) > This is similar to another bug we filed 3467 > > Pedro, if you want to pursue something different here I am happy to move my > patches to that bug. Otherwise we could combine the two Hey, thanks Nick! Yeah, I think bug 34676 should definitely be made a duplicate of this. I've signed-off and I believe this is good to go, although I have 2 observations: 1) With this new enhancement, would there ever be a situation where using RTHQ without the cron build_holds_queue.pl --force --unallocated be advisable? In other words, it seems that it would always be beneficial to run RTHQ in conjunction with the cron build_holds_queue.pl --force --unallocated, in order to catch unallocated holds "just in case". If the above is true, we're moving from: "It's recommended to use RTHQ or the build_holds_queue.pl cron, not both" to "RTHQ should be used in conjunction with build_holds_queue.pl --force --unallocated cron". This works, but it may be confusing in terms of system administration. Maybe not, ignore if that's the case. 2) Should we rename the unallocated option to unallocated-only to make it clearer? We're not adding "unallocated" on top of what the script already does, instead we're making the script consider unallocated holds only, correct? Ignore if not. Good questions Pedro. I do not know enough that functional area to help here. But at least here is a finding on another thing: test "Test unallocated option" actually doesn't test it. 1. Apply test patch 2. run tests 3. They pass! :o they shouldn't since there is not implementation Created attachment 162269 [details] [review] Bug 32565: QA Follow-up: Fix tests Apply original tests patch + this follow-up. Run: prove t/db_dependent/HoldsQueue.t Notice tests fail. Apply fix patch, run the above prove command again. Notice tests pass. Created attachment 162297 [details] [review] Bug 32565: Remove unallocated param from cron script unallocated param is redundant, it is only relevant if RealTimeHoldsQueue is enabled, the cron should just fall back to unallocated mode if the sys pref is enabled. This simplifies system administration, and does not require current Koha installations to update their cron configuration entry to make use of this fix. I think we should add a documentation note recommending using this cronjob when RealTimeHoldsQueue is enabled Hey guys, I'd love some feedback on my latest patch. I think it makes sense considering build_holds_queue.pl --unallocated should be idempotent. I'm okay with it being dropped if you feel that's best or if I missed something. (In reply to Pedro Amorim from comment #22) > Hey guys, I'd love some feedback on my latest patch. > I think it makes sense considering build_holds_queue.pl --unallocated should > be idempotent. > I'm okay with it being dropped if you feel that's best or if I missed > something. In a system without RTHQ there are times a library would like the queue to remain static for a period of time, but have new holds be picked up. i.e. giving a library more time to fill holds until a full rebuild, so I think the switch is necessary even when not using the RTHQ. Having the switch allows this code to satisfy bug 34676 and 32328 at leats partially (In reply to Nick Clemens from comment #23) > (In reply to Pedro Amorim from comment #22) > > Hey guys, I'd love some feedback on my latest patch. > > I think it makes sense considering build_holds_queue.pl --unallocated should > > be idempotent. > > I'm okay with it being dropped if you feel that's best or if I missed > > something. > > In a system without RTHQ there are times a library would like the queue to > remain static for a period of time, but have new holds be picked up. i.e. > giving a library more time to fill holds until a full rebuild, so I think > the switch is necessary even when not using the RTHQ. Having the switch > allows this code to satisfy bug 34676 and 32328 at leats partially I've made my latest patch obsolete for these reasons, I had not considered this specific use case. As it stands now, the following cron entries configurations will work like this: Without RTHQ: 0 * * * * /usr/share/koha/bin/cronjobs/holds/build_holds_queue.pl --unallocated-only # run this hourly 0 1 * * * /usr/share/koha/bin/cronjobs/holds/build_holds_queue.pl # run this daily With RTHQ: 0 * * * * /usr/share/koha/bin/cronjobs/holds/build_holds_queue.pl --unallocated-only # run this hourly 0 1 * * * /usr/share/koha/bin/cronjobs/holds/build_holds_queue.pl # run this daily !This will not do anything because RTHQ is enabled and --force is missing Nick, please intervene if any of the above needs correcting. My opinion is that a note along the lines of "It is recommended to use the cron build_holds_queue.pl --unallocated-only cron if using RTHQ" would be useful in the UI. I think this is good to go and I don't have anything else to add, but because of my involvement in it I'll leave it to someone else to PQA. (In reply to Pedro Amorim from comment #20) > Created attachment 162269 [details] [review] [review] > Bug 32565: QA Follow-up: Fix tests > > Apply original tests patch + this follow-up. Run: > prove t/db_dependent/HoldsQueue.t > Notice tests fail. > Apply fix patch, run the above prove command again. > Notice tests pass. Confirmed :) Created attachment 164272 [details] [review] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164273 [details] [review] Bug 32565: Unit tests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 164274 [details] [review] Bug 32565: QA Follow-up: Fix tests Apply original tests patch + this follow-up. Run: prove t/db_dependent/HoldsQueue.t Notice tests fail. Apply fix patch, run the above prove command again. Notice tests pass. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> HoldsQueue.t needed a rebase after bug 35997 *** Bug 34676 has been marked as a duplicate of this bug. *** Created attachment 165166 [details] [review] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165167 [details] [review] Bug 32565: Unit tests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165168 [details] [review] Bug 32565: (QA follow-up) Fix tests Apply original tests patch + this follow-up. Run: prove t/db_dependent/HoldsQueue.t Notice tests fail. Apply fix patch, run the above prove command again. Notice tests pass. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165169 [details] [review] Bug 32565: (follow-up) Tidy Created attachment 165800 [details] [review] Bug 32565: Add unallocated option to holds queue Add an unallocated option to CreateQueue and pass through as needed Avoid deletion of the tmp_holdsqueue, and only check holds and items that are not currently matched A future hold with a higher priority will still fail here - because the item may already be assigned, but on next change to the biblio it would be corrected To test: 1) Apply both patches 2) Enable RealTimeHoldsQueue and set HoldsQueueSkipClosed to "open" 3) Add a holiday to the calendar for all libraries for today, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Holiday repeated every same day of the week" -- Click "Copy to all libraries". Hit "Save. 4) Place a biblio-level hold on a biblio record and set the pickup location to a library that has available copies, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 -- Click the first "Place hold" button to place the biblio-level hold. 5) Verify that that hold got added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 6) Place a biblio-level hold on a biblio record where there are no other holds and copies are available at another location, but not the pickup location, visit: -- /cgi-bin/koha/reserve/request.pl?biblionumber=437&borrowernumber=51 -- On the "pickup at" dropdown, pick something else other than "Centerville", e.g. "Fairfield". -- Click the first "Place hold" button to place the biblio-level hold. 7) Check the holds queue again, notice that this 2nd hold was not added to the queue: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 8) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 9) Notice nothing changed in the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 10) Remove the holiday we created previously, visit: /cgi-bin/koha/tools/holidays.pl -- Click today's day on the calendar and pick "Delete this holiday" -- Click "Copy to all libraries". Hit "Save. 11) Run the updated cronscript: perl misc/cronjobs/holds/build_holds_queue.pl --force --unallocated 12) Confirm the second hold is added to the holds queue, visit: /cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&ccodeslimit=&locationslimit=&run_report=1 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165801 [details] [review] Bug 32565: Unit tests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165802 [details] [review] Bug 32565: (QA follow-up) Fix tests Apply original tests patch + this follow-up. Run: prove t/db_dependent/HoldsQueue.t Notice tests fail. Apply fix patch, run the above prove command again. Notice tests pass. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165803 [details] [review] Bug 32565: (follow-up) Tidy (In reply to Pedro Amorim from comment #24) > > Nick, please intervene if any of the above needs correcting. > My opinion is that a note along the lines of "It is recommended to use the > cron build_holds_queue.pl --unallocated-only cron if using RTHQ" would be > useful in the UI. Agreed. And we need good release notes as well. Pushed for 24.05! Well done everyone, thank you! *** Bug 32778 has been marked as a duplicate of this bug. *** *** Bug 36817 has been marked as a duplicate of this bug. *** Pushed to 23.11.x for 23.11.06 |