[Bug 13116] New: Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Bug ID: 13116 Summary: Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com A reservation can be blocked for various reasons, but C4::Reserves::CanItemBeReserved() only either fails or not, regardless of the reason. This enhancement makes it possible to more easily show different error messages to web-templates. -- 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=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |13113 -- 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=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |olli-antti.kivilahti@jns.fi |ity.org | -- 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=13116 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32544 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32544&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #2 from Marc Véron <veron@veron.ch> --- Should status be "Needs Signoff"? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@enger.priv.no --- Comment #3 from Magnus Enger <magnus@enger.priv.no> --- No test plan? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32544|0 |1 is obsolete| | --- Comment #4 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32550 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32550&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32550|0 |1 is obsolete| | --- Comment #5 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32551 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32551&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) OPAC A. In OPAC go to opac-detail.pl for the ageRestricted Biblio. The "Place Hold" -button is missing on the right sidebar if the underage user is logged in. B. From the opac-search.pl a hold can still be placed, but it will fail giving an error about age restrictions. ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32551|0 |1 is obsolete| | --- Comment #6 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32552 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32552&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) OPAC A. In OPAC go to opac-detail.pl for the ageRestricted Biblio. The "Place Hold" -button is missing on the right sidebar if the underage user is logged in. B. From the opac-search.pl a hold can still be placed, but it will fail giving an error about age restrictions. ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #7 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Hi there early birds! I didn't set "Needs Signoff" because I didn't think the patch was ready for testing. Just wanted to share my work at the days end to see where the feature set is moving to. Thanks for caring though! There are still bugs there, but setting the status to "Needs Signoff" for your leisure. It's all your now :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32552|0 |1 is obsolete| | --- Comment #8 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32627 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32627&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. ((-- MULTIPLE HOLDS STAFF CLIENT --)) 3. Observe the error notification "Cannot place hold on some items" ((-- MULTIPLE HOLDS OPAC --)) 1. Make a search with many results, of which atleast one is age restricted to the current borrower. 2. Select few results and "Place hold" from to result summary header element. (Not individual results "Place hold") 3. Observe individual Biblios getting the "age restricted"-notification, where others can be reserved just fine. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32627|0 |1 is obsolete| | --- Comment #9 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32662 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32662&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. ((-- MULTIPLE HOLDS STAFF CLIENT --)) 3. Observe the error notification "Cannot place hold on some items" ((-- MULTIPLE HOLDS OPAC --)) 1. Make a search with many results, of which atleast one is age restricted to the current borrower. 2. Select few results and "Place hold" from to result summary header element. (Not individual results "Place hold") 3. Observe individual Biblios getting the "age restricted"-notification, where others can be reserved just fine. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- Does this duplicate some or all of Bug 11999? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 --- Comment #11 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- (In reply to Owen Leonard from comment #10)
Does this duplicate some or all of Bug 11999?
Yes it does. My feature propagates the error codes from CanItemBeReserved through CanBookBeReserved resulting in more error handling options in the templates. I went through the Bug 11999 and it is up to preference which one should make it. I can easily refactor my feature chain to support Bug 11999, but I insist that all error handling should be centralized to the CanBook/ItemBeReserved instead of getting scattered all around scripts and modules. This means CanBookBeReserved should somehow propagate the errors collected at CanItemBeReserved(). I think Bug 11999 has the right base for it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11999 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32662|0 |1 is obsolete| | --- Comment #12 from Marc Véron <veron@veron.ch> --- Created attachment 32920 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32920&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. ((-- MULTIPLE HOLDS STAFF CLIENT --)) 3. Observe the error notification "Cannot place hold on some items" ((-- MULTIPLE HOLDS OPAC --)) 1. Make a search with many results, of which atleast one is age restricted to the current borrower. 2. Select few results and "Place hold" from to result summary header element. (Not individual results "Place hold") 3. Observe individual Biblios getting the "age restricted"-notification, where others can be reserved just fine. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t Followed test plan. Works as expected and displays meaningful messages for the reason why placing a hold is not possible. Signed-off-by: Marc Véron <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=13116 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Rafal Kopaczka <rkopaczka@afm.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rkopaczka@afm.edu.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32920|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 33075 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33075&action=edit Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. ((-- MULTIPLE HOLDS STAFF CLIENT --)) 3. Observe the error notification "Cannot place hold on some items" ((-- MULTIPLE HOLDS OPAC --)) 1. Make a search with many results, of which atleast one is age restricted to the current borrower. 2. Select few results and "Place hold" from to result summary header element. (Not individual results "Place hold") 3. Observe individual Biblios getting the "age restricted"-notification, where others can be reserved just fine. Updated the unit tests to match the new method return values. t/db_dependent/Holds.t & Reserves.t Followed test plan. Works as expected and displays meaningful messages for the reason why placing a hold is not possible. Signed-off-by: Marc Veron <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=13116 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 33076 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33076&action=edit Bug 13116 [QA Followup] - Remove tabs, use unless instead of if -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33076|0 |1 is obsolete| | --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 33077 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33077&action=edit Bug 13116 [QA Followup] - Remove tabs, use unless instead of if -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13116 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Olli! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org