Bug 22330

Summary: Transfer limits should be respected for placing holds in staff interface and APIs
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Hold requestsAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: critical    
Priority: P5 - low CC: gmcharlt, henridamien.laurent, josef.moravec, koha, martin.renvoize, nick, olli-antti.kivilahti, victor, wizzyrea
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23774
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05
Bug Depends on: 7614    
Bug Blocks: 22160    
Attachments: Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug 22330: Cache item and library objects when building the holds queue
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug 22330: Cache item and library objects when building the holds queue
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug 22330: Cache item and library objects when building the holds queue
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines
Bug 22330: (QA follow-up) Fix QA script failure
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs
Bug 22330: Cache item and library objects when building the holds queue
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines
Bug 22330: (QA follow-up) Fix QA script failure
Bug 22330: (QA follow-up) Enable datatable in patron search
Bug 22330: (QA follow-up) Fix number of tests in Reserves.t

Description Kyle M Hall 2019-02-13 20:33:59 UTC
Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.
Comment 1 Kyle M Hall 2019-02-13 20:35:51 UTC
*** Bug 11020 has been marked as a duplicate of this bug. ***
Comment 2 Kyle M Hall 2019-02-13 20:36:14 UTC
*** Bug 11024 has been marked as a duplicate of this bug. ***
Comment 3 Kyle M Hall 2019-02-13 20:36:52 UTC
*** Bug 7376 has been marked as a duplicate of this bug. ***
Comment 4 Kyle M Hall 2019-02-13 20:46:02 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2019-02-13 20:46:12 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2019-02-14 11:27:14 UTC
Setting severity to critical since this replaces 7376 which had that severity.
Comment 7 Josef Moravec 2019-02-18 10:51:48 UTC
Comment on attachment 85067 [details] [review]
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs

Review of attachment 85067 [details] [review]:
-----------------------------------------------------------------

::: C4/Reserves.pm
@@ +34,3 @@
>  use C4::Letters;
>  use C4::Log;
> +use C4::Members qw();

C4::Members is just few lines above with full import...

@@ +48,4 @@
>  use Koha::ItemTypes;
> +use Koha::Items;
> +use Koha::Libraries;
> +use Koha::Libraries;

The same use line twice.
Comment 8 Kyle M Hall 2019-03-11 18:27:11 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2019-03-11 18:37:12 UTC
Created attachment 86480 [details] [review]
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs

Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.

Test Plan:
1) Set up a branch transfer limit from Library A to Library B
2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding )
3) Verify you cannot place that hold via ILS-DI
4) Verify you cannot place that hold via SIP
4) Verify a forced hold from Library A to Library B will not show up in the holds queue
Comment 10 Kyle M Hall 2019-03-11 18:37:21 UTC
Created attachment 86481 [details] [review]
Bug 22330: Cache item and library objects when building the holds queue
Comment 11 Kyle M Hall 2019-03-11 18:37:23 UTC
Created attachment 86482 [details] [review]
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines
Comment 12 Liz Rea 2019-03-19 18:32:57 UTC
Created attachment 86758 [details] [review]
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs

Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.

Test Plan:
1) Set up a branch transfer limit from Library A to Library B
2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding )
3) Verify you cannot place that hold via ILS-DI
4) Verify you cannot place that hold via SIP
4) Verify a forced hold from Library A to Library B will not show up in the holds queue

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 13 Liz Rea 2019-03-19 18:33:00 UTC
Created attachment 86759 [details] [review]
Bug 22330: Cache item and library objects when building the holds queue

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 14 Liz Rea 2019-03-19 18:33:03 UTC
Created attachment 86760 [details] [review]
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 15 Liz Rea 2019-03-19 18:46:50 UTC
I realised half a second too late that there was a QA tool fail here, but I'm not 100% sure it's not a false positive, and no one was around to ask. Anyway, here's what it was: 

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
   FAIL	  filters
		missing_filter at line 666 (        var biblionumber = "[% biblionumber %]";)
		missing_filter at line 667 (        var borrowernumber = "[% patron.borrowernumber %]";)
Comment 16 Kyle M Hall 2019-03-20 11:08:28 UTC
Created attachment 86779 [details] [review]
Bug 22330: (QA follow-up) Fix QA script failure
Comment 17 Josef Moravec 2019-03-20 14:37:37 UTC
Created attachment 86802 [details] [review]
Bug 22330: Transfer limits should be respected for placing holds in staff interface and APIs

Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.

Test Plan:
1) Set up a branch transfer limit from Library A to Library B
2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding )
3) Verify you cannot place that hold via ILS-DI
4) Verify you cannot place that hold via SIP
4) Verify a forced hold from Library A to Library B will not show up in the holds queue

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 18 Josef Moravec 2019-03-20 14:37:40 UTC
Created attachment 86803 [details] [review]
Bug 22330: Cache item and library objects when building the holds queue

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Josef Moravec 2019-03-20 14:37:44 UTC
Created attachment 86804 [details] [review]
Bug 22330: (QA follow-up) Remove duplicate use lines, combine and sort remaning lines

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 20 Josef Moravec 2019-03-20 14:37:47 UTC
Created attachment 86805 [details] [review]
Bug 22330: (QA follow-up) Fix QA script failure

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 21 Josef Moravec 2019-03-20 14:37:51 UTC
Created attachment 86806 [details] [review]
Bug 22330: (QA follow-up) Enable datatable in patron search

Test plan:
Try to search patron for circulation (or for creating hold), when there
are more than 1 patron, you should see a table with patrons found.
--> without this patch, the datatable is not created (for example
        sorting the table by clicking to column heading does not work)
--> with this patch, the datatable works

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Josef Moravec 2019-03-20 14:37:54 UTC
Created attachment 86807 [details] [review]
Bug 22330: (QA follow-up) Fix number of tests in Reserves.t

Test plan:
prove t/db_dependent/Reserves.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 23 Josef Moravec 2019-03-20 14:43:03 UTC
I removed schema files changes accidentally added to third patch and added two small fixes
- number of test, that is obvious
- activate again the commented out javascript for creating datatable when searching for patron to circulate/create hold
Comment 24 Nick Clemens 2019-03-21 18:32:39 UTC
Awesome work all!

Pushed to master for 19.05
Comment 25 Martin Renvoize 2019-03-25 14:27:52 UTC
This is proving non-trivial to backport. Kyle, do you feel this should make the cut to 18.11?
Comment 26 Victor Grousset/tuxayo 2019-03-27 10:25:19 UTC
Can someone confirm if this bug encompasses bug 7376? (Transfer limits should be checked at check-in)
Because this bug seems to only be about holds.
Comment 27 Martin Renvoize 2019-04-25 13:04:04 UTC
Non-trivial backport for a long standing issue. I'm taking the decision not to backport at the present time.