Summary: | Possible RealTimeHoldsQueue check missing in modrequest.pl for BatchUpdateBiblioHoldsQueue background job | ||
---|---|---|---|
Product: | Koha | Reporter: | Pedro Amorim <pedro.amorim> |
Component: | Architecture, internals, and plumbing | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | Pushed to oldstable --- | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | dcook, fridolin.somers, jonathan.druart, lucas, m.de.rooy, nick, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30728 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
24.05.00,23.11.06,23.05.12
|
Circulation function: | |
Bug Depends on: | |||
Bug Blocks: | 36675 | ||
Attachments: |
Bug 35979: Check pref before inserting holds_queue background jobs
Bug 35997: (follow-up) Add check in ->enqueue Bug 35979: Check pref before inserting holds_queue background jobs Bug 35979: (follow-up) Add check in ->enqueue Bug 35979: Check pref before inserting holds_queue background jobs Bug 35979: (follow-up) Add check in ->enqueue |
Description
Pedro Amorim
2024-02-01 14:24:02 UTC
(In reply to Pedro Amorim from comment #0) > Not sure if a bug or not, but bug 30728 introduced a check for the > RealTimeHoldsQueue everywhere the BatchUpdateBiblioHoldsQueue background job > is enqueued. > However, modrequest.pl enqueues BatchUpdateBiblioHoldsQueue without this > check. > Opening this bug to perhaps get confirmation on this or discover what bug > this may cause. Sure this is a bug. Patch coming. I saw lots of those jobs in my table while not using holds queue.. Created attachment 165380 [details] [review] Bug 35979: Check pref before inserting holds_queue background jobs Test plan: Confirm that modrequest does no longer insert when pref is off :) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Trivial: Self SO Grepped all the occurrences before reading this report. And the last one before the tests was this script ;) Not for now, but would be great to have this safe check be done once at BatchUpdateBiblioHoldsQueue::enqueue instead of having to use it every time that particular background job is invoked. Without a QA script check it's hard to make sure a bug like this won't creep up again. Another example of a safe check being WET is GetPlugins having to always check for C4::Context->config("enable_plugins") beforehand. Bit of a tangent, and not for now, sorry! Thanks, Marcel. Created attachment 165385 [details] [review] Bug 35997: (follow-up) Add check in ->enqueue Foundation for removing same check elsewhere (later on). Test plan: Run t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Pedro Amorim from comment #5) > Not for now, but would be great to have this safe check be done once at > BatchUpdateBiblioHoldsQueue::enqueue instead of having to use it every time > that particular background job is invoked. > Without a QA script check it's hard to make sure a bug like this won't creep > up again. > > Another example of a safe check being WET is GetPlugins having to always > check for C4::Context->config("enable_plugins") beforehand. > > Bit of a tangent, and not for now, sorry! > Thanks, Marcel. Follow-up adds foundation. This could be backported. Changing all occurrences is a future enhancement (will open new report). Created attachment 165395 [details] [review] Bug 35979: Check pref before inserting holds_queue background jobs Test plan: Confirm that modrequest does no longer insert when pref is off :) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 165396 [details] [review] Bug 35979: (follow-up) Add check in ->enqueue Foundation for removing same check elsewhere (later on). Test plan: Run t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> PA amended: Fix bug # in commit message Created attachment 165397 [details] [review] Bug 35979: Check pref before inserting holds_queue background jobs Test plan: Confirm that modrequest does no longer insert when pref is off :) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 165398 [details] [review] Bug 35979: (follow-up) Add check in ->enqueue Foundation for removing same check elsewhere (later on). Test plan: Run t/db_dependent/Koha/BackgroundJob/BatchUpdateBiblioHoldsQueue.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> PA amended: Fix bug # in commit message Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Pushed for 24.05! Well done everyone, thank you! What's the symptom for this one? Entries being added needlessly to a table like tmp_holdsqueue? (In reply to David Cook from comment #13) > What's the symptom for this one? Entries being added needlessly to a table > like tmp_holdsqueue? Yes Pushed to 23.11.x for 23.11.06 *** Bug 36968 has been marked as a duplicate of this bug. *** Backported to 23.05.x for upcoming 23.05.12 |