https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |18928 See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22431 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- To test: 1 - Disable strict mysql modes 2 - Place some reserves on a biblio with some items 3 - Browse to Circulation->Holds to pull 4 - Note there are no results in report 5 - Check logs - MySQL error 6 - Apply patch 7 - Refresh page, should load! 8 - Mark an itemtype as nothholdable in default circ rules 9 - Those items no longer show in report Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18928 [Bug 18928] Move holdallowed, hold_fulfillment_policy, returnbranch to circulation_rules -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=23484 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The error is: DBD::mysql::st execute failed: Table 'koha_kohadev.default_branch_item_rules' doesn't exist [for Statement "SELECT min(reservedate) as l_reservedate, -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #4)

I am wondering if we should not retrieve the list of itemtypes for which the holds are allowed, then use them in the WHERE clause. It will make the query more readable and we will remove a LEFT JOIN. What do you think?

That makes some sense, but will be two queries instead of one? In either case I think we should fix this now and re-engineer this report on another bug -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 --- Comment #8 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 93731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93731&action=edit Bug 23484: (follow-up) add parenthesis in JOIN -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED CC| |m.de.rooy@rijksmuseum.nl --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QAing -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: [1] Nick, please tell me if we strictly need the rule. If a hold is already placed (it passed the rules of that moment), shouldn't we allow it? The rule should be used when placing the hold, not pulling it. If we dont pull it, this hold will be in the system indefinitely. [2] Secondly, the rule applies to item level holds only. You are listing biblio level holds, but hey the items of that book could all be 'forbidden'? Note that this says more about circulation rules than the pull report btw. [3] If we follow the rule, shouldnt we make a distinction between From any library and From home library. In other words: why dont you check the branchcode here? My recommendation would be cf [1] remove this condition. Keep it simple when you pull holds. Changing status for need of feedback. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |BLOCKED --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #12)

I am not sure I agree about 1): If you have it show up on the holds to pull report, it should be made sure that the items you pull can trigger the hold on return. Hiding the itemtypes that are not holdable makes sense to me.

Changing status again. Still rethinking a bit about what you say. Thx for feedback.

But more important, as this is a bug fix, the page should behave the same as it behaved before we broke it for now and changes should be discussed separately.

Hmm. A fix should not be a new bug? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- So, I will pass QA. The only thing raised about a hold that no longer should be a hold since it does no longer pass circ rules, should not block this report indeed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93728|0 |1 is obsolete| | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 93985 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93985&action=edit Bug 23484: Update pendingreserves.pl to use circulation_rules Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Nick Clemens from comment #5)

(In reply to Jonathan Druart from comment #4)

...

I am wondering if we should not retrieve the list of itemtypes for which the holds are allowed, then use them in the WHERE clause. It will make the query more readable and we will remove a LEFT JOIN. What do you think?

That makes some sense, but will be two queries instead of one? In either case I think we should fix this now and re-engineer this report on another bug

2 queries but 1 LEFT JOIN removed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23484 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #21 from Lucas Gass <lucas@bywatersolutions.com> --- missing dependency for 18.11.x, no backport -- You are receiving this mail because: You are watching all bug changes.