Bug 7710 - multiple holds per title
Summary: multiple holds per title
Status: RESOLVED DUPLICATE of bug 14695
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Target Milestone: ---
Assignee: Alex Arnaud
QA Contact:
URL:
Whiteboard:
Keywords:
: 4239 (view as bug list)
Depends on: 9394
Blocks: 7065
  Show dependency treegraph
 
Reported: 2012-03-13 21:26 UTC by Melia Meggs
Modified: 2017-01-10 13:30 UTC (History)
34 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC (4.31 KB, patch)
2012-03-23 14:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord (3.20 KB, patch)
2012-03-23 14:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord (3.20 KB, patch)
2012-03-23 14:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 2 - Switch to using reservenumber (30.69 KB, patch)
2012-03-23 14:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 3 - Add multiple holds per record for intranet (7.93 KB, patch)
2012-03-23 14:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC (4.31 KB, patch)
2012-03-23 14:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord (3.23 KB, patch)
2012-06-10 11:35 UTC, MJ Ray (software.coop)
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord (3.25 KB, patch)
2012-06-10 13:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 2 - Switch to using reserve_id (30.81 KB, patch)
2012-06-10 13:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 3 - Add multiple holds per record for intranet (7.93 KB, patch)
2012-06-10 13:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC (4.32 KB, patch)
2012-06-10 13:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title - Part 5 - Clean up missed 'reservenumber's, convert to reserve_id (1.56 KB, patch)
2012-06-10 13:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (49.01 KB, patch)
2012-07-13 12:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Example - 3 reserves for 1 item (48.96 KB, image/png)
2012-07-17 14:19 UTC, Jonathan Druart
Details
Bug 7710 - multiple holds per title (47.97 KB, patch)
2012-10-01 11:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (49.19 KB, patch)
2012-12-13 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (59.56 KB, patch)
2013-01-15 15:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (15.69 KB, patch)
2013-01-15 15:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (15.89 KB, patch)
2013-01-15 15:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (14.91 KB, patch)
2013-02-11 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (15.37 KB, patch)
2013-02-11 18:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (16.62 KB, patch)
2013-02-13 12:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (19.71 KB, patch)
2013-02-27 18:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (19.83 KB, patch)
2013-02-27 18:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (20.72 KB, patch)
2013-04-15 11:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (14.86 KB, patch)
2013-05-17 18:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Signed off patch with small bug fixed (14.42 KB, patch)
2013-05-22 19:35 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Selenium tests (3.20 KB, application/zip)
2013-05-22 19:40 UTC, Maxime Pelletier
Details
Bug 7710 - multiple holds per title (15.46 KB, patch)
2013-06-18 14:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (15.83 KB, patch)
2014-04-22 17:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (15.88 KB, patch)
2014-07-28 15:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (13.57 KB, patch)
2015-01-20 09:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7710 - multiple holds per title (13.67 KB, patch)
2015-01-26 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Melia Meggs 2012-03-13 21:26:56 UTC
Users shall be able to place holds on multiple items attached to a single biblio, subject to two circulation policies (configurable on the branch/item type/patron category level).  One policy shall indicate the number of holds that can be placed on items with different enumchron values (different volumes), while another shall indicate the number of holds that can be placed on items with the same enumchron field (different copies).  This will allow libraries to allow patrons to place holds on multiple volumes of a title, up to a reasonable limit, while limiting or preventing multiple holds on 'equivalent' copies.
Comment 1 Kyle M Hall 2012-03-23 14:45:19 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-03-23 14:46:12 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-03-23 14:47:21 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-03-23 14:47:45 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2012-03-23 14:47:56 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2012-03-23 14:48:38 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2012-03-23 14:51:20 UTC
Adds multiple holds per title. This does not function quite as what Melia describes. This feature adds a new system preference MaxHoldsPerRecord which defaults to 1 ( so upgrading will not change the default functionality ). This pref allows a borrower to place bib-level and item-level holds up to the number specified. I think added the ability to limit in the ways described by Melia could come later as another development.
Comment 8 MJ Ray (software.coop) 2012-06-10 11:35:57 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2012-06-10 13:13:42 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2012-06-10 13:13:58 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2012-06-10 13:14:13 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2012-06-10 13:14:29 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-06-10 13:14:44 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2012-07-13 08:22:40 UTC
Global symbol "$biblio" requires explicit package name at /home/koha/C4/Reserves.pm line 1056.
Global symbol "$borrower" requires explicit package name at /home/koha/C4/Reserves.pm line 1056.
Global symbol "$biblio" requires explicit package name at /home/koha/C4/Reserves.pm line 1058.
Global symbol "$borrower" requires explicit package name at /home/koha/C4/Reserves.pm line 1058.
Compilation failed in require at /home/koha/C4/Circulation.pm line 27.

And in updatedatabase.pl : 
+    print "Upgrade to $DBversion done (Add syspref to tell Koha if ICU indexing is in use for Zebra or not.)\n";
It think it is not the right message :)

Note: Patch 1 does not apply, but the conflict is easy to fix
Comment 15 Kyle M Hall 2012-07-13 12:58:39 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2012-07-13 13:01:05 UTC
Fixed up a number of issues caused by new development. I've also squashed everything down to one patch for convenience.
Comment 17 Jonathan Druart 2012-07-17 14:18:47 UTC
Hi Kyle,

Is it a limitation per biblio or item ?

In both cases, I managed to place hold the same item for 3 patrons, with MaxHoldsPerRecord = 1

my test plan:
On catalogue/detail.pl > place hold > search and select a patron > choose a specific copy
Comment 18 Jonathan Druart 2012-07-17 14:19:42 UTC
Created attachment 10897 [details]
Example - 3 reserves for 1 item
Comment 19 Kyle M Hall 2012-07-17 17:01:51 UTC
It is per-person, per-biblio. What you are describing is correct behavior.

Kyle

(In reply to comment #17)
> Hi Kyle,
> 
> Is it a limitation per biblio or item ?
> 
> In both cases, I managed to place hold the same item for 3 patrons, with
> MaxHoldsPerRecord = 1
> 
> my test plan:
> On catalogue/detail.pl > place hold > search and select a patron > choose a
> specific copy
Comment 20 Jonathan Druart 2012-07-18 09:41:57 UTC
There is a problem with the new routine GetMultiHoldsCountSameEnum. It returns a query string and the only place where it is used, the return is not used (CanItemBeReserved).

However, when I place hold, I get 2 errors:

Cannot place hold
    Too Many Holds: [USER] has too many holds.
    [USER] has already placed the maximum allowed number of holds on this record


However, my syspref maxreserves is equal to 50 and I should not have the first error.
Comment 21 Paul Poulain 2012-10-01 09:14:31 UTC
ping. I've just been asked by one of our libraries about this feature. Any hope to see it submitted again ?
Comment 22 Kyle M Hall 2012-10-01 11:51:29 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2012-10-01 11:52:57 UTC
> However, when I place hold, I get 2 errors:
> 
> Cannot place hold
>     Too Many Holds: [USER] has too many holds.
>     [USER] has already placed the maximum allowed number of holds on this
> record
> 
> 
> However, my syspref maxreserves is equal to 50 and I should not have the
> first error.

Have you set the "Holds allowed (count)" field in circulation rules? That field can also trigger the same error message.
Comment 24 Jonathan Druart 2012-12-13 11:44:49 UTC
Hi Kyle,

Please correct some qa issues please:

* C4/Reserves.pm                                                           FAIL
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (1309)
		The patch introduces a forbidden pattern: tabulation character (1276)
		The patch introduces a forbidden pattern: tabulation character (1308)
		The patch introduces a forbidden pattern: tabulation character (723)
* opac/opac-modrequest.pl                                                  FAIL
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (43)
* opac/opac-reserve.pl                                                     FAIL
	valid                       FAIL
		Global symbol "$MAXIMUM_NUMBER_OF_RESERVES" requires explicit package name 
		opac/opac-reserve.pl had compilation errors.
* reserve/request.pl                                                       FAIL
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (181)
		The patch introduces a forbidden pattern: tabulation character (241)
		The patch introduces a forbidden pattern: tabulation character (182)
		The patch introduces a forbidden pattern: tabulation character (146)
		The patch introduces a forbidden pattern: tabulation character (160)

The error on opac-reserve.pl is blocker.
Comment 25 Kyle M Hall 2012-12-13 16:03:25 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2012-12-14 10:51:41 UTC
Hi Kyle,
Regarding the code this patch will break a lot af things.
You change the prototype for some routines but you don't change their calls (for example ModReserve, CancelReserve and ModReserveMinusPriority).
Maybe more.
Perhaps the right way to do this is to open another bug for this rewrite.

Marked as Failed QA.
Comment 27 Kyle M Hall 2012-12-17 20:15:52 UTC
(In reply to comment #26)
> Hi Kyle,
> Regarding the code this patch will break a lot af things.
> You change the prototype for some routines but you don't change their calls
> (for example ModReserve, CancelReserve and ModReserveMinusPriority).
> Maybe more.
> Perhaps the right way to do this is to open another bug for this rewrite.
> 
> Marked as Failed QA.

I'm not sure what this new bug would be for. To separate out the switch from bib/patron to reserve_id?
Comment 28 Jonathan Druart 2012-12-18 09:08:18 UTC
(In reply to comment #27)
> I'm not sure what this new bug would be for. To separate out the switch from
> bib/patron to reserve_id?

Yes but it is just a proposal.
With 2 bugs, one is a feature, the other one is a code refactoring.
This way of doing allow not to block the feature. If you provide a big patch with the feature and the code refactoring I am under the impression that the patch will not be pushed in master before several months :)
But the choice is yours!
Comment 29 Kyle M Hall 2013-01-15 15:00:16 UTC Comment hidden (obsolete)
Comment 30 Kyle M Hall 2013-01-15 15:18:48 UTC Comment hidden (obsolete)
Comment 31 Kyle M Hall 2013-01-15 15:19:16 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2013-02-11 18:00:31 UTC Comment hidden (obsolete)
Comment 33 Kyle M Hall 2013-02-11 18:41:47 UTC Comment hidden (obsolete)
Comment 34 David Cook 2013-02-12 23:53:15 UTC
I really like the idea of this patch, but it still has quite a few issues.

1) This patch doesn't apply to the staff client, so library staff are still restricted to 1 hold per patron per record, even with the new system preference set to 3.

2) This patch does apply to the OPAC. You are able to place holds (as the patron) on 3 items in a record when you have the system preference set to 3. However, when you try to place a 4th hold, the error message that appears is:

"Sorry, none of these items can be placed on hold."

"Title / by Author. This title cannot be requested."

That error message doesn't really reflect the actual situation. While I think the first message is accurate, I think the second one should say "You have exceeded the maximum number of holds (3) for this record." or something along those lines.

--

Stylistically, I think the Javascript warning on the staff client when exceeding the maximum number of holds could be improved.

Currently, it is:

"This patron has already placed the maximum number of holds for this record
Please cancel a previous hold first"

These two sentences should have periods at the end and be separated with another new line. 

--

From a purely functional level, it seems like you're nearly there though! I'll be excited to see this one come through :)
Comment 35 Kyle M Hall 2013-02-13 12:28:17 UTC Comment hidden (obsolete)
Comment 36 Kyle M Hall 2013-02-13 12:30:03 UTC
> 1) This patch doesn't apply to the staff client, so library staff are still
> restricted to 1 hold per patron per record, even with the new system
> preference set to 3.

It sounds like you forgot to apply the patch for Bug 9394 first, this bug depends on bug 9394.
 
> 2) This patch does apply to the OPAC. You are able to place holds (as the
> patron) on 3 items in a record when you have the system preference set to 3.
> However, when you try to place a 4th hold, the error message that appears is:
> 
> "Sorry, none of these items can be placed on hold."
> 
> "Title / by Author. This title cannot be requested."

Fixed!

> Stylistically, I think the Javascript warning on the staff client when
> exceeding the maximum number of holds could be improved.
> 
> Currently, it is:
> 
> "This patron has already placed the maximum number of holds for this record
> Please cancel a previous hold first"
> 
> These two sentences should have periods at the end and be separated with
> another new line. 

Also fixed!
Comment 37 David Cook 2013-02-14 01:01:58 UTC
(In reply to comment #36)
> > 1) This patch doesn't apply to the staff client, so library staff are still
> > restricted to 1 hold per patron per record, even with the new system
> > preference set to 3.
> 
> It sounds like you forgot to apply the patch for Bug 9394 first, this bug
> depends on bug 9394.
>  
> > 2) This patch does apply to the OPAC. You are able to place holds (as the
> > patron) on 3 items in a record when you have the system preference set to 3.
> > However, when you try to place a 4th hold, the error message that appears is:
> > 
> > "Sorry, none of these items can be placed on hold."
> > 
> > "Title / by Author. This title cannot be requested."
> 
> Fixed!
> 
> > Stylistically, I think the Javascript warning on the staff client when
> > exceeding the maximum number of holds could be improved.
> > 
> > Currently, it is:
> > 
> > "This patron has already placed the maximum number of holds for this record
> > Please cancel a previous hold first"
> > 
> > These two sentences should have periods at the end and be separated with
> > another new line. 
> 
> Also fixed!

Thanks for the style changes. Much appreciated!

However, there still seem to be some bugs:

Staff Client: It is possible to put 3 holds on the same item.

OPAC: It is possible to put 3 holds on the same item. PLUS, it is impossible to cancel any holds (N.B. you can still cancel holds in the staff client though). I'm not sure if this is a result of 7710 or 9394 though.
Comment 38 David Cook 2013-02-14 01:03:00 UTC
(In reply to comment #36)
> > 1) This patch doesn't apply to the staff client, so library staff are still
> > restricted to 1 hold per patron per record, even with the new system
> > preference set to 3.
> 
> It sounds like you forgot to apply the patch for Bug 9394 first, this bug
> depends on bug 9394.
>  
> > 2) This patch does apply to the OPAC. You are able to place holds (as the
> > patron) on 3 items in a record when you have the system preference set to 3.
> > However, when you try to place a 4th hold, the error message that appears is:
> > 
> > "Sorry, none of these items can be placed on hold."
> > 
> > "Title / by Author. This title cannot be requested."
> 
> Fixed!
> 
> > Stylistically, I think the Javascript warning on the staff client when
> > exceeding the maximum number of holds could be improved.
> > 
> > Currently, it is:
> > 
> > "This patron has already placed the maximum number of holds for this record
> > Please cancel a previous hold first"
> > 
> > These two sentences should have periods at the end and be separated with
> > another new line. 
> 
> Also fixed!

As a side note, it might be helpful to include "first apply Bug 9394" in the test plan :).
Comment 39 Kyle M Hall 2013-02-27 18:22:43 UTC Comment hidden (obsolete)
Comment 40 Kyle M Hall 2013-02-27 18:24:31 UTC Comment hidden (obsolete)
Comment 41 Christophe Croullebois 2013-03-19 15:05:00 UTC
Hi Kyle,
could you rebase please
thx
=>
Applying: Bug 7710 - multiple holds per title
/home/christophe/workspace/versions/community/.git/rebase-apply/patch:49: trailing whitespace.
    
/home/christophe/workspace/versions/community/.git/rebase-apply/patch:51: trailing whitespace.
    
/home/christophe/workspace/versions/community/.git/rebase-apply/patch:54: trailing whitespace.
    
/home/christophe/workspace/versions/community/.git/rebase-apply/patch:74: trailing whitespace.
    
/home/christophe/workspace/versions/community/.git/rebase-apply/patch:76: trailing whitespace.
    
error: patch failed: installer/data/mysql/sysprefs.sql:416
error: installer/data/mysql/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/updatedatabase.pl:6437
error: installer/data/mysql/updatedatabase.pl: patch does not apply
Patch failed at 0001 Bug 7710 - multiple holds per title
Comment 42 Kyle M Hall 2013-04-15 11:57:06 UTC Comment hidden (obsolete)
Comment 43 Frédérick Capovilla 2013-05-14 21:04:54 UTC
Comment on attachment 17448 [details] [review]
Bug 7710 - multiple holds per title

Review of attachment 17448 [details] [review]:
-----------------------------------------------------------------

Some lines in this patch are needed in bug 9394 so it can pass QA as a standalone patch.

::: koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ +564,4 @@
>    [% FOREACH reserveloo IN biblioloo.reserveloop %]
>    [% UNLESS ( loop.odd ) %]<tr class="highlight">[% ELSE %]<tr>[% END %]
>          <td>
> +          <input type="hidden" name="reserve_id" value="[% reserveloo.reserve_id %]" />

Needed in Bug 9394.

@@ +584,4 @@
>  
>       [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
>          <td style="white-space:nowrap;">
> +            <a title="Move Hold Up" href="request.pl?action=move&amp;where=up&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +588,4 @@
>              <img src="/intranet-tmpl/[% theme %]/img/go-up.png" border="0" alt="Go up" />
>                  </a>
>  
> +                <a title="Move Hold To Top" href="request.pl?action=move&amp;where=top&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +592,4 @@
>                      <img src="/intranet-tmpl/[% theme %]/img/go-top.png" border="0" alt="Go top" />
>                  </a>
>  
> +                <a title="Move Hold To Bottom" href="request.pl?action=move&amp;where=bottom&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +596,4 @@
>                      <img src="/intranet-tmpl/[% theme %]/img/go-bottom.png" border="0" alt="Go bottom" />
>                  </a>
>  
> +                <a title="Move Hold Down" href="request.pl?action=move&amp;where=down&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +676,4 @@
>  
>      [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
>  	<td>
> +                <a title="Toggle Lowest Priority" href="request.pl?action=setLowestPriority&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +687,4 @@
>      [% END %]
>  
>  	<td>
> +           <a title="Cancel Hold" href="request.pl?action=cancel&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;reserve_id=[% reserveloo.reserve_id %]&amp;date=[% reserveloo.date %]">

Needed in Bug 9394.

@@ +695,4 @@
>          [% IF SuspendHoldsIntranet %]
>  	<td>
>  	[% UNLESS ( reserveloo.wait ) %]
> +            <input type="button" value="[% IF ( reserveloo.suspend ) %]Unsuspend[% ELSE %]Suspend[% END %]" onclick="window.location.href='request.pl?action=toggleSuspend&amp;reserve_id=[% reserveloo.reserve_id %]&amp;borrowernumber=[% reserveloo.borrowernumber %]&amp;biblionumber=[% reserveloo.biblionumber %]&amp;date=[% reserveloo.date %]&amp;suspend_until=' + $('#suspend_until_[% reserveloo.reserve_id %]').val()" />

Needed in Bug 9394.

@@ +699,3 @@
>  
>              [% IF AutoResumeSuspendedHolds %]
> +        <label for="suspend_until_[% reserveloo.reserve_id %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label>

Needed in Bug 9394.

@@ +699,4 @@
>  
>              [% IF AutoResumeSuspendedHolds %]
> +        <label for="suspend_until_[% reserveloo.reserve_id %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label>
> +            <input name="suspend_until" id="suspend_until_[% reserveloo.reserve_id %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" />

Needed in Bug 9394.

@@ +699,5 @@
>  
>              [% IF AutoResumeSuspendedHolds %]
> +        <label for="suspend_until_[% reserveloo.reserve_id %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label>
> +            <input name="suspend_until" id="suspend_until_[% reserveloo.reserve_id %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" />
> +            <a href='#' onclick="document.getElementById('suspend_until_[% reserveloo.reserve_id %]').value='';">Clear Date</a>

Needed in Bug 9394.
Comment 44 Kyle M Hall 2013-05-17 18:29:06 UTC Comment hidden (obsolete)
Comment 45 Kyle M Hall 2013-05-17 18:30:15 UTC
I've taken the needed lines, moved them to a followup on 9394, and removed them from this patch.
Comment 46 Maxime Pelletier 2013-05-22 19:35:19 UTC Comment hidden (obsolete)
Comment 47 Maxime Pelletier 2013-05-22 19:40:04 UTC
Created attachment 18321 [details]
Selenium tests

As a bonus here are selenium tests that create 3 holds for an user. The tests create a biblio, a testuser and them use them to tests the holds.
Those tests aren't really solid, so you may have to change them a bit if you want to use them. Also, make sure MaxHoldsPerRecord is set to >= 3.
Comment 48 Chris Cormack 2013-06-18 09:28:47 UTC
Applying: Bug 7710 - multiple holds per title
fatal: sha1 information is lacking or useless (C4/Reserves.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7710 - multiple holds per title
Comment 49 Kyle M Hall 2013-06-18 14:25:30 UTC Comment hidden (obsolete)
Comment 50 Jonathan Druart 2013-07-18 08:41:04 UTC
Kyle,
Don't you think it is more consistent to add a new circulation rule instead of create a new syspref (we already have a issuingrules.reservesallowed field)?
Comment 51 Katrin Fischer 2013-08-29 06:11:22 UTC
From Melia's description this would be quite useful, but Kyle stated later that some of it was not implemented as described. Can you clarify?

I think for a global system preference it would not be useful without using the enumchrom field. If it does not depend on it, it would be better in the circulation matrix.
Comment 52 Katrin Fischer 2013-09-22 18:53:04 UTC
Hi Kyle, could you please give feedback? (comment 50 and comment 51)

There are also a couple of conflicts - I was not sure how to resolve the one in reserve.pl.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 7710 - multiple holds per title
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging reserve/request.pl
CONFLICT (content): Merge conflict in reserve/request.pl
Auto-merging opac/opac-reserve.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging C4/Reserves.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 7710 - multiple holds per title
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-7710---multiple-holds-per-title-KL3g2b.patch
Comment 53 Chris Cormack 2013-10-20 18:13:28 UTC
*** Bug 4239 has been marked as a duplicate of this bug. ***
Comment 54 Amber Hunt 2013-10-20 18:47:39 UTC
I think Ian's Wall's comment from a while back applies here:

"The need for some kind of restriction mechanism still exists; not all libraries will want patrons to be able to place multiple holds on a material; this could lead to gaming the system by placing holds on every available copy of a popular book, for example.  I think a control mechanism that looks at the items.enumchron field for distinct values would be a way to go; this would let different vol/issue items, for example, take in multiple holds, but equivalent copies could still be restricted (unless they were explicitly given distinct enumchron values)"

I also wonder if it might be possible to allow staff with the right permissions to override. So, patrons would still not have the ability to do this, but staff (who, in the scenarios I have encountered are the ones that really need this change) could override.
Comment 55 Katrin Fischer 2013-10-20 18:54:00 UTC
I like the enumchron idea, another thing would be moving the switch to the circulation conditions. So you can have different settings for branch/itemtype/patron category. But this would require that the records are identifiable by itemtype (if that makes sense). I am not sure there is a 'perfect' way of doing it and we should really syspref it.
Comment 56 Amber Hunt 2013-10-21 13:48:05 UTC
I agree that we should system preference it. I would like to see one of the override syst pref apply to it or have a new override syst pref that would allow librarians in the staff side to override the syst pref when needed. 

It could be set up however AllowHoldsPoliciyOverride and AllowOnShelfHolds interact.
Comment 57 mshafer@ckls.org 2013-11-05 20:25:24 UTC
I would like to send my encouragement on behalf of my 43 libraries. We would love to be able to have multiples holds on items on a single biblio! It would make book club ordering so much simpler!
Comment 58 David Cook 2013-12-20 00:25:59 UTC
One quick question...

The premise that that this patch will allow multiple holds per record per borrower.

Does that mean that they can place multiple item-level holds or multiple bib-level holds or both?

I assume it would just be multiple item-level holds, but I figured I would ask anyway.
Comment 59 Kyle M Hall 2014-04-22 17:31:36 UTC Comment hidden (obsolete)
Comment 60 Kyle M Hall 2014-04-22 17:32:19 UTC
Rebased the patch, but it's probably in need of a rewrite. I have not tested it.
Comment 61 Christopher Brannon 2014-05-23 15:30:10 UTC
I am unable to test.  When applying patch 9394, I get the following:

The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 9394:
<h1>Something went wrong !</h1>Applying: Bug 9394: Use reserve_id where possible Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Biblio.pm
Auto-merging C4/ILSDI/Services.pm
Auto-merging C4/Reserves.pm
CONFLICT (content): Merge conflict in C4/Reserves.pm Auto-merging C4/SIP/ILS/Transaction/Hold.pm CONFLICT (content): Merge conflict in C4/SIP/ILS/Transaction/Hold.pm Auto-merging circ/circulation.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt
Auto-merging reserve/request.pl
CONFLICT (content): Merge conflict in reserve/request.pl Auto-merging serials/routing-preview.pl Auto-merging t/db_dependent/Holds.t CONFLICT (add/add): Merge conflict in t/db_dependent/Holds.t Failed to merge in the changes.
Patch failed at 0001 Bug 9394: Use reserve_id where possible When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 9394 - Use reserve_id where possible

19902 - Bug 9394: Use reserve_id where possible
19904 - Bug 9394: QA Followup
19905 - Bug 9394: (follow-up) modernize test cases
19906 - Bug 9394: (follow-up) silence spurious warns
19907 - Bug 9394: (follow-up) restore ability to cancel holds from the patron summary
19908 - Bug 9394: (follow-up) fix query column alias
19909 - Bug 9394: (follow-up) stylistic tidying

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-9394-Use-reserveid-where-possible-oBVhgG.patch .

I am using a sandbox, so this is as far as I got in testing.  I will not go any further until I get further instructions.  Does 7710 still depend on 9394?  Should I ignore this error and proceed?

Christopher
Comment 62 Christopher Brannon 2014-05-28 21:09:19 UTC
I get the following errors when trying to load this patch in a sandbox:

The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 7710:
<h1>Something went wrong !</h1>Applying: Bug 7710 - multiple holds per title Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging C4/Reserves.pm
Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt
Auto-merging opac/opac-reserve.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 7710 - multiple holds per title When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 7710 - multiple holds per title

27429 - Bug 7710 - multiple holds per title

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-7710---multiple-holds-per-title-VSuTc4.patch .

Christopher
Comment 63 Kyle M Hall 2014-07-28 15:45:25 UTC
That is correct, multiple bib-level holds, or multiple item-level holds on different items, but not multiple item-level holds on the same item.

(In reply to David Cook from comment #58)
> One quick question...
> 
> The premise that that this patch will allow multiple holds per record per
> borrower.
> 
> Does that mean that they can place multiple item-level holds or multiple
> bib-level holds or both?
> 
> I assume it would just be multiple item-level holds, but I figured I would
> ask anyway.
Comment 64 Kyle M Hall 2014-07-28 15:45:45 UTC Comment hidden (obsolete)
Comment 65 Marcel de Rooy 2014-08-07 13:03:30 UTC
Applying: Bug 7710 - multiple holds per title
fatal: sha1 information is lacking or useless (C4/Reserves.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Another question too:
Why do you remove this line:
-('XSLTResultsDisplay','default','','Enable XSL stylesheet control over results page display on intranet','Free'),
Comment 66 David Cook 2014-08-28 04:32:13 UTC
I've had a library express a desire for this feature, so I'm interested in it once again!

It does seem like a bit of a predicament.

On one hand, if we allow multiple holds per record per borrower, some borrowers could "game the system". On the other hand, if you need all three volumes of a set, it would be frustrating to put the first on hold, and then put the next on hold once you got the first, even though you need all three at once.

I don't know how much we can control borrower behaviour. I suppose we could mitigate it a bit by only allowing multiple holds on multipart records (ie either a "serial" or a "set"). Although that would require perfectly catalogued MARC21 records. You might also have two copies of each 3 volume set, so a person could still game the system, unless you somehow tied the 3 volumes (ie items) together... but that would require extra special weird cataloguing.

I suppose we could handle this behaviour in the circ rules and allow it only for certain item types, but you might occasionally have a different item type with only a few multipart records. Plus, you'd still have the potential for the gaming of the system.

I don't really know what the best answer is. 

The ultimate level of control would be brought about by cataloguing every volume separately, but that would be a pain in terms of discovery and placing holds as well.

But...if we can more about convenience than control...then maybe we have to allow for the possibility of people abusing the system. By using a system preference, we put the choice at the global level. Either this library/consortium allows multiple holds per record per patron, or they don't.

--

tl;dr

Kyle, if you rebase and fix the line that removes the XSLT syspref, I'd be happy to test this.
Comment 67 Russell Ault 2014-09-04 05:37:05 UTC
I, too, would be very interested in testing this patch out. I'm trying to set up Koha for a membership organization with a large lending library that typically will lend 30 or 40 copies of the same score to a choir, and being able to reserve multiple copies of the same item would make library requests a lot simpler.
Comment 68 David Cook 2014-09-05 00:44:19 UTC
(In reply to Russell Ault from comment #67)
> I, too, would be very interested in testing this patch out. I'm trying to
> set up Koha for a membership organization with a large lending library that
> typically will lend 30 or 40 copies of the same score to a choir, and being
> able to reserve multiple copies of the same item would make library requests
> a lot simpler.

Yay Koha in Canada! :D
Comment 69 Sean 2014-09-05 22:56:12 UTC
We would also be interested in testing this patch (consortium of 8 public and 1 academic libraries). Sean@Coos
Comment 70 Olli-Antti Kivilahti 2014-09-17 13:10:57 UTC
Does this still work with 3.16.3?
We have a similar feature and it broke during the upgrade.

Too bad the author never shared his code or actually used this as a basis.
I need to fix our version of this feature, but would like to push any feature bringing the same capabilities to master ASAP.
Comment 71 Chris Cormack 2014-09-17 20:30:03 UTC
(In reply to Olli-Antti Kivilahti from comment #70)
> Does this still work with 3.16.3?
> We have a similar feature and it broke during the upgrade.
> 
> Too bad the author never shared his code or actually used this as a basis.
> I need to fix our version of this feature, but would like to push any
> feature bringing the same capabilities to master ASAP.

AFAICT its waiting on the one little fix mentioned, then it should be ready for testing.
Comment 72 Katrin Fischer 2014-11-06 14:17:17 UTC
Kyle, could you please take a look at comment 66? People seem very interested in testing this :)
Comment 73 Jonathan Druart 2015-01-06 14:35:40 UTC
(In reply to David Cook from comment #66)
> tl;dr
> 
> Kyle, if you rebase and fix the line that removes the XSLT syspref, I'd be
> happy to test this.

Kyle, this patch looks very easy to unblock.

I tried to apply it to submit a follow-up, but the patch does not apply:
  fatal: sha1 information is lacking or useless (C4/Reserves.pm).
Comment 74 Jonathan Druart 2015-01-20 09:44:12 UTC Comment hidden (obsolete)
Comment 75 Jonathan Druart 2015-01-20 09:46:08 UTC
I rebased this patch manually (using git apply --reject) and recreated the patch.
Note that I did *not* test it!
Comment 76 Jonathan Druart 2015-01-20 09:46:33 UTC
(In reply to M. de Rooy from comment #65)
> Another question too:
> Why do you remove this line:
> -('XSLTResultsDisplay','default','','Enable XSL stylesheet control over
> results page display on intranet','Free'),

I reintroduced this line in the last patch.
Comment 77 Marcel de Rooy 2015-01-21 15:41:39 UTC
(In reply to Jonathan Druart from comment #75)
> I rebased this patch manually (using git apply --reject) and recreated the
> patch.
> Note that I did *not* test it!

Thanks for doing so. I would be interested too in seeing this get further.
The current patch suggest that Kyle wrote it and Maxime signed off. Could you add a comment about your rebase; I assume that it was not a trivial one?

Of course, it would be great if you also tested [as second author]. This feature cannot be tested enough in a part of Koha (circulation/holds) that is sensible to bugs.
Comment 78 Jonathan Druart 2015-01-26 10:07:36 UTC
(In reply to M. de Rooy from comment #77)
> Thanks for doing so. I would be interested too in seeing this get further.
> The current patch suggest that Kyle wrote it and Maxime signed off. Could
> you add a comment about your rebase; I assume that it was not a trivial one?

About the manual rebase I did: It was not trivial because the patch did not applied even on 3-way merge.
I used 'git apply --reject' to apply it, caught the errors and manually applied the changes that have not been applied (caused by a big diff on some hunks).
I compared visually the 2 patches and quickly tested to be sure I did not introduced a big issue (compilation error, etc.).
Comment 79 Jonathan Druart 2015-01-26 10:56:35 UTC
Created attachment 35518 [details] [review]
Bug 7710 - multiple holds per title

Adds the ability to allow multiple holds
on the same record for the same borrower.

Adds new system preference MaxHoldsPerRecord,
which controls how many holds one person can
have on items for the same record.

Test Plan:
1) Apply patch for Bug 9394
2) Apply this patch
3) Run updatedatabase.pl
4) Attempt to place 2 holds for the same patron on the same record,
   you should not be able to ( default is 1 per record )
5) Set MaxHoldsPerRecord to 3
6) Attempt to place multiple holds for one patron on one record,
   you should be able to place 3 holds for one patron on a given
   record. A 4th hold should result in a message stating the
   patron has placed the max number of holds allowed on this record.

Signed-off-by: Maxime Pelletier <maxime.pelletier@libeo.com>

Note: Jonathan Druart has manually rebased this patch using git apply
--reject and has not tested it!
Comment 80 Nick Clemens 2015-01-31 18:56:14 UTC
I tested and it seemed to work in terms of allowing patron to place multiple holds next available or specific item and limits were respected for item type.

I placed three holds on one record, 2 item specific for items with different barcodes and 1 next avilable hold

When checking in the first item, however, the next available hold triggered for all holds on the account and all were marked as waiting.  When I checked out the item all holds were cleared from the account

I thought perhaps it was because of dependency on 5144 (marked at top, not in directions) but adding those patches prevented me from placing more than one hold on any title.

Hope that is clear, happy to restest
Comment 81 Katrin Fischer 2015-02-08 12:33:32 UTC
Hi Nick, reading your comment - good catch! Moving to Failed QA for now.
Comment 82 Olli-Antti Kivilahti 2015-05-25 11:42:36 UTC
Is this a duplicate of Bug 12085?
Atleast partially overlapping.
Comment 83 Marcel de Rooy 2015-05-25 12:00:19 UTC
(In reply to Olli-Antti Kivilahti from comment #82)
> Is this a duplicate of Bug 12085?
No
Comment 84 Alex Arnaud 2015-06-16 15:26:35 UTC
Could anyone tell me what we expect exactly ? Do we need to remove all holds of the same record/patron (except 1 which keep waiting) when checking in an item ? 
Do we let them as they were instead of marking them as waiting ?
Comment 85 Katrin Fischer 2015-06-16 15:40:23 UTC
Hm. Trying to think of a use case: serial record with multiple items - each representing a bound volume of issues for a specific year. You need to get 2 articles from 2 different years - so you want to place holds on 2 different items and want both of them to be filled.

For me the case of a record level + item level holds on the same record doesn't make much sense. I can't think of a use case.

Maybe multiple holds should only be allowed item level? A warning if you already have an item level hold, that you need to cancel it first, to set another item level hold?

Looking at the first description of the feature and the current patches I feel like things have drifted apart - especially about the use of the enumchron field (comment#0) - I didn't spot that in the current patches?

How will it be possible to limit the multiple holds on records where it makes sense?

I think having this as a circ rule or being tied to some other condition (enumchron) would be really good to make it more generally useful. As a circulation rule we could combine it with the option to enforce item level holds.
Comment 86 Alex Arnaud 2015-06-17 12:26:28 UTC
I let this bug in discussion.
Librarians/non librarians, feel free to comment/advice .... /help :)
Comment 87 Nick Clemens 2015-06-17 21:23:54 UTC
(In reply to Alex Arnaud from comment #86)
> I let this bug in discussion.
> Librarians/non librarians, feel free to comment/advice .... /help :)

Based on IRC chat with rangi, cait, and liz:

- You shouldn't be able to mix record level / item level holds (tie to OPACItemHolds pref?)
- Staff/Patrons should be able to place multiple item level holds
- Staff should be able to place a record level hold and specify number of copies needed
- Allowing multiple holds should be not be a global syspref, but should be in circ rules (so you can only allow on certain types of items/patron etc.)
Comment 88 Kyle M Hall 2016-01-12 16:21:44 UTC

*** This bug has been marked as a duplicate of bug 14695 ***
Comment 89 Jonathan Druart 2016-06-08 14:57:56 UTC
General message to all people involved in this discussion: A new patch set has been submitted on bug 14695. If you are still waiting for this feature, it would be awesome to see people testing it, even if it is already signoff!
Comment 90 Katrin Fischer 2016-12-27 10:46:51 UTC
Can this now be closed duplicate? (see bug 14695)
Comment 91 Olli-Antti Kivilahti 2017-01-10 11:06:00 UTC
Since the other one went to master, I dont see a point in this bug.
Comment 92 Kyle M Hall 2017-01-10 13:30:36 UTC
(In reply to Olli-Antti Kivilahti from comment #91)
> Since the other one went to master, I dont see a point in this bug.

Yeah, this should have been closed a long time ago.