Bug 29015

Summary: Add option to limit Holds Queue report by shelving location / collection
Product: Koha Reporter: Nick Clemens <nick>
Component: Hold requestsAssignee: Nick Clemens <nick>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kyle
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30411
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on:    
Bug Blocks: 28966, 29389    
Attachments: Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Unit tests
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Unit tests
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Unit tests
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Add options for itemtype, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Unit tests
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: (QA follow-up) Change collection code to be just collection
Bug 29015: Add options for itemtype, collection, and shelving location to view_holdsqueue.pl
Bug 29015: Unit tests
Bug 29015: (follow-up) Show sidebar filter form with results
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Bug 29015: (QA follow-up) Change collection code to be just collection
Bug 29015: Unit tests

Description Nick Clemens 2021-09-14 13:33:17 UTC
In large consortia it can take a long time to load the whole holds queue - additionally staff may only work on a section at a time.

We should add options for limiting to a specific location/collection
Comment 1 Nick Clemens 2021-09-14 19:33:25 UTC
Created attachment 124871 [details] [review]
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"
Comment 2 Nick Clemens 2021-09-15 10:34:35 UTC
Created attachment 124881 [details] [review]
Bug 29015: Unit tests
Comment 3 Owen Leonard 2021-09-15 12:27:32 UTC
Created attachment 124883 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.
Comment 4 Owen Leonard 2021-09-15 12:30:03 UTC
I think the original patches work well, and I hope my follow-up makes it even better. I do have one request because I think as it is this patch changes a behavior that libraries depend on:

The form when it is initially displayed should pre-select the logged-in user's home library.
Comment 5 Nick Clemens 2021-09-15 17:17:54 UTC
Created attachment 124903 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue
Comment 6 Owen Leonard 2021-09-15 18:15:22 UTC
Created attachment 124907 [details] [review]
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Owen Leonard 2021-09-15 18:15:26 UTC
Created attachment 124908 [details] [review]
Bug 29015: Unit tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Owen Leonard 2021-09-15 18:15:30 UTC
Created attachment 124909 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.
Comment 9 Owen Leonard 2021-09-15 18:15:33 UTC
Created attachment 124910 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Nick Clemens 2021-09-16 09:50:36 UTC
Created attachment 124915 [details] [review]
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Nick Clemens 2021-09-16 09:50:39 UTC
Created attachment 124916 [details] [review]
Bug 29015: Unit tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 12 Nick Clemens 2021-09-16 09:50:43 UTC
Created attachment 124917 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens 2021-09-16 09:50:47 UTC
Created attachment 124918 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Kyle M Hall 2021-09-28 16:12:48 UTC
Created attachment 125419 [details] [review]
Bug 29015: Add options for itemtpye, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 15 Kyle M Hall 2021-09-28 16:13:46 UTC
Created attachment 125420 [details] [review]
Bug 29015: Add options for itemtype, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 16 Kyle M Hall 2021-09-28 16:13:58 UTC
Created attachment 125421 [details] [review]
Bug 29015: Unit tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 17 Kyle M Hall 2021-09-28 16:14:02 UTC
Created attachment 125422 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 18 Kyle M Hall 2021-09-28 16:14:06 UTC
Created attachment 125423 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 19 Katrin Fischer 2021-10-21 20:27:24 UTC
The tests are failing for me, can you please check?

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/HoldsQueue.t
t/db_dependent/HoldsQueue.t .. 56/58 Use of uninitialized value in hash element at /home/vagrant/kohaclone/C4/HoldsQueue.pm line 466.
    # No tests run!

#   Failed test 'No tests run for subtest "Remove holds on check-in match"'
#   at t/db_dependent/HoldsQueue.t line 1739.
Can't call method "branchcode" on an undefined value at /home/vagrant/kohaclone/Koha/Item.pm line 664.
# Looks like your test exited with 255 just after 57.
t/db_dependent/HoldsQueue.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 2/58 subtests 

Test Summary Report
-------------------
t/db_dependent/HoldsQueue.t (Wstat: 65280 Tests: 57 Failed: 1)
  Failed test:  57
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 58 tests but ran 57.
Comment 20 Nick Clemens 2021-10-27 13:19:37 UTC
I don't recreate failure, it looks like you have a hold with no pickup location on your system
Comment 21 Katrin Fischer 2021-11-01 20:50:34 UTC
Testing this again and all seems to work as advertised, but there are some things that keep irritating me (unrelated to this patch, so not blocker):

* If I have a record level hold and I have 2 items for the record, that are identical apart from collection, how is the collection that shows on holds queue chosen? Only one shows and I can only filter on the one that shows...

* We can limit on library, so then I know which library I am looking at. But... when I choose "All", there seems to be no clue in the table about the library that owns the item and is supposed to provide it?
Comment 22 Katrin Fischer 2021-11-01 21:02:11 UTC
Created attachment 127191 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2021-11-01 21:02:16 UTC
Created attachment 127192 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2021-11-01 21:02:20 UTC
Created attachment 127193 [details] [review]
Bug 29015: (QA follow-up) Change collection code to be just collection

We did some clean-up in that area a little while ago, agreeing that
as we acutally display the descriptions, the label should be just
"collection".

This patch also removes some ():

Before:
4 items found for Centerville and item type:(Books) and collection: (Reference) and shelving location: (General Stacks)

Now:
4 items found for Centerville and item type: Books and collection: Reference and shelving location: General Stacks

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2021-11-01 21:04:49 UTC
Created attachment 127194 [details] [review]
Bug 29015: Add options for itemtype, collection, and shelving location to view_holdsqueue.pl

This patch makes the code for itemtypeslimit work, and adds options for shelving location and collection code

This also remove the 'post' method from the form to allow easy bookmarking

To test:
1 - Add holds to your system
2 - Run the holds queue builder
3 - Browse to Circulation->Holds queue
4 - Note the library dropdown
5 - Apply patch
6 - Reload and note new options
7 - Test that both limits and 'All' options work as expected
8 - Note that description at top includes options when selected
    "### items found for All libraries and item type:(Books)"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2021-11-01 21:04:52 UTC
Created attachment 127195 [details] [review]
Bug 29015: Unit tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2021-11-01 21:04:57 UTC
Created attachment 127196 [details] [review]
Bug 29015: (follow-up) Show sidebar filter form with results

This patch adds the new filter form in the sidebar when showing results.
To do so the form markup is moved into a BLOCK so that it can be reused.

This patch also adds pre-selection of submitted options on the filter
form so that it reflects the user's choices.

To test, apply the patch and rebuild the staff client CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

- Go to Circulation -> Holds queue.
- Submit the form with one or more filters selected.
- On the results page the queue should be correctly filtered and the
  sidebar should show your search fields preselected.
- Test with the CircSidebar preference enabled and disabled. The
  circulation menu should appear correctly, with the filter form only
  displaying in the sidebar after submitting the form.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2021-11-01 21:05:01 UTC
Created attachment 127197 [details] [review]
Bug 29015: (follow-up) Default to logged in branch unless viewing queue

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2021-11-01 21:05:05 UTC
Created attachment 127198 [details] [review]
Bug 29015: (QA follow-up) Change collection code to be just collection

We did some clean-up in that area a little while ago, agreeing that
as we acutally display the descriptions, the label should be just
"collection".

This patch also removes some ():

Before:
4 items found for Centerville and item type:(Books) and collection: (Reference) and shelving location: (General Stacks)

Now:
4 items found for Centerville and item type: Books and collection: Reference and shelving location: General Stacks

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 30 Nick Clemens 2021-11-02 12:04:23 UTC
(In reply to Katrin Fischer from comment #21)
> Testing this again and all seems to work as advertised, but there are some
> things that keep irritating me (unrelated to this patch, so not blocker):
> 
> * If I have a record level hold and I have 2 items for the record, that are
> identical apart from collection, how is the collection that shows on holds
> queue chosen? Only one shows and I can only filter on the one that shows...

Yes, the holds queue chooses a single item to fill the hold, so you see the one chosen. The script uses a lot of logic to determine which is the 'best' item, and we choose only one so that only one library attempts to fill the hold

> * We can limit on library, so then I know which library I am looking at.
> But... when I choose "All", there seems to be no clue in the table about the
> library that owns the item and is supposed to provide it?

Bug 29389 filed
Comment 31 Jonathan Druart 2021-11-03 13:52:17 UTC
Created attachment 127261 [details] [review]
Bug 29015: Unit tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD Amended patch: perltidy!
Comment 32 Jonathan Druart 2021-11-03 14:31:52 UTC
Pushed to master for 21.11, thanks to everybody involved!