|
Description
Alex Arnaud
2018-01-10 09:59:57 UTC
Created attachment 70409 [details] [review] Bug 19945 - ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Created attachment 73351 [details] [review] Bug 19945 - ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 73352 [details] [review] Bug 19945: Follow up optimization and test correction The double call to CanItemBeReserved was pointless in the C4/Reserves.pm HoldItem, as CanBookBeReserved calls CanItemBeReserved and even returns that code. Additionally, the tests failed, because there were 5 tests not 4. TEST PLAN ---------- prove t/db_dependent/ILSDI_Services.t run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 75688 [details] [review] Bug 19945: ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 75689 [details] [review] Bug 19945: (follow-up) optimization and test correction The double call to CanItemBeReserved was pointless in the C4/Reserves.pm HoldItem, as CanBookBeReserved calls CanItemBeReserved and even returns that code. Additionally, the tests failed, because there were 5 tests not 4. TEST PLAN ---------- prove t/db_dependent/ILSDI_Services.t run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> I don't think the second patch is quite right - CanBookBeReserved calls CanItemBeReserved, but it might not be correct for a specific item Set a rule for 'No Holds Allowed' for an itemtype Find a bib with no items or create on Add an item with the disallowed item type Attempt hold on item via ILSDI - Not allowed Add a second item with a holdable itemtype Attempt hold on the original unholdable item via ILSDI - Allowed? I think the call in 'Hold Item' to CanBookBeReserved is the extraneous one unless I missed something I also note that neither of the routines mentioned check for 'On shelf holds allowed' policy - filed as bug 20985 (In reply to Nick Clemens from comment #6) > I don't think the second patch is quite right - CanBookBeReserved calls > CanItemBeReserved, but it might not be correct for a specific item > > Set a rule for 'No Holds Allowed' for an itemtype > Find a bib with no items or create on > Add an item with the disallowed item type > Attempt hold on item via ILSDI - Not allowed > Add a second item with a holdable itemtype > Attempt hold on the original unholdable item via ILSDI - Allowed? > > I think the call in 'Hold Item' to CanBookBeReserved is the extraneous one > unless I missed something > > I also note that neither of the routines mentioned check for 'On shelf holds > allowed' policy - filed as bug 20985 I agree. For further, it seems to me that HoldItem should call CanItemBeReserved only, even though it may not be the point of this bug. Created attachment 79046 [details] [review] Bug 19945: ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 79047 [details] [review] Bug 19945: (follow-up) optimization and test correction The double call to CanItemBeReserved was pointless in the C4/Reserves.pm HoldItem, as CanBookBeReserved calls CanItemBeReserved and even returns that code. Additionally, the tests failed, because there were 5 tests not 4. TEST PLAN ---------- prove t/db_dependent/ILSDI_Services.t run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 79048 [details] [review] Bug 19945: Add a test proving that using CanBookBeReserved in ILSDI::HoldItem is wrong. Created attachment 79049 [details] [review] Bug 19945: Use CanItemBeReserved in ILSDI HoldItem instead of CanBookBeReserved. Hello, I've rebased the first two patches. Also i provide a third one that prove what Nick said is right and a fourth one that fix it. Created attachment 81371 [details] [review] Bug 21434: (follow-up) Switch two-column templates to Bootstrap grid: Tools part 2 This follow-up corrects CSS which was dependent on YUI-specific markup context. To test, apply the patch, regenerate the staff client CSS, go to Tools -> Calendar. - Click any date on the calendar - In the entry form the checkbox and radio button options should be styled correctly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 81372 [details] [review] Bug 21435: Update two-column templates with Bootstrap grid: Tools part 3 This patch modifies several tools templates to use the Bootstrap grid instead of YUI. This patch also removes obsolete "text/javascript" attributes from <script> tags and "text/css" attributes from <style> tags in the modified templates. To test, apply the patch and view the following pages, confirming that they look correct at various browser widths: - Tools -> MARC modification templates - View and edit templates - Tools -> Batch patron modification - Test each step of the process - Tools -> Overdue notice/status triggers - Tools -> Upload patron images - Test each step of the upload process - Tools -> Quote editor -> Quote uploader - Import quotes Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 81373 [details] [review] Bug 18591: Add missing schema file Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 81374 [details] [review] Bug 18591: Fix pluralization of type Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Michal Denar <black23@gmail.com> When I try to add title level hold on biblio without items, I get this error: Can't use an undefined value as a HASH reference at /home/vagrant/kohaclone/C4/ILSDI/Services.pm line 670 (In reply to Josef Moravec from comment #17) > When I try to add title level hold on biblio without items, I get this error: > > Can't use an undefined value as a HASH reference at > /home/vagrant/kohaclone/C4/ILSDI/Services.pm line 670 Got the same error on master. This should be fixed in an other bug imo. I've created Bug 21738. Turn this back to Signed off Created attachment 83745 [details] [review] Bug 19945: ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 83746 [details] [review] Bug 19945: (follow-up) optimization and test correction The double call to CanItemBeReserved was pointless in the C4/Reserves.pm HoldItem, as CanBookBeReserved calls CanItemBeReserved and even returns that code. Additionally, the tests failed, because there were 5 tests not 4. TEST PLAN ---------- prove t/db_dependent/ILSDI_Services.t run koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 83747 [details] [review] Bug 19945: Add a test proving that using CanBookBeReserved in ILSDI::HoldItem is wrong. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 83748 [details] [review] Bug 19945: Use CanItemBeReserved in ILSDI HoldItem instead of CanBookBeReserved. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Awesome work all! Pushed to master for 19.05 As this is an old and very stable API and this could be perceived as a breaking change I am not going to backport this one to 18.11.x series at this time.. please speak up if you disagree and would like it as I may be persuadable under my caveat of backporting API improvements. |