Bug 7970 - Allow OPAC holds per library policy without default rule
Summary: Allow OPAC holds per library policy without default rule
Status: CLOSED DUPLICATE of bug 5795
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P1 - high major (vote)
Assignee: Amit Gupta
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-17 09:44 UTC by Amit Gupta
Modified: 2016-01-15 14:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Hold Request (67.72 KB, image/png)
2012-04-17 11:09 UTC, Amit Gupta
Details
Holds not working in OPAC (1.14 KB, patch)
2012-04-17 11:36 UTC, Amit Gupta
Details | Diff | Splinter Review
Bug 7970 - Allow OPAC holds per library policy without default rule (3.34 KB, patch)
2012-05-03 07:57 UTC, Amit Gupta
Details | Diff | Splinter Review
Update the database as follow-up to Bug 5795 (2.36 KB, patch)
2012-05-03 12:26 UTC, Amit Gupta
Details | Diff | Splinter Review
Update the database as follow-up to Bug 5795 (2.18 KB, patch)
2012-05-08 11:26 UTC, Amit Gupta
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Amit Gupta 2012-04-17 09:44:03 UTC
Holds not working in OPAC
Comment 1 Katrin Fischer 2012-04-17 09:49:22 UTC
Hi Amit, 

can you give some more details? Which error message is displayed?
Item level or record level holds?
Have you checked your settings for maxreserves, AllowOnShelfHolds, ...?
Do you have a default rule definded for all libraries all itemtypes and all patron categories with an allowed number of holds > 0?

I am resetting the version to master - 3.8 should only be used by the RM at the moment or later after release to file a bug specific for 3.8.
Comment 2 Savitra Sirohi 2012-04-17 11:05:34 UTC
Katrin, it works if we setup a "default rule definded for all libraries all itemtypes and all patron categories with an allowed number of holds > 0".

When such a rule is not defined, the error "sorry, none of the items can be placed on hold", also "title cannot be requested" is shown. 

The problem seems to be that in CanBookbeReserved:

my @items = get_itemnumbers_of($biblionumber);
   #get items linked via host records
   my @hostitems = get_hostitemnumbers_of($biblionumber);
   
   foreach my $item (@items){
       return 1 if CanItemBeReserved($borrowernumber, $item);
   }
Comment 3 Amit Gupta 2012-04-17 11:09:23 UTC Comment hidden (obsolete)
Comment 4 Amit Gupta 2012-04-17 11:36:45 UTC Comment hidden (obsolete)
Comment 5 Savitra Sirohi 2012-04-17 12:59:27 UTC
My comment above got truncated for some reason:

Katrin, it works if we setup a "default rule definded for all libraries all itemtypes and all patron categories with an allowed number of holds > 0".

When such a rule is not defined, the error "sorry, none of the items can be placed on hold", also "title cannot be requested" is shown. 

The problem seems to be that in CanBookbeReserved:

my @items = get_itemnumbers_of($biblionumber);
   #get items linked via host records
   my @hostitems = get_hostitemnumbers_of($biblionumber);
   
   foreach my $item (@items){
       return 1 if CanItemBeReserved($borrowernumber, $item);
   }

CanItemBeReserved is expecting an itemnumber, but is receiving a hashref instead.
Comment 6 Chris Cormack 2012-04-19 03:49:33 UTC
Doesn't this totally change the behaviour?

-	push (@items,@hostitems);
+        push (@hostitems,$items);

Then

-    foreach my $item (@items){
+    my %itemhash = %$items;
+        foreach my $item (keys %itemhash) {

These are not equivalent, you are now totally ignoring the hostitems, whereas before it was looping over them
Comment 7 Amit Gupta 2012-05-03 07:57:02 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2012-05-03 09:25:44 UTC
Hi Amit, 
I wrote a patch for bug 5795 to add the missing entry to the sql file. I think you are right, that we also need an updatedatabase. Would it perhaps be possible to check for existence of the system preference in the update? So we could avoid a red error message for users who already have the preference.
The Patch for 5759 is signed off - but missing the update. How to proceed?
Comment 9 Fridolin SOMERS 2012-05-03 09:41:53 UTC
Hie, you can mark bug 5795 as duplicate of this one.

> Would it perhaps be possible to check for existence of the system preference in the update? So we could avoid a red error message for users who already have the preference.

I agree.
Add an existence test in update and patch will be ok.
Comment 10 Katrin Fischer 2012-05-03 11:18:32 UTC
Amit - perhaps you could add another patch to update the database as follow-up to 5795?
Comment 11 Amit Gupta 2012-05-03 11:23:23 UTC
Kartin,

I am confuse if value already exists in database so we need to update database version?
Comment 12 Katrin Fischer 2012-05-03 11:35:41 UTC
Hi Amit,
I think we have a problem here where some installations have the value and others not. Apparently it was missing from the syspref.sql file for a while, so new installations don't have the value. Older installations got it with an earlier database update.
Comment 13 Amit Gupta 2012-05-03 12:26:35 UTC Comment hidden (obsolete)
Comment 14 Fridolin SOMERS 2012-05-03 13:35:36 UTC
If value exists, why update it with the same value ? (see patch 9398)

Note that even in a database where pref was missing at install, a change of it in intranet would create it in database.
That's why if value exists, no need to do something.

Someone agree ?
Comment 15 Katrin Fischer 2012-05-03 13:39:26 UTC
I agree that a change would create it and that thre is no need to change or udate if it already exists - I think this is what Amit's follow-up achieves, but have not tested it yet.
Comment 16 Fridolin SOMERS 2012-05-03 13:48:15 UTC
if (C4::Context->preference('ReservesControlBranch')){
    my $sth = $dbh->prepare("UPDATE systempreferences SET value = ? WHERE variable = 'ReservesControlBranch'");
    $sth->execute(C4::Context->preference('ReservesControlBranch'));
} ...
This part of patch updates pref with the same value.
Comment 17 Katrin Fischer 2012-05-03 13:54:22 UTC
Oh, I see it now, I think you are right, we only need to do something if it doesn't exist at all.
Comment 18 Chris Cormack 2012-05-07 01:08:14 UTC
I agree no need to update it when it hasn't changed.
Comment 19 Amit Gupta 2012-05-08 11:26:24 UTC
Created attachment 9450 [details] [review]
Update the database as follow-up to Bug 5795

If sys pref ReservesControlBranch has null then update the table to insert a ItemHomeLibrary as value

To Test:
1) Go to mysql use koha database
2) select * from systempreferences where variable = 'ReservesControlBranch'; Check if value set to ItemHomeLibrary.
+-----------------------+-----------------+-------------------------------+-------------------------------------------------+--------+
| variable              | value           | options                       | explanation                                     | type   |
+-----------------------+-----------------+-------------------------------+-------------------------------------------------+--------+
| ReservesControlBranch | ItemHomeLibrary | ItemHomeLibrary|PatronLibrary | Branch checked for members reservations rights. | Choice |
+-----------------------+-----------------+-------------------------------+-------------------------------------------------+--------+
Comment 20 Amit Gupta 2012-05-10 03:25:13 UTC

*** This bug has been marked as a duplicate of bug 5795 ***