Currently, the Holds To Pull List includes items that are unreserveable or cannot be placed on hold. For instance: Bib Record A has 2 items: Item Record X (can be placed on hold) Item Record Y (cannot be placed on hold) If you place a bib-level hold on Bib Record A, both Item Record X and Item Record Y show up as available items in the Holds To Pull List. -- Fixing this will actually probably involve re-writing the whole of pendingreserves.pl, which is probably long overdue anyway, since it includes a lot of SQL in the script itself.
We are observing this as well. For a record with 2 items: 1 item is not for loan (via itemtype) and on the shelf 1 item is checked out The holds to pull list shows the not for loan item.
We have this as well. If 1 item has no holds allowed and the other is checked out the no holds allowed will show up on the holds list.
Just a note that I have no plans to work on this currently, so I'm happy for someone else to work on fixing this bug.
This has been raised by one of our customers and I can see it being a big problem for the larger libraries. In our customer's case, they have marked certain item types - e.g. Reference as not for loan in the item type policy. This means that the 'not for loan' subfield in the item record has not been populated. It appears that the SQL for the Holds to Pull report is looking at the not for loan item attribute and therefore not considering the item type at all. There are 'Default holds policy by item type' set up to mark these item types with the value of No Holds Allowed. The customer is therefore confused as to why the Holds to Pull report should list these items. Of course the Reference items will not satisfy the hold so they can be ignored but in larger libraries they could have a large list. It seems that perhaps a solution would be to modify the SQL to add a JOIN to look at the item type policies too.
I have a customer expeiencing problems with this too. Could an easy solution be to have a checkbox on the itemtypes that says "Do not include in 'Holds to pull' report"? And then exlude items with that setting when the "Holds to pull" is generated?
Hm, I don't think we need more configuration options - we need the report to use the ones we have correctly.
Yeah, probably a good idea!
(In reply to Katrin Fischer from comment #6) > Hm, I don't think we need more configuration options - we need the report to > use the ones we have correctly. ...but I can't convince myself 100% that it will work. For example, inter library loans. When you start an ILL request you want to put a hold on the item, so there is a connection between the patron and the record/item. So if you use an itemtype for ILL that itemtype must be "holdable". At the same time the item will not be on loan, and so it will look like an "on the shelf hold" to the Holds to pull report, I think? Not sure I can see all the consequences, there might be ways to work around this, but perhaps something to consider when the time comes. BTW, this is the SQL that generates the holds to pull report: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=circ/pendingreserves.pl;h=7f03d88cc10a3f6af4c14711242201adb41dfb80;hb=HEAD#l174
I think I managed to take the 'Default holds policy by item type' into account in this report: SELECT min(reservedate) as l_reservedate, reserves.reserve_id, reserves.borrowernumber as borrowernumber, GROUP_CONCAT(DISTINCT items.holdingbranch ORDER BY items.itemnumber SEPARATOR '|') l_holdingbranch, reserves.biblionumber, reserves.branchcode as l_branch, reserves.itemnumber, items.holdingbranch, items.homebranch, GROUP_CONCAT(DISTINCT items.itype ORDER BY items.itemnumber SEPARATOR '|') l_item_type, GROUP_CONCAT(DISTINCT items.location ORDER BY items.itemnumber SEPARATOR '|') l_location, GROUP_CONCAT(DISTINCT items.itemcallnumber ORDER BY items.itemnumber SEPARATOR '<br/>') l_itemcallnumber, GROUP_CONCAT(DISTINCT items.enumchron ORDER BY items.itemnumber SEPARATOR '<br/>') l_enumchron, GROUP_CONCAT(DISTINCT items.copynumber ORDER BY items.itemnumber SEPARATOR '<br/>') l_copynumber, biblio.title, biblio.author, count(DISTINCT items.itemnumber) as icount, count(DISTINCT reserves.borrowernumber) as rcount, borrowers.firstname, borrowers.surname FROM reserves LEFT JOIN items ON items.biblionumber=reserves.biblionumber LEFT JOIN biblio ON reserves.biblionumber=biblio.biblionumber LEFT JOIN biblioitems ON biblio.biblionumber=biblioitems.biblionumber LEFT JOIN branchtransfers ON items.itemnumber=branchtransfers.itemnumber LEFT JOIN issues ON items.itemnumber=issues.itemnumber LEFT JOIN borrowers ON reserves.borrowernumber=borrowers.borrowernumber LEFT JOIN default_branch_item_rules ON items.itype=default_branch_item_rules.itemtype WHERE reserves.found IS NULL AND (reserves.itemnumber IS NULL OR reserves.itemnumber = items.itemnumber) AND items.itemnumber NOT IN (SELECT itemnumber FROM branchtransfers where datearrived IS NULL) AND items.itemnumber NOT IN (select itemnumber FROM reserves where found IS NOT NULL) AND issues.itemnumber IS NULL AND reserves.priority <> 0 AND reserves.suspend = 0 AND notforloan = 0 AND damaged = 0 AND itemlost = 0 AND withdrawn = 0 AND default_branch_item_rules.holdallowed != 0 GROUP BY reserves.biblionumber ORDER BY biblio.title What is changed compared to the "Holds to pull" SQL are these two lines: - LEFT JOIN default_branch_item_rules ON items.itype=default_branch_item_rules.itemtype - AND default_branch_item_rules.holdallowed != 0 However, default_branch_item_rules.holdallowed is of the "bit" type: http://schema.koha-community.org/18_05/tables/default_branch_item_rules.html I gotta confess I'm not familiar with that. Any special tricks to consider? When I look in the database, the value of that column is 0 or 2. Why not a boolean "0 or 1" value?
(In reply to Magnus Enger from comment #9) > What is changed compared to the "Holds to pull" SQL are these two lines: > > - LEFT JOIN default_branch_item_rules ON > items.itype=default_branch_item_rules.itemtype > - AND default_branch_item_rules.holdallowed != 0 This seems to fix the issue. > However, default_branch_item_rules.holdallowed is of the "bit" type: > http://schema.koha-community.org/18_05/tables/default_branch_item_rules.html > I gotta confess I'm not familiar with that. Any special tricks to consider? > When I look in the database, the value of that column is 0 or 2. Why not a > boolean "0 or 1" value? Because there are 3 values: From any library, From home library and not allowed (0)
Created attachment 89665 [details] [review] Bug 13640: Do not display unreserveable items on the Holds to pull screen We should only display the items that meet the hold policies Test plan: It would be good to have a huge list of holds displayed on the "Holds to pull" and confirm that the display is now correct. One of the possible test plan has been let in a comment on the bug report: Bib Record A has 2 items: Item Record X (can be placed on hold) Item Record Y (cannot be placed on hold) If you place a bib-level hold on Bib Record A, both Item Record X and Item Record Y show up as available items in the Holds To Pull List. With this patch applied you must not see Y
(In reply to Jonathan Druart from comment #10) > Because there are 3 values: From any library, From home library and not > allowed (0) I'm wondering if we're taking into account the 1, and 2 options correctly then in that case?
Created attachment 90639 [details] [review] Bug 13640: Do not display unreserveable items on the Holds to pull screen We should only display the items that meet the hold policies Test plan: It would be good to have a huge list of holds displayed on the "Holds to pull" and confirm that the display is now correct. One of the possible test plan has been let in a comment on the bug report: Bib Record A has 2 items: Item Record X (can be placed on hold) Item Record Y (cannot be placed on hold) If you place a bib-level hold on Bib Record A, both Item Record X and Item Record Y show up as available items in the Holds To Pull List. With this patch applied you must not see Y Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signing off as in a testing it appears to work as expected.. I'd love to see this query moved into a module and unit tested really, but this fix works for now in my opinion.. good luck QAers.
(In reply to Martin Renvoize from comment #12) > (In reply to Jonathan Druart from comment #10) > > Because there are 3 values: From any library, From home library and not > > allowed (0) > > I'm wondering if we're taking into account the 1, and 2 options correctly > then in that case? Why not? The patch adds the holdallowed != 0 condition, which matches 1 and 2 :)
(In reply to Jonathan Druart from comment #15) > (In reply to Martin Renvoize from comment #12) > > (In reply to Jonathan Druart from comment #10) > > > Because there are 3 values: From any library, From home library and not > > > allowed (0) > > > > I'm wondering if we're taking into account the 1, and 2 options correctly > > then in that case? > > Why not? The patch adds the holdallowed != 0 condition, which matches 1 and > 2 :) It was more of a 'note to self' to make sure I improved my understanding of how the value is used before Signing Off. It looks sane enough to me, but I'm still not 100%. What I'm trying to work out is if the clause is in the right place and is indeed correct allowing both 1 and 2 options. I note that with independent branches switched on we limit by holdingbranch being the local branch.. hence wondering if we aught to be handling library limitations in this part of the query too... It's one monster query.
QA: Looking here
Fails on strict sql mode: DBD::mysql::st execute failed: 'koha_master.reserves.reserve_id' isn't in GROUP BY [for Statement [ ... etc ]
The where condition is wrong. It should also incorporate the case that default_branch_item_rules is NULL (for istance, no corresponding record found for a biblio level hold). Will add a follow-up.
(In reply to Martin Renvoize from comment #14) > Signing off as in a testing it appears to work as expected.. > > I'd love to see this query moved into a module and unit tested really, but > this fix works for now in my opinion.. good luck QAers. It does not work, but I agree about the module thing ;)
Created attachment 90871 [details] [review] Bug 13640: Do not display unreserveable items on the Holds to pull screen We should only display the items that meet the hold policies Test plan: It would be good to have a huge list of holds displayed on the "Holds to pull" and confirm that the display is now correct. One of the possible test plan has been let in a comment on the bug report: Bib Record A has 2 items: Item Record X (can be placed on hold) Item Record Y (cannot be placed on hold) If you place a bib-level hold on Bib Record A, both Item Record X and Item Record Y show up as available items in the Holds To Pull List. With this patch applied you must not see Y Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 90872 [details] [review] Bug 13640: (QA follow-up) Allow NULL values for default_branch_item_rules.holdallowed If a rule has not been defined for an itemtype or you submit a biblio level hold, you cannot filter on holdallowed<>0. Test plan: Test with a biblio level hold. Have two items; one item should be blocked with a policy. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
@RM: Passing QA with a follow-up. Note that the current query also fails on strict SQL mode. So not blocking for that. And yes, it could be moved to a module on a new report.
Nice work! Pushed to master for 19.11.00
Pushed to 19.05.x for 19.05.02
Does this fix take into account patron type? We have some item types that are requestable by one type of patron category but not by other patron categories.