http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 26589 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26589&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved - Check if borrower already has reserved the same biblio (or item). - Check if borrower has reached the maximum number of holds allowed (syspref maxreserves) This patch makes CanBookBeReserved and CanItemBeReserved to return a another variable, a hashref that describes why it cannot be reserved. It changes all calls to these functions and removes code in Perl scripts that is not needed anymore. The goal of this patch is to do these checks also when using ILS-DI services HoldTitle and HoldItem Test plan: ---------- Before patch: 1/ Set syspref maxreserves to 1 2/ Place some holds through ILS-DI and note that you can place more than 1 hold for a borrower 3/ Try to hold the same biblio/item with the same borrower through ILS-DI and note that you can Apply the patch 4/ Place some holds through ILS-DI, you shouldn't be able to place more than 1 hold for a borrower 5/ Try to hold the same biblio/item with the same borrower through ILS-DI. You shouldn't be able to do that. 6/ Try to place holds on staff interface and OPAC. The behaviour must be identical than before the patch. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal --- Comment #2 from Julian Maurice <julian.maurice@biblibre.com> --- Increasing severity from enh to normal, as the patch fixes a bug in ILS-DI -- You are receiving this mail because: You are watching all bug changes.

The change in interface that passes back reasons is lost, when you don't include a variable for it. Also, it makes this look like a pointless variable. I suppose it's just hard to read and not really incorrect. I don't understand. CanBookBeReserved now returns a list so the return value is affected to a list. Why does the variable look pointless ? I just don't need

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #4 from Julian Maurice <julian.maurice@biblibre.com> --- the $reasons here, so I don't retrieve it.

Why is there a C4::Reserves::CanBookBeReserved call here, but a CanBookBeReserved in the earlier code? I don't know, but it was like that before the patch, so I didn't change this.

-- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26589|0 |1 is obsolete| | --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 28986 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=28986&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@quecheelibrary.org --- Comment #6 from Nick Clemens <nick@quecheelibrary.org> --- Pardon if I am confused, but CanBooksBeReserved can return three different reasons [alreadyreserved, maxreseres, noholdableitems] But the check in request.pl checks for alreadyreserved and assumes maxreserves otherwise my $alreadyreserved = 0; 190 $maxreserves = 1; 191 my ($can_book_be_reserved, $reasons) = 192 CanBookBeReserved( $borrowerinfo->{borrowernumber}, $biblionumber ); 193 194 unless ($can_book_be_reserved) { 195 if ($reasons->{alreadyreserved}) { 196 $alreadyreserved = 1; 197 $biblioloopiter{warn} = 1; 198 $biblioloopiter{alreadyres} = 1; 199 } else { 200 $maxreserves = 1; 201 } This seems to be the same issue as Bug 10912 To my mind, maxreserves shouldn't be flagged unless the patron is over their number of reserves. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31524|0 |1 is obsolete| | --- Comment #8 from Nick Clemens <nick@quecheelibrary.org> --- Comment on attachment 31524 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31524 Payment Thanks value entered into accountlines.description Wrong bug# sorry -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |olli-antti.kivilahti@jns.fi --- Comment #9 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi! It looks like this duplicates Bug 13116. Could you take a look at Comment 11 of Bug 13116 and give me some ideas? Also, could you change your title to something more descriptive? Like "Propagating error messages out from CanBookBeReserved"? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #10 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi there! Apparently Bug 13116 is already signed off. Can you refactor your feature to supplement mine? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #28986|0 |1 is obsolete| | --- Comment #12 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 35697 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35697&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved - Check if borrower already has reserved the same biblio (or item). - Check if borrower has reached the maximum number of holds allowed (syspref maxreserves) The goal of this patch is to do these checks also when using ILS-DI services HoldTitle and HoldItem Test plan: ---------- Before patch: 1/ Set syspref maxreserves to 1 2/ Place some holds through ILS-DI and note that you can place more than 1 hold for a borrower 3/ Try to hold the same biblio/item with the same borrower through ILS-DI and note that you can Apply the patch 4/ Place some holds through ILS-DI, you shouldn't be able to place more than 1 hold for a borrower 5/ Try to hold the same biblio/item with the same borrower through ILS-DI. You shouldn't be able to do that. 6/ Try to place holds on staff interface and OPAC. The behaviour must be identical than before the patch. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #14 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Julian Maurice from comment #13)

Patch rebased on master (and consequently on top of bug 13116)

There is one unit test that fails in t/db_dependent/Holds.t (line 331), but it fails on master too. Should I open a separate bug for this ?

Current master e5b834a1c474f761c9f9c8c01dd6abeb99ee5eac does not fail this test with me. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Alex Sassmannshausen <alex.sassmannshausen@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.sassmannshausen@ptfs-e | |urope.com --- Comment #16 from Alex Sassmannshausen <alex.sassmannshausen@ptfs-europe.com> --- Hi Julian, I've just tried to sign this one off using a sandbox, but there was a problem applying the patch. I think it may need to be rebased again? I'll try again when you've had a look. Cheers, Alex -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35697|0 |1 is obsolete| | --- Comment #17 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 36358 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36358&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Alex Sassmannshausen <alex.sassmannshausen@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- Alex, to change the patch's author, you can use git commit --amend --author "Author Name <authoremail@example.org>" -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Alex Sassmannshausen from comment #20)

(In reply to Jonathan Druart from comment #19)

...

Alex, to change the patch's author, you can use git commit --amend --author "Author Name <authoremail@example.org>"

Sorry Jonathan, I'm afraid I'm confused: do you need me to change the name of the contributor on the patch?

Actually it could be done by the QAer, just wanted to share with you the trick if you didn't know. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|alex.sassmannshausen@ptfs-e |julian.maurice@biblibre.com |urope.com | CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #24 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 38397 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38397&action=edit Bug 11999: Check for age restriction in CanBookBeReserved + improve t/db_dependent/Reserves.t to make tests pass on UNIMARC installation -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #26 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Julian Maurice from comment #25)

This patch should make the "age restriction" test pass

This may totally be tangential and unrelated, but I vaguely recall hitting an age restriction problem in some test file, and it turned out to be a timezone mismatch problem somewhere. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36630|0 |1 is obsolete| | --- Comment #27 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 41951 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41951&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #29 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Katrin Fischer from comment #23)

This seems to break one of the existing tests, please check:

not ok 52 - Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails # Failed test 'Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails' # at t/db_dependent/Reserves.t line 498. # got: 'none_available' # expected: 'ageRestricted' ok 53 - Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds # Looks like you failed 1 test of 53.

Please also include a short explanation in your new tests:

ok 35 - cannot request item if policy that matches on bib-level item type forbids it (bug 9532) ok 36 ok 37 ok 38 ok 39 - Waiting reserve beyond max pickup delay *not* canceled on holiday

You ran it at the magical failure hour for your timezone. See bug 14362. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|z.tajoli@cineca.it | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41951|0 |1 is obsolete| | --- Comment #31 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 44631 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44631&action=edit Bug 11999: Add two checks in CanBookBeReserved and CanItemBeReserved Rebased on master -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44632|0 |1 is obsolete| | --- Comment #34 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46263 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46263&action=edit Bug 11999: Check for age restriction in CanBookBeReserved + improve t/db_dependent/Reserves.t to make tests pass on UNIMARC installation Maxreserves and alreadyreserved works on ILSDI Maxreserves works also on staff and opac Already reserves works also on staff and opac tests on t/db_dependent/Reserves.t and t/db_dependent/Holds.t passe (using koha_ut db) Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 --- Comment #35 from Alex Arnaud <alex.arnaud@biblibre.com> --- *** Bug 14155 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=15404 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14695 -- You are receiving this mail because: You are watching all bug changes.