http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Nicole C. Engard <nengard@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED CC| |nengard@gmail.com Resolution| |DUPLICATE --- Comment #1 from Nicole C. Engard <nengard@gmail.com> 2011-10-05 16:43:44 UTC --- *** This bug has been marked as a duplicate of bug 4045 *** -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 --- Comment #3 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-10-13 15:51:53 UTC --- The person who was going to confirm the fix works in production was away. They are now back and I'll submit the patch on confirmation -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |PATCH-Sent Patch Status|--- |Needs Signoff --- Comment #5 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-11-21 17:48:13 UTC --- Patch as deployed and tested on site. Reopening this bug as 4045 covers some other issues as well -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 --- Comment #7 from Liz Rea <wizzyrea@gmail.com> 2011-11-21 18:49:00 UTC --- that should read, I was able to circumvent the limits. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |4045 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Melia Meggs <melia@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |melia@bywatersolutions.com Patch Status|Needs Signoff |Failed QA --- Comment #10 from Melia Meggs <melia@bywatersolutions.com> 2011-12-08 23:56:35 UTC --- With maxreserves sys pref set to allow 100 holds and circ rules set to allow 5 holds (as in Liz's example), I was also able to place 6 holds. So I have changed the status to "Failed QA." -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 --- Comment #12 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-12-11 14:03:54 UTC --- Katrin's comment above is valid as this script has always checked the circ matrix limits via the C4::reserves routines but this does not restrict the user from placing the hold. The patch only addresses the users ability to circumvent the maxreserves setting -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #6371|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall <kyle.m.hall@gmail.com> 2012-02-23 14:36:11 UTC --- Created attachment 7832 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=7832 Bug 6976 - User hold limit can be avoided by users in OPAC Adds a check to see if a multi-hold request would possibly exceed the maximum number of allowed holds. If it is possible, then we display an error message if the borrower has more checkboxes checked than new holds that can be placed. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 --- Comment #14 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Kyle, As far as I can tell, your patch just a reimplementation of the patch that Melia marked as failed QA for only addressing maxreserves. Am I missing something? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Francois Charbonnier <francois.charbonnier@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |francois.charbonnier@biblib | |re.com --- Comment #16 from Francois Charbonnier <francois.charbonnier@biblibre.com> --- I'm not sure I have been able to reproduce this bug. I didn't know the set up so, here are the set ups I used to test this : The count of "Holds allowed" within the circulation matrix (Library / Category code / Item type) is : All / All / All = 3 All / Librarian / Book = 3 Library A / All / All = 3 Library A / Librarian / Book = 3 I worked with 4 system preferences : * maxreserves * item-level_itypes * OPACItemHolds * ReservesControlBranch The maxreserves syspref is set up to : maxreserves = 4 Then I tried every set up possible with the three other sysprefs. I worked with 4 differents items that have : biblioitems.itemtype = Book items.itype = Book The patron I used to place hold is a Librarian from Library A. When I tested the set ups with OPACItemHolds = yes, every holds placed were on the item level. If OPACItemHolds was set up to "no", every holds placed were on the biblio level. What I noticed is that : * if koha knows the patron category code, the patron or the item library and the itemtype, the limit used is the one from the circulation matrix _BUT_ I have the wrong message : "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." * if koha doesn't know the item's library ou the item's item type, I can't place more than 4 holds. So the limit comes from the maxreserves syspref. It's logical but I think, we should use the "default" rule from the circulation matrix instead to be more flexible. The maxreserves syspref should be used only when nothing else apply. Or maybe, we could get rid of the maxreserves syspref, saying that the default rule (Library =all / Category code = all / Item type = all) is the one koha should use when nothing apply. Since, the documentation says this rule is required, I think it would logical to give it a use. Set ups and test results : 1/ item-level_itypes = biblio OPACItemHolds = no ReservesControlBranch = patron's branch -> it looks like it works but I have the wrong message "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." 2/ item-level_itypes = item OPACItemHolds = no ReservesControlBranch = patron's branch -> The control looks like it comes from "maxreserves". 3/ item-level_itypes = item OPACItemHolds = yes ReservesControlBranch = patron's branch -> it looks like it works but I have the wrong message "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." 4/ item-level_itypes = biblio OPACItemHolds = no ReservesControlBranch = item's branch -> The control looks like it comes from "maxreserves". 5/ item-level_itypes = biblio OPACItemHolds = yes ReservesControlBranch = item's branch -> it looks like it works but I have the wrong message "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." 6/ item-level_itypes = item OPACItemHolds = yes ReservesControlBranch = item's branch -> it looks like it works but I have the wrong message "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." 7/ item-level_itypes = biblio OPACItemHolds = yes ReservesControlBranch = patron's branch -> it looks like it works but I have the wrong message "Sorry, none of these items can be placed on hold". I should have "Sorry, you cannot place more than X holds." 8/ item-level_itypes = item OPACItemHolds = no ReservesControlBranch = item's branch -> The control looks like it comes from "maxreserves". -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #17 from Liz Rea <wizzyrea@gmail.com> --- I want to see both patches get anot -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |joy@bywatersolutions.com --- Comment #20 from Joy Nelson <joy@bywatersolutions.com> --- Marking as Does Not Apply Auto-merging opac/opac-reserve.pl CONFLICT (content): Merge conflict in opac/opac-reserve.pl Failed to merge in the changes. Patch failed at 0001 Bug 6976 Close loophole allowing borrowers extra holds -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #22 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Have updated original patch against current master. Tested by staff at Staffordshire University Incidentally in testing found that if the patch is not in place it only took a couple of days for students to start using the loophole to abuse the system placing greater than the maxholds value -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #7832|0 |1 is obsolete| | Attachment #11874|0 |1 is obsolete| | --- Comment #24 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 11876 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11876&action=edit Bug 6976 Close loophole allowing borrowers extra holds via opac The check that the max number of reserves has not been reached needs to take account of the fact that we may have added enouch reserves to reach the limit while this script is running. Add a check against the incrementing count The variable canreserve was only used when looping through multiple holds but was not being reinitialized on every iteration make variable local to loop it is used in and initialize to zero Have used perltidy to correct the indention of the while loop as it was misleading to the reader Some numeric comparisons were using a string operator fix them before strange bugs manifest themselves The loophole manifests thus: borrowers are only allowed Y holds. If holds are done individually, it is obeying that law i.e. X has held Y separate books – when I tried to do a 6th book it told me that I could not place a hold. However, if a borrower checks the catalogue, gets the intial list up and clicks items on the “results” box and then clicks “place hold” they can hold more than Y items Please enter the commit message for your changes. Lines starting Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |mtj@kohaaloha.com QA Contact|koha-bugs@lists.koha-commun |mtj@kohaaloha.com |ity.org | --- Comment #26 from Mason James <mtj@kohaaloha.com> ---

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

patch looks good, passing QA... $ koha-qa.pl testing 1 commit(s) (applied to commit 60790d0) * 3ebd669 Bug 6976 Close loophole allowing borrowers extra holds via opac opac/opac-reserve.pl * opac/opac-reserve.pl OK -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #28 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.8.x, will be in 3.8.5 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6976 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.8 |unspecified CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Still valid. -- You are receiving this mail because: You are watching all bug changes.