Bug 13374 - AllowHoldsOnPatronsPossessions not working
Summary: AllowHoldsOnPatronsPossessions not working
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
: 15873 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-12-02 15:51 UTC by Jorge de Cardenas
Modified: 2020-01-08 23:42 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13374 - AllowHoldsOnPatronsPossessions not working (1.84 KB, patch)
2017-07-20 18:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13374 - AllowHoldsOnPatronsPossessions not working (1.91 KB, patch)
2017-08-03 15:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13374 - AllowHoldsOnPatronsPossessions not working (1.99 KB, patch)
2017-08-04 06:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jorge de Cardenas 2014-12-02 15:51:35 UTC
We are trying to used AllowHoldsOnPatronsPossessions preference set to "Don't Allow" and it works only some times.

opac-reserve.pl calls CheckIfIssuedToPatron($borrowernumber, $biblionumber) in Circulation.pm

that calls GetItemsByBiblioitemnumber($biblionumber) in Items.pm

that is using a query "SELECT * FROM items WHERE items.biblioitemnumber = ?"

The problem is the query is searching for biblioitemnumber but we are passing biblionumber.

I don't know if this is not a problem for other libraries but most of our biblioitemnumber DO NOT EQUAL biblionumber.

It seems that we should not be using biblioitemnumbers for anything.
Comment 1 Jorge de Cardenas 2014-12-02 16:01:26 UTC
I only found 4 places using GetItemsByBiblioitemnumber()

C4/Items.pm - function defined
C4/Circulations.pm - called using biblionumber
C4/ILSDI/Services.pm - called using biblioitemnumber
intranet/cgi-bin/cataloguing/additem.pl - called using biblioitemnumber

I propose:
1) renaming function to GetItemsByBiblionumber
2) changing all call to use biblionumber
Comment 2 Owen Leonard 2016-08-10 18:37:45 UTC
(In reply to Jorge de Cardenas from comment #0)
> We are trying to used AllowHoldsOnPatronsPossessions preference set to
> "Don't Allow" and it works only some times.

In master it seems to be working to block holds in the OPAC and staff client, so unless someone can come up with an exception I think this bug should be considered fixed.
Comment 3 Kyle M Hall 2017-07-20 18:12:08 UTC
I can reproduce this on master.
Comment 4 Kyle M Hall 2017-07-20 18:34:55 UTC
Created attachment 65158 [details] [review]
Bug 13374 - AllowHoldsOnPatronsPossessions not working

Test Plan:
1) Set AllowHoldsOnPatronsPossessions to "Don't"
2) Check out an item to a patron
3) Place a hold on that item for the same patron
4) Note you are allowed to with no alert
5) Delete the hold
6) Apply this patch
7) Place a hold on that item for the same patron
8) Note you recieve an alert now
Comment 5 Owen Leonard 2017-08-03 15:52:29 UTC
Created attachment 65477 [details] [review]
[SIGNED-OFF] Bug 13374 - AllowHoldsOnPatronsPossessions not working

Test Plan:
1) Set AllowHoldsOnPatronsPossessions to "Don't"
2) Check out an item to a patron
3) Place a hold on that item for the same patron
4) Note you are allowed to with no alert
5) Delete the hold
6) Apply this patch
7) Place a hold on that item for the same patron
8) Note you recieve an alert now

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Marcel de Rooy 2017-08-04 06:37:16 UTC
Created attachment 65489 [details] [review]
Bug 13374 - AllowHoldsOnPatronsPossessions not working

Test Plan:
1) Set AllowHoldsOnPatronsPossessions to "Don't"
2) Check out an item to a patron
3) Place a hold on that item for the same patron
4) Note you are allowed to with no alert
5) Delete the hold
6) Apply this patch
7) Place a hold on that item for the same patron
8) Note you recieve an alert now

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2017-08-08 12:29:41 UTC
I am not sure the patch fixes what has been reported originally.

This patch fixes a problem added by
  commit bc39f0392bbebaad4c083f81308f652a325be042
    Bug 14695 - Add ability to place multiple item holds on a given record per patron

Which has been opened and pushed after this bug has been reported.
Comment 8 Jonathan Druart 2017-08-08 19:36:56 UTC
Kyle, I think a new bug report must be opened, the original issue seems fixed.
Comment 9 Nick Clemens 2017-10-13 14:58:39 UTC

*** This bug has been marked as a duplicate of bug 19135 ***
Comment 10 Jonathan Druart 2017-10-13 15:39:30 UTC
(In reply to Nick Clemens from comment #9)
> 
> *** This bug has been marked as a duplicate of bug 19135 ***

Nope, it's not a duplicate, but it's fixed indeed.
Comment 11 Katrin Fischer 2020-01-08 23:42:28 UTC
*** Bug 15873 has been marked as a duplicate of this bug. ***