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.
Created attachment 8598 [details] [review] Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC
Created attachment 8599 [details] [review] Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord
Created attachment 8600 [details] [review] Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord
Created attachment 8601 [details] [review] Bug 7710 - multiple holds per title - Part 2 - Switch to using reservenumber Where necessary, switch from using biblionumber/borrowernumber to using reservenumber to reference individual reserves.
Created attachment 8602 [details] [review] Bug 7710 - multiple holds per title - Part 3 - Add multiple holds per record for intranet
Created attachment 8603 [details] [review] Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC
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.
Created attachment 10091 [details] [review] Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord
Created attachment 10098 [details] [review] Bug 7710 - multiple holds per title - Part 1 - Add new system preference MaxHoldsPerRecord
Created attachment 10099 [details] [review] Bug 7710 - multiple holds per title - Part 2 - Switch to using reserve_id Where necessary, switch from using biblionumber/borrowernumber to using reserve_id to reference individual reserves.
Created attachment 10100 [details] [review] Bug 7710 - multiple holds per title - Part 3 - Add multiple holds per record for intranet
Created attachment 10101 [details] [review] Bug 7710 - multiple holds per title - Part 4 - Add multiple holds per record for the OPAC
Created attachment 10102 [details] [review] Bug 7710 - multiple holds per title - Part 5 - Clean up missed 'reservenumber's, convert to reserve_id
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
Created attachment 10818 [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.
Fixed up a number of issues caused by new development. I've also squashed everything down to one patch for convenience.
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
Created attachment 10897 [details] Example - 3 reserves for 1 item
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
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.
ping. I've just been asked by one of our libraries about this feature. Any hope to see it submitted again ?
Created attachment 12620 [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.
> 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.
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.
Created attachment 14083 [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.
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.
(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?
(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!
Created attachment 14600 [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.
Created attachment 14602 [details] [review] Bug 7710 - multiple holds per title
Created attachment 14603 [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.
Created attachment 15252 [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.
Created attachment 15253 [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 2) Run updatedatabase.pl 3) Attempt to place 2 holds for the same patron on the same record, you should not be able to ( default is 1 per record ) 4) Set MaxHoldsPerRecord to 3 5) 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.
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 :)
Created attachment 15295 [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 2) Run updatedatabase.pl 3) Attempt to place 2 holds for the same patron on the same record, you should not be able to ( default is 1 per record ) 4) Set MaxHoldsPerRecord to 3 5) 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.
> 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!
(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.
(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 :).
Created attachment 15731 [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 1) Apply this patch 2) Run updatedatabase.pl 3) Attempt to place 2 holds for the same patron on the same record, you should not be able to ( default is 1 per record ) 4) Set MaxHoldsPerRecord to 3 5) 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.
Created attachment 15732 [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 1) Apply this patch 2) Run updatedatabase.pl 3) Attempt to place 2 holds for the same patron on the same record, you should not be able to ( default is 1 per record ) 4) Set MaxHoldsPerRecord to 3 5) 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.
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
Created attachment 17448 [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 1) Apply this patch 2) Run updatedatabase.pl 3) Attempt to place 2 holds for the same patron on the same record, you should not be able to ( default is 1 per record ) 4) Set MaxHoldsPerRecord to 3 5) 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.
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&where=up&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&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&where=top&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&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&where=bottom&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&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&where=down&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&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&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> Needed in Bug 9394. @@ +687,4 @@ > [% END %] > > <td> > + <a title="Cancel Hold" href="request.pl?action=cancel&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&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&reserve_id=[% reserveloo.reserve_id %]&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]&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.
Created attachment 18217 [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.
I've taken the needed lines, moved them to a followup on 9394, and removed them from this patch.
Created attachment 18320 [details] [review] Signed off patch with small bug fixed
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.
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
Created attachment 19117 [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>
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)?
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.
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
*** Bug 4239 has been marked as a duplicate of this bug. ***
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.
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.
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.
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!
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.
Created attachment 27429 [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>
Rebased the patch, but it's probably in need of a rewrite. I have not tested it.
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
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
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.
Created attachment 30218 [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>
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'),
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.
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.
(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
We would also be interested in testing this patch (consortium of 8 public and 1 academic libraries). Sean@Coos
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.
(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.
Kyle, could you please take a look at comment 66? People seem very interested in testing this :)
(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).
Created attachment 35397 [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>
I rebased this patch manually (using git apply --reject) and recreated the patch. Note that I did *not* test it!
(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.
(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.
(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.).
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!
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
Hi Nick, reading your comment - good catch! Moving to Failed QA for now.
Is this a duplicate of Bug 12085? Atleast partially overlapping.
(In reply to Olli-Antti Kivilahti from comment #82) > Is this a duplicate of Bug 12085? No
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 ?
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.
I let this bug in discussion. Librarians/non librarians, feel free to comment/advice .... /help :)
(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.)
*** This bug has been marked as a duplicate of bug 14695 ***
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!
Can this now be closed duplicate? (see bug 14695)
Since the other one went to master, I dont see a point in this bug.
(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.