Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype
Summary: Add ability to split view of holds view on record by pickup library and/or it...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks: 21611
  Show dependency treegraph
 
Reported: 2017-10-15 13:18 UTC by Kyle M Hall
Modified: 2019-05-30 11:52 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This enhancement adds the ability to visually separate the holds list on a record by library and/or itemtype. This can make it a bit clearer who is in line next for a hold at each branch or if user has specified a specific type. The system preference HoldsSplitQueue and HoldsSplitQueueNumbering control this behaviour. When activated holds can only be adjusted using the arrows, the dropdown priority menu is disabled.
Version(s) released in:


Attachments
Bug 19469 - Clean up template whitespace (26.15 KB, patch)
2017-10-15 13:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype (40.65 KB, patch)
2017-10-15 13:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 - Clean up template whitespace (26.15 KB, patch)
2017-10-15 13:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype (41.42 KB, patch)
2017-10-15 13:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 - Clean up template whitespace (26.22 KB, patch)
2017-10-21 06:44 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype (41.55 KB, patch)
2017-10-21 06:46 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype (41.71 KB, patch)
2017-10-26 14:06 UTC, Andreas Hedström Mace
Details | Diff | Splinter Review
Bug 19469 - Clean up template whitespace (26.01 KB, patch)
2018-03-01 10:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype (41.70 KB, patch)
2018-03-01 10:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Clean up template whitespace (26.07 KB, patch)
2018-03-05 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype (41.81 KB, patch)
2018-03-05 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403 (1.39 KB, patch)
2018-03-05 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19469: Remove use of Dumper on request.tt (714 bytes, patch)
2018-03-07 12:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: [DO NOT PUSH] Dump of the reserves table (5.20 KB, patch)
2018-04-16 19:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19469: Clean up template whitespace (26.09 KB, patch)
2018-04-17 14:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Clean up template whitespace (26.09 KB, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype (41.85 KB, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403 (1.41 KB, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Remove use of Dumper on request.tt (753 bytes, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: [DO NOT PUSH] Dump of the reserves table (5.21 KB, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 [QA Followup]: Use hold item's itemtype if available, fix priority changing (4.26 KB, patch)
2018-04-17 14:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 [QA Followup]: When altering priorities, use the existing priority numbers of the given table (1.86 KB, patch)
2018-04-27 09:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Clean up template whitespace (26.62 KB, patch)
2018-09-07 11:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Clean up template whitespace (26.62 KB, patch)
2018-09-07 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype (127.04 KB, patch)
2018-09-07 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403 (1.39 KB, patch)
2018-09-07 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Remove use of Dumper on request.tt (752 bytes, patch)
2018-09-07 11:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: [DO NOT PUSH] Dump of the reserves table (5.20 KB, patch)
2018-09-07 11:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 [QA Followup]: Use hold item's itemtype if available, fix priority changing (4.22 KB, patch)
2018-09-07 11:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469 [QA Followup]: When altering priorities, use the existing priority numbers of the given table (1.72 KB, patch)
2018-09-07 11:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: (RM follow-up) Add filters and fix variable name (24.99 KB, patch)
2018-09-07 14:55 UTC, Nick Clemens
Details | Diff | Splinter Review
(RM follow-up) Add filters and fix variable name (24.25 KB, patch)
2018-09-07 15:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19469: (QA followup) Disable priority pulldowns when using (1.55 KB, patch)
2018-09-14 19:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19469: Fix record links on multi holds (3.00 KB, patch)
2018-11-01 15:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19469: Fix record links on multi holds (1.18 KB, patch)
2018-11-01 15:58 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2017-10-15 13:18:46 UTC

    
Comment 1 Kyle M Hall 2017-10-15 13:26:26 UTC
It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype. To make it easier to understand which holds will be trapped in a given circumstance, it would be nice if we could optionally group holds by pickup library and/or itemtype.
Comment 2 Kyle M Hall 2017-10-15 13:29:00 UTC
Created attachment 68166 [details] [review]
Bug 19469 - Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 3 Kyle M Hall 2017-10-15 13:29:07 UTC
Created attachment 68167 [details] [review]
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2017-10-15 13:35:38 UTC
Created attachment 68168 [details] [review]
Bug 19469 - Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2017-10-15 13:35:45 UTC
Created attachment 68169 [details] [review]
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Alex Buckley 2017-10-21 06:44:58 UTC
Created attachment 68347 [details] [review]
Bug 19469 - Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 7 Alex Buckley 2017-10-21 06:46:40 UTC
Created attachment 68348 [details] [review]
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 8 Andreas Hedström Mace 2017-10-26 14:06:07 UTC
Created attachment 68609 [details] [review]
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Comment 9 Kyle M Hall 2018-03-01 10:54:44 UTC
Created attachment 72304 [details] [review]
Bug 19469 - Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 10 Kyle M Hall 2018-03-01 10:54:52 UTC
Created attachment 72305 [details] [review]
Bug 19469 - Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Comment 11 Tomás Cohen Arazi 2018-03-05 19:25:15 UTC
Created attachment 72441 [details] [review]
Bug 19469: Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 12 Tomás Cohen Arazi 2018-03-05 19:25:27 UTC
Created attachment 72442 [details] [review]
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 13 Tomás Cohen Arazi 2018-03-05 19:25:34 UTC
Created attachment 72443 [details] [review]
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403

This patch makes the new template include file use the patron-title.inc
include to render the patron name.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 14 Josef Moravec 2018-03-06 11:45:57 UTC
Comment on attachment 72442 [details] [review]
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ +1,1 @@
> +[% USE Dumper %]

debug?
Comment 15 Kyle M Hall 2018-03-07 12:04:49 UTC
(In reply to Josef Moravec from comment #14)
> Comment on attachment 72442 [details] [review] [review]
> Bug 19469: Add ability to split view of holds view on record by pickup
> library and/or itemtype
> 
> Review of attachment 72442 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
> @@ +1,1 @@
> > +[% USE Dumper %]
> 
> debug?

Yep! Followup coming!
Comment 16 Kyle M Hall 2018-03-07 12:05:23 UTC
Created attachment 72501 [details] [review]
Bug 19469: Remove use of Dumper on request.tt
Comment 17 Jonathan Druart 2018-04-16 19:09:49 UTC
I think I found few inconsistencies.

Try with HoldsSplitQueue = both and HoldsSplitQueueNumbering = actual.

From this starting point:
https://screenshots.firefox.com/TX15YNJl7t9SF3LD/pro.kohadev.org

1. Last lines of the 2 blocks have the "lowest priority" icon, I would expect only the first one (with priority=9)

2. item-level_itypes=item and all of the items.itype=BK, why do I have the item level holds in the "Any item type" block?

3. If I change one of the priority from x to 9 using the selects, nothing happens (same issue if I change 9 to x).


I will continue to test when this will be fixed.
It is certainly better for the readability to squash the patches.
Comment 18 Jonathan Druart 2018-04-16 19:12:14 UTC
Created attachment 74260 [details] [review]
Bug 19469: [DO NOT PUSH] Dump of the reserves table
Comment 19 Jonathan Druart 2018-04-16 19:12:44 UTC
(In reply to Jonathan Druart from comment #18)
> Created attachment 74260 [details] [review] [review]
> Bug 19469: [DO NOT PUSH] Dump of the reserves table

Here is the content of my table "reserves" (on top of sample data).
Comment 20 Kyle M Hall 2018-04-17 14:30:45 UTC
Created attachment 74328 [details] [review]
Bug 19469: Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 21 Kyle M Hall 2018-04-17 14:31:06 UTC
Created attachment 74329 [details] [review]
Bug 19469: Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 22 Kyle M Hall 2018-04-17 14:31:13 UTC
Created attachment 74330 [details] [review]
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 23 Kyle M Hall 2018-04-17 14:31:16 UTC
Created attachment 74331 [details] [review]
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403

This patch makes the new template include file use the patron-title.inc
include to render the patron name.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 24 Kyle M Hall 2018-04-17 14:31:19 UTC
Created attachment 74332 [details] [review]
Bug 19469: Remove use of Dumper on request.tt
Comment 25 Kyle M Hall 2018-04-17 14:31:21 UTC
Created attachment 74333 [details] [review]
Bug 19469: [DO NOT PUSH] Dump of the reserves table
Comment 26 Kyle M Hall 2018-04-17 14:31:24 UTC
Created attachment 74334 [details] [review]
Bug 19469 [QA Followup]: Use hold item's itemtype if available, fix priority changing
Comment 27 Kyle M Hall 2018-04-17 14:36:13 UTC
Thanks for the table data! It made fixing this up much easier.

> 1. Last lines of the 2 blocks have the "lowest priority" icon, I would
> expect only the first one (with priority=9)

This is good and correct behavior. Any number of holds can be set to "lowest priority" at once.
 
> 2. item-level_itypes=item and all of the items.itype=BK, why do I have the
> item level holds in the "Any item type" block?

Good idea! My new followup takes care of this.

> 3. If I change one of the priority from x to 9 using the selects, nothing
> happens (same issue if I change 9 to x).

I've updated the javascript to work with the multiple tables. I decided to let it re-flow the priorities in the affected tables, but we could also simply leave the priorities as they are if we have split holds views. Either solution seem good and correct in its own way. I chose to re-flow to keep consistency with the current behavior.

> I will continue to test when this will be fixed.
> It is certainly better for the readability to squash the patches.

If you would like to squash all the patches and re-upload them as one please do so! If you'd like me to do it instead just let me know!
Comment 28 Jonathan Druart 2018-04-24 16:09:16 UTC
I wanted to retest but stopped quickly because I either do not understand how it is supposed to work or it is buggy:

I am using the same reserves.sql data, with  HoldsSplitQueue=pickup, HoldsSplitQueueNumbering=actual
On the reserve page, I see that: https://screenshots.firefox.com/DDTe7dEmVOK4regX/pro.kohadev.org

First line of Franklin, I select 9, and I get: https://screenshots.firefox.com/r8FS1f2Q3SFw9Ngg/pro.kohadev.org (without resubmitting the form, just selecting a value in the dropdown list).


Moreover I am wondering why we have the column with "go lowest" and "go bottom" (before the red X to cancel). Is not it redundant with the other 4 arrows column?
Same behaviour in master, just wondering if we should not remove this column on a separate bug report.
Comment 29 Jonathan Druart 2018-04-24 16:18:47 UTC
There is something else I think:
If you change the order with the select, do not save but click on one of the arrows, it will not do what you expect (i.e. save what you saw just before saving).
Comment 30 Kyle M Hall 2018-04-24 16:47:23 UTC
> Moreover I am wondering why we have the column with "go lowest" and "go
> bottom" (before the red X to cancel). Is not it redundant with the other 4
> arrows column?
> Same behaviour in master, just wondering if we should not remove this column
> on a separate bug report.

The 'lowest priority' is a separate feature and is not the same as simply moving the hold to the bottom of the queue. It 'pins' the hold to the bottom so any new holds added are automatically put in front of those holds in the queue. We should not remove this column as it is a feature and is not redundant.
Comment 31 Jonathan Druart 2018-04-25 13:51:20 UTC
I need either follow-ups, second signoff or another QA.
Moving out of PQA, back to "Signed Off"
Comment 32 Andreas Hedström Mace 2018-04-26 21:10:20 UTC
(In reply to Jonathan Druart from comment #28)
> I am using the same reserves.sql data, with  HoldsSplitQueue=pickup,
> HoldsSplitQueueNumbering=actual
> On the reserve page, I see that:
> https://screenshots.firefox.com/DDTe7dEmVOK4regX/pro.kohadev.org
> 
> First line of Franklin, I select 9, and I get:
> https://screenshots.firefox.com/r8FS1f2Q3SFw9Ngg/pro.kohadev.org (without
> resubmitting the form, just selecting a value in the dropdown list).


I'm a bit confused by these two. So you are using the same setting for the second screenshot, but when editing the first hold of Franklin, instead of showing the actual numbers it reverts to showing separate queue priorities? That does sound like a unwanted behavior, unless I'm missing something...
Comment 33 Kyle M Hall 2018-04-27 09:22:08 UTC
Created attachment 74909 [details] [review]
Bug 19469 [QA Followup]: When altering priorities, use the existing priority numbers of the given table
Comment 34 Kyle M Hall 2018-04-27 09:22:26 UTC
(In reply to Jonathan Druart from comment #31)
> I need either follow-ups, second signoff or another QA.
> Moving out of PQA, back to "Signed Off"

Followup posted!
Comment 35 Jonathan Druart 2018-05-03 17:23:54 UTC
Either it still does not work or it is very confusing (or I do not understand how it is supposed to work).

As we are in feature freeze I am not willing to push this one for 18.05 and would recommend another QA stamp.
Comment 36 Martin Renvoize 2018-08-09 12:26:39 UTC
I've added his one to my pile to do a second round of qa on.. see if I can verify or otherwise Jonathans concerns.
Comment 37 Nick Clemens 2018-08-25 14:54:42 UTC
Kyle can you rebase this for SCSS and bug 13618?
Comment 38 Kyle M Hall 2018-09-07 11:37:59 UTC
Created attachment 78506 [details] [review]
Bug 19469: Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 39 Kyle M Hall 2018-09-07 11:38:35 UTC
Created attachment 78507 [details] [review]
Bug 19469: Clean up template whitespace

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 40 Kyle M Hall 2018-09-07 11:38:46 UTC
Created attachment 78508 [details] [review]
Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype

It is possible to set up circulation rules to limit trapping of holds by pickup library and itemtype.
To make it easier to understand which holds will be trapped in a given circumstance,
it would be nice if we could optionally group holds by pickup library and/or itemtype.

Test Plan:
1) Apply this patch set
2) Run updatedatabase.pl
3) Enable AllowHoldItemTypeSelection
4) Pick a record and create holds with various pickup libraries and itemtype combinations
5) Enable HoldsSplitQueueNumbering
6) Try the different combinations of HoldsSplitQueue
7) Ensure the hold "arrows" move the items correctly
   * Up and down arrows should move hold above or below the adjacent hold within a hold fieldset
   * Top and borrom arrows should move hold to the top or bottom within a hold fieldset

Sponsored-by: Stockholm University Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Followed test plan, patch worked as described. Also passed QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 41 Kyle M Hall 2018-09-07 11:38:52 UTC
Created attachment 78509 [details] [review]
Bug 19469: (followup) Use patron-title.inc as proposed by bug 18403

This patch makes the new template include file use the patron-title.inc
include to render the patron name.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 42 Kyle M Hall 2018-09-07 11:38:56 UTC
Created attachment 78510 [details] [review]
Bug 19469: Remove use of Dumper on request.tt
Comment 43 Kyle M Hall 2018-09-07 11:39:00 UTC
Created attachment 78511 [details] [review]
Bug 19469: [DO NOT PUSH] Dump of the reserves table
Comment 44 Kyle M Hall 2018-09-07 11:39:04 UTC
Created attachment 78512 [details] [review]
Bug 19469 [QA Followup]: Use hold item's itemtype if available, fix priority changing
Comment 45 Kyle M Hall 2018-09-07 11:39:08 UTC
Created attachment 78513 [details] [review]
Bug 19469 [QA Followup]: When altering priorities, use the existing priority numbers of the given table
Comment 46 Nick Clemens 2018-09-07 14:55:47 UTC
Created attachment 78526 [details] [review]
Bug 19469: (RM follow-up) Add filters and fix variable name
Comment 47 Nick Clemens 2018-09-07 14:58:13 UTC
(In reply to Jonathan Druart from comment #29)
> There is something else I think:
> If you change the order with the select, do not save but click on one of the
> arrows, it will not do what you expect (i.e. save what you saw just before
> saving).

This is a bug in master too, set last hold to priority one via select - then click an arrow
Comment 48 Nick Clemens 2018-09-07 15:03:26 UTC
Created attachment 78529 [details] [review]
(RM follow-up) Add filters and fix variable name
Comment 49 Kyle M Hall 2018-09-14 19:01:18 UTC
Created attachment 78707 [details] [review]
Bug 19469: (QA followup) Disable priority pulldowns when using
Comment 50 Nick Clemens 2018-09-14 19:55:19 UTC
Awesome work all!

Pushed to master for 18.11
Comment 51 Martin Renvoize 2018-09-17 13:32:52 UTC
Enhancement, will not be backported to 18.05.x series.
Comment 52 Jonathan Druart 2018-11-01 15:50:31 UTC
Links on the "multi holds" view are broken, please fix.

To recreate:
- Search
- Select few records
- Click "Place hold"
=> On "Home › Catalog › Confirm holds" the link of the record does not have a biblionumber value, the link is "/cgi-bin/koha/catalogue/detail.pl?biblionumber="
Comment 53 Jonathan Druart 2018-11-01 15:52:15 UTC
Created attachment 81828 [details] [review]
Bug 19469: Fix record links on multi holds
Comment 54 Jonathan Druart 2018-11-01 15:52:48 UTC
(In reply to Jonathan Druart from comment #52)
> Links on the "multi holds" view are broken, please fix.

Ok done, it was an easy fix.
Comment 55 Jonathan Druart 2018-11-01 15:58:30 UTC
Created attachment 81829 [details] [review]
Bug 19469: Fix record links on multi holds
Comment 56 Nick Clemens 2019-03-21 18:33:25 UTC
(In reply to Jonathan Druart from comment #55)
> Created attachment 81829 [details] [review] [review]
> Bug 19469: Fix record links on multi holds

Follow-up pushed to master