[Bug 17247] New: ILS-DI HoldTitle and HoldItem should check if patron is debarred
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Bug ID: 17247 Summary: ILS-DI HoldTitle and HoldItem should check if patron is debarred Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Web services Assignee: julian.maurice@biblibre.com Reporter: julian.maurice@biblibre.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 55174 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55174&action=edit Bug 17247: Check if patron is debarred in CanItemBeReserved This way, calls to ILS-DI HoldTitle and HoldItem do this check too -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |hector.hecaxmmx@gmail.com --- Comment #2 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- This patch produce software error on Staff Interfacce To reproduce: Do: http://dev.koha/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=127.0.0.1 This works as expected, but when you go to bib record #1 and click in Hold tab menu at left side of screen you get the error: <http://dev.koha:8080/cgi-bin/koha/reserve/request.pl?biblionumber=1> Software error: Can't call method "is_debarred" on an undefined value at /home/koha/kohaclone/C4/Reserves.pm line 476. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #3 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Yet another error: if you run: $ prove t/db_dependent/Holds.t t/db_dependent/Holds.t .. 40/61 # Looks like you planned 61 tests but ran 62. t/db_dependent/Holds.t .. Dubious, test returned 255 (wstat 65280, 0xff00) All 61 subtests passed Test Summary Report ------------------- t/db_dependent/Holds.t (Wstat: 65280 Tests: 62 Failed: 1) Failed test: 62 Non-zero exit status: 255 Parse errors: Bad plan. You planned 61 tests but ran 62. Files=1, Tests=62, 4 wallclock secs ( 0.04 usr 0.01 sys + 1.81 cusr 0.04 csys = 1.90 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55174|0 |1 is obsolete| | --- Comment #4 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 70684 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70684&action=edit Bug 17247: Check if patron is debarred in CanItemBeReserved This way, calls to ILS-DI HoldTitle and HoldItem do this check too -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- Hi Héctor, I cannot reproduce the error in comment 2, and the tests pass for me. Can you try again ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70684|0 |1 is obsolete| | --- Comment #6 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 72963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72963&action=edit Bug 17247: Check if patron is debarred in CanItemBeReserved This way, calls to ILS-DI HoldTitle and HoldItem do this check too Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Status|Signed Off |BLOCKED CC| |m.de.rooy@rijksmuseum.nl --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- What about moving the debarred check to CanBookBeReserved instead of each time in Item ? Looking thru ILS-DI, it would not be a problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72963|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 75261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75261&action=edit Bug 17247: Check if patron is debarred in CanItemBeReserved This way, calls to ILS-DI HoldTitle and HoldItem do this check too Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 75262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75262&action=edit Bug 17247: (QA follow-up) Add check in CanBookBeReserved too If a patron is debarred, it is useless to do the same check over and over again in CanItemBeReserved. So adding an escape in CanBookBeReserved. Additionally, catching an invalid borrowernumber in Item. Test plan: Run t/db_dependent/Holds.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- if( !$patron || $patron->is_debarred ) { return 'debarred'; } we should not return "debarred" if the borrowernumber passed is wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 75270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75270&action=edit Bug 17247: Do not return "debarred" if borrowernumber is invalid exception must be thrown instead -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- This is not finished and it introduces changes that are not advertised. Placing holds for restricted patron is now impossible: before: https://screenshots.firefox.com/vYnI1v89SSv9DA8M/pro.kohadev.org after: https://screenshots.firefox.com/CLXPhG1xjMNn4yUS/pro.kohadev.org It's not only related to ILS-DI Moreover, the "debarred" string, close to the checkbox, is not translatable. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Victor Grousset/tuxayo <victor.grousset@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor.grousset@biblibre.co | |m See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=17229 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #14 from Victor Grousset/tuxayo <victor.grousset@biblibre.com> --- (In reply to Jonathan Druart from comment #13)
before: https://screenshots.firefox.com/vYnI1v89SSv9DA8M/pro.kohadev.org after: https://screenshots.firefox.com/CLXPhG1xjMNn4yUS/pro.kohadev.org
The screenshots expired (default 14 days) but you can set a expiration to no limit. Or store them in bugzilla. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off CC| |arthur.suzuki@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- https://snag.gy/v5QUZg.jpg https://snag.gy/GQqzoW.jpg -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #16 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 89652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89652&action=edit Bug 17247 : Check if patron is debarred in HoldItem and HoldTitle Test plan : Make sure a debarred patron cannot hold a title or item through ILS-DI. Response from the WebService should be "PatronDebarred". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75261|0 |1 is obsolete| | Attachment #75262|0 |1 is obsolete| | Attachment #75270|0 |1 is obsolete| | Attachment #89652|0 |1 is obsolete| | --- Comment #17 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 89653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89653&action=edit Bug 17247 : Check if patron is debarred in HoldItem and HoldTitle Test plan : Make sure a debarred patron cannot hold a title or item through ILS-DI. Response from the WebService should be "PatronDebarred". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #18 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Hi Jonathan, I moved the checks from C4/Reserve to C4/ILDSI. This way it doesn't affect other behaviours than the ILS-DI WebService. Also the WebService now sends explicit response code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Brendan Gallagher <brendan@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89653|0 |1 is obsolete| | --- Comment #19 from Brendan Gallagher <brendan@bywatersolutions.com> --- Created attachment 93549 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93549&action=edit Bug 17247 : Check if patron is debarred in HoldItem and HoldTitle Test plan : Make sure a debarred patron cannot hold a title or item through ILS-DI. Response from the WebService should be "PatronDebarred". Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|ILS-DI HoldTitle and |ILS-DI HoldTitle and |HoldItem should check if |HoldItem should check if |patron is debarred |patron is restricted CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|m.de.rooy@rijksmuseum.nl |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This is a really small bug fix now, so I am inclined to passing it. It would be nice to have a regression test for this. Also, we should be using "Restricted" instead of Debarred. I'll add a follow-up suggesting the terminology change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 94037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94037&action=edit Bug 17247: Check if patron is restricted in HoldItem and HoldTitle Test plan: Make sure a restricted patron cannot hold a title or item through ILS-DI. Response from the WebService should be "PatronDebarred". Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 94038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94038&action=edit Bug 17247: (follow-up) Terminology fix: Use Restricted instead of debarred -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93549|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #24 from Magnus Enger <magnus@libriotech.no> --- Has this been considered for the 19.05.x branch? I have seen problems caused by this there too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24841 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24841 [Bug 24841] REST API should check if patron is restricted/debarred -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17247 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26463 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org