[Bug 14155] New: Placing holds via ILSDI bypass some checks
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Bug ID: 14155 Summary: Placing holds via ILSDI bypass some checks Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: alex.arnaud@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com If you make a hold request using ILSDI webservices (HoldItem or HoldTitle) 2 important checks are skipped while they are done using staff user interface. 1) maxreserves system preference is ignored so you can place as many hold as issuingrules allow you even if maxreserves is set. 2) nothing checks if the borrower already reserved the biblio. Patch is coming. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |alex.arnaud@biblibre.com |ity.org | CC| |alex.arnaud@biblibre.com Status|NEW |Needs Signoff --- Comment #1 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 38884 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38884&action=edit Add maxreserves and alreadyreserved checks when placing hold via ILSDI -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 --- Comment #2 from Indranil Das Gupta <indradg@gmail.com> --- Comment on attachment 38884 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38884 Add maxreserves and alreadyreserved checks when placing hold via ILSDI Review of attachment 38884: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14155&attachment=38884) ----------------------------------------------------------------- Alex, I think the following : ::: C4/Reserves.pm @@ +1117,5 @@
+ return 1 if $borrowernumber eq $res->{borrowernumber}; + } + + return 0; +}
needs added test in t/db_dependent/Reserves.t @@ +1139,5 @@
+ return 1; + } + + return 0; +}
needs added test in t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED CC| |indradg@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 --- Comment #3 from Indranil Das Gupta <indradg@gmail.com> --- (In reply to Indranil Das Gupta from comment #2)
Comment on attachment 38884 [details] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI
Review of attachment 38884 [details] [review]: -----------------------------------------------------------------
Alex,
I think the following :
::: C4/Reserves.pm
C4::Reserves::AlreadyReserved() C4::Reserves::ReachedMaxReserves() needs added test in t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 --- Comment #4 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 40170 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40170&action=edit [Follow-up] Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 --- Comment #5 from Alex Arnaud <alex.arnaud@biblibre.com> --- (In reply to Indranil Das Gupta from comment #3)
(In reply to Indranil Das Gupta from comment #2)
Comment on attachment 38884 [details] [review] [review] Add maxreserves and alreadyreserved checks when placing hold via ILSDI
Review of attachment 38884 [details] [review] [review]: -----------------------------------------------------------------
Alex,
I think the following :
::: C4/Reserves.pm
C4::Reserves::AlreadyReserved() C4::Reserves::ReachedMaxReserves()
needs added test in t/db_dependent/Reserves.t Hello,
Thank for having had a look at this bug. Just added tests for ttthis 2 sub? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40170|0 |1 is obsolete| | --- Comment #6 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40174 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40174&action=edit Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38884|0 |1 is obsolete| | --- Comment #7 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40175 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40175&action=edit Add maxreserves and alreadyreserved checks when placing hold via ILSDI http://bugs.koha-community.org/show_bug.cgi?id=14155 Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40174|0 |1 is obsolete| | --- Comment #8 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40176 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40176&action=edit Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.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=14155 --- Comment #9 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40208 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40208&action=edit (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 --- Comment #10 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40210 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40210&action=edit (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #11 from Indranil Das Gupta <indradg@gmail.com> --- Setting this back to "Needs Signoff" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40210|0 |1 is obsolete| | --- Comment #12 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 40239 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40239&action=edit (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <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=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40208|0 |1 is obsolete| | --- Comment #13 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 40240 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40240&action=edit (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); Signed-off-by: Alex Arnaud (Biblibre) <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=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40239|0 |1 is obsolete| | --- Comment #14 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 40241 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40241&action=edit (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <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=14155 Alex Arnaud <alex.arnaud@biblibre.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=14155 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14526 CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Be care, AddReserve prototype changed by bug 14526 (contrainst param removed). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|http://bugs.koha-community. |http://bugs.koha-community. |org/bugzilla3/show_bug.cgi? |org/bugzilla3/show_bug.cgi? |id=14526 |id=9809 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I meant bug 9809. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have some problems with the conflict in the test files - can you please take a look? If I just fix the number of tests, I get some output like this: bumblebee:~/kohaclone (14155-ilsdi|AM 1/1) $ perl t/db_dependent/Reserves.t 1..71 ok 1 - use C4::Reserves; "my" variable $title masks earlier declaration in same scope at t/db_dependent/Reserves.t line 85. "my" variable $title masks earlier declaration in same scope at t/db_dependent/Reserves.t line 98. "my" variable $bibitemnum masks earlier declaration in same scope at t/db_dependent/Reserves.t line 98. ... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Tom Misilo <misilot@fit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misilot@fit.edu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40175|0 |1 is obsolete| | --- Comment #18 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 44047 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44047&action=edit Add maxreserves and alreadyreserved checks when placing hold via ILSDI http://bugs.koha-community.org/show_bug.cgi?id=14155 Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40176|0 |1 is obsolete| | --- Comment #19 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 44048 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44048&action=edit Bug 14155 - Add tests in t/db_dependent/Reserves.t - add tests for C4::Reserves::AlreadyReserved(), - add tests for C4::Reserves::ReachedMaxReserves(), Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40240|0 |1 is obsolete| | --- Comment #20 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 44049 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44049&action=edit (fix typo) AddBiblio call in Reserves.t Fix for the typo, it read : my ($broken_id, $title, $bibitemnum) = AddBiblio($miniaturist, ''); it should read : my ($broken_id, $title, $bibitemnum) = AddBiblio($broken, ''); Signed-off-by: Alex Arnaud (Biblibre) <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=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40241|0 |1 is obsolete| | --- Comment #21 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 44050 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44050&action=edit (qa followup) variable declaration and cleanup 1/ running koha qa tools showed up valid FAIL. This patch fix that error. FAIL t/db_dependent/Reserves.t OK critic OK forbidden patterns OK pod FAIL valid "my" variable $title masks earlier declaration in same scope "my" variable $bibitemnum masks earlier declaration in same scope 2/ the following redundant line was removed as it had no use: t::lib::Mocks::mock_preference('maxreserves', 15); Signed-off-by: Alex Arnaud (Biblibre) <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=14155 --- Comment #22 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 44064 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44064&action=edit Bug 14155 - Update tests and fix failures -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@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=14155 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz --- Comment #23 from Liz Rea <liz@catalyst.net.nz> --- Hi, I'm happy to have a guess at how to test this, please in the future may we have a test plan? It makes things go a lot faster. :) Thanks very much, Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14155 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Needs Signoff |RESOLVED --- Comment #24 from Alex Arnaud <alex.arnaud@biblibre.com> --- *** This bug has been marked as a duplicate of bug 11999 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org