From ee9cc08a84bb70b048ffba44e75846a4052a85d6 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Sat, 30 Sep 2017 08:02:57 -0400 Subject: [PATCH] Bug 19469: Add ability to split view of holds view on record by pickup library and/or itemtype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Followed test plan, patch worked as described. Also passed QA test tool Signed-off-by: Alex Buckley Signed-off-by: Andreas Hedström Mace Signed-off-by: Tomas Cohen Arazi --- C4/Reserves.pm | 25 +- .../data/mysql/atomicupdate/split_holds_queue.sql | 3 + installer/data/mysql/sysprefs.sql | 1 + koha-tmpl/intranet-tmpl/prog/css/staff-global.css | 8 + .../intranet-tmpl/prog/en/includes/holds_table.inc | 200 ++++++++++++++++ .../en/modules/admin/preferences/circulation.pref | 14 ++ .../prog/en/modules/reserve/request.tt | 257 +++++++-------------- reserve/request.pl | 10 +- t/db_dependent/Holds.t | 8 +- 9 files changed, 333 insertions(+), 193 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/split_holds_queue.sql create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 73b86b606c..60355c3d96 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1177,7 +1177,7 @@ sub _get_itype { =head2 AlterPriority - AlterPriority( $where, $reserve_id ); + AlterPriority( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ); This function changes a reserve's priority up, down, to the top, or to the bottom. Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was placed @@ -1185,7 +1185,7 @@ Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was =cut sub AlterPriority { - my ( $where, $reserve_id ) = @_; + my ( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ) = @_; my $hold = Koha::Holds->find( $reserve_id ); return unless $hold; @@ -1195,21 +1195,18 @@ sub AlterPriority { return; } - if ( $where eq 'up' || $where eq 'down' ) { - - my $priority = $hold->priority; - $priority = $where eq 'up' ? $priority - 1 : $priority + 1; - _FixPriority({ reserve_id => $reserve_id, rank => $priority }) - + if ( $where eq 'up' ) { + return unless $prev_priority; + _FixPriority({ reserve_id => $reserve_id, rank => $prev_priority }) + } elsif ( $where eq 'down' ) { + return unless $next_priority; + _FixPriority({ reserve_id => $reserve_id, rank => $next_priority }) } elsif ( $where eq 'top' ) { - - _FixPriority({ reserve_id => $reserve_id, rank => '1' }) - + _FixPriority({ reserve_id => $reserve_id, rank => $first_priority }) } elsif ( $where eq 'bottom' ) { - - _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); - + _FixPriority({ reserve_id => $reserve_id, rank => $last_priority }); } + # FIXME Should return the new priority } diff --git a/installer/data/mysql/atomicupdate/split_holds_queue.sql b/installer/data/mysql/atomicupdate/split_holds_queue.sql new file mode 100644 index 0000000000..f612dd44d1 --- /dev/null +++ b/installer/data/mysql/atomicupdate/split_holds_queue.sql @@ -0,0 +1,3 @@ +INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES +('HoldsSplitQueue','nothing','nothing|branch|itemtype|branch_itemtype','In the staff client, split the holds view by the given criteria','Choice'), +('HoldsSplitQueueNumbering', 'actual', 'actual|virtual', 'If the holds queue is split, decide if the acual priorities should be displayed', 'Choice'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index ad8f41d677..acbd8d5927 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -192,6 +192,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('HoldFeeMode','not_always','any_time_is_placed|not_always|any_time_is_collected','Set the hold fee mode','Choice'), ('HoldsLog','0',NULL,'If ON, log create/cancel/suspend/resume actions on holds.','YesNo'), ('HoldsQueueSkipClosed', '0', NULL, 'If enabled, any libraries that are closed when the holds queue is built will be ignored for the purpose of filling holds.', 'YesNo'), +('HoldsSplitQueue','nothing','nothing|branch|itemtype|branch_itemtype','In the staff client, split the holds view by the given criteria','Choice'), ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'), ('HomeOrHoldingBranch','holdingbranch','holdingbranch|homebranch','Used by Circulation to determine which branch of an item to check with independent branches on, and by search to determine which branch to choose for availability ','Choice'), ('HouseboundModule',0,'','If ON, enable housebound module functionality.','YesNo'), diff --git a/koha-tmpl/intranet-tmpl/prog/css/staff-global.css b/koha-tmpl/intranet-tmpl/prog/css/staff-global.css index 32a614bd70..f6fe236877 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/staff-global.css +++ b/koha-tmpl/intranet-tmpl/prog/css/staff-global.css @@ -368,6 +368,14 @@ fieldset { border-radius:5px; } +fieldset.standard { + background-color:#f4f8f9 !important; +} + +fieldset.contrast { + background-color:#F3F3F3 !important; +} + fieldset.lastchecked { margin-bottom : 0; border-bottom-width: 0; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc new file mode 100644 index 0000000000..38e14e45b0 --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -0,0 +1,200 @@ + + + [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] + + + [% ELSE %] + + [% END %] + + + + + + + [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] + + [% END %] + + [% IF SuspendHoldsIntranet %][% END %] + + + [% FOREACH hold IN holds %] + + + + [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] + [% SET first_priority = holds.first.priority %] + [% SET last_priority = holds.last.priority %] + [% SET prev_priority = loop.prev.priority %] + [% SET next_priority = loop.next.priority %] + [% holds.index %] + + + [% END %] + + + + + + + + + + + + [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] + + [% END %] + + + + [% IF SuspendHoldsIntranet %] + + [% END # IF SuspendHoldsIntranet %] + + + + [% END %] +
Priority Delete?PatronNotesDateExpirationPickup libraryDetailsToggle set to lowest priority  
+ + + + + + + Go up + + + + Go top + + + + Go bottom + + + + Go down + + + + [% IF ( hold.hidename ) %] + [% hold.cardnumber (hold.borrowernumber) %] + [% ELSE %] + [% hold.firstname %] [% hold.surname %] + [% END %] + + [% hold.notes %][% hold.date %][% hold.expirationdate %] + [% IF ( hold.found ) %] + [% IF ( hold.atdestination ) %] + [% IF ( hold.found ) %] + Item waiting at [% hold.wbrname %] since [% hold.waiting_date | $KohaDates %] + [% ELSE %] + Waiting to be pulled + [% END %] + [% ELSE %] + Item being transferred to [% hold.wbrname %] + [% END %] + [% ELSE %] + [% IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 %] + [% Branches.GetName(hold.branchcode) %] + [% ELSE %] + + [% END %] + [% END %] + + [% IF ( hold.found ) %] + + [% IF ( hold.barcodenumber ) %] + [% hold.barcodenumber %] + + [% ELSE %] + No barcode + [% END %] + + [% ELSE %] + [% IF ( hold.item_level_hold ) %] + + Only item + + [% IF ( hold.barcodenumber ) %] + [% hold.barcodenumber %] + + [% ELSE %] + No barcode + [% END %] + + + [% ELSE %] + [% IF hold.itemtype %] + Next available [% ItemTypes.GetDescription( hold.itemtype ) %] item + [% ELSE %] + Next available + [% END %] + + + [% END %] + [% END %] + + + [% IF ( hold.lowestPriority ) %] + Unset lowest priority + [% ELSE %] + Set to lowest priority + [% END %] + + + + Cancel + + + [% UNLESS ( hold.found ) %] + + + [% IF AutoResumeSuspendedHolds %] + + + Clear date + [% ELSE %] + + [% END %] + + [% ELSE %] + + [% END %] +
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 8ec6a6b5b6..d3e171b22c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -506,6 +506,20 @@ Circulation: no: "Don't cumulate" - the restriction periods. Holds Policy: + - + - In the staff client, split the holds queue into separate tables by + - pref: HoldsSplitQueue + choices: + nothing: nothing + branch: "pickup library" + itemtype: "hold itemtype" + branch_itemtype: "pickup library & itemtype" + - + - If the holds queue is split, show librarians + - pref: HoldsSplitQueueNumbering + choices: + actual: "the actual priority, which may be out of order" + virtual: "'virtual' priorities, where each group is numbered separately" - - pref: AllowHoldItemTypeSelection choices: diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index e551aacb6f..2e70b6edf3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -1,4 +1,5 @@ [% USE Asset %] +[% USE Dumper %] [% USE Koha %] [% USE KohaDates %] [% USE Branches %] @@ -727,194 +728,106 @@ function checkMultiHold() { [% FOREACH biblioloo IN biblioloop %] [% IF ( biblioloo.reserveloop ) %] - - [% IF ( multi_hold ) %] - - [% END %] - - - [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] - - - [% ELSE %] - - [% END %] - - - - - - - [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] - - [% END %] - - [% IF SuspendHoldsIntranet %][% END %] - - - [% FOREACH reserveloo IN biblioloo.reserveloop %] - - + [% IF Koha.Preference('HoldsSplitQueue') == 'branch' %] + [% SET branchcodes = [] %] - [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] - + [% FOREACH i IN itemtypes.sort %] + [% SET holds_by_itemtype = [] %] + [% FOREACH h IN biblioloo.reserveloop %] + [% IF h.itemtype == i %] + [% holds_by_itemtype.push( h ) %] [% END %] + [% END %] - - - - - - - - - - - [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] - [% END %] - - - [% IF SuspendHoldsIntranet %] - - [% END # IF SuspendHoldsIntranet %] - + [% SET holds_by_itemtype = [] %] + [% FOREACH h IN holds_by_branch %] + [% IF h.itemtype == i %] + [% holds_by_itemtype.push( h ) %] + [% END %] + [% END %] + [% INCLUDE holds_table.inc holds=holds_by_itemtype %] + + [% END %] + + [% END %] + [% ELSE %] + [% INCLUDE holds_table.inc holds=biblioloo.reserveloop %] + [% END %] - [% END %] -
[% biblioloo.title |html %]
Priority Delete?PatronNotesDateExpirationPickup libraryDetailsToggle set to lowest priority  
- - - - - - - Go up - + [% FOREACH h IN biblioloo.reserveloop %] + [% branchcodes.push( h.branchcode ) %] + [% END %] + [% branchcodes = branchcodes.unique %] - - Go top - + [% FOREACH b IN branchcodes.sort %] + [% SET holds_by_branch = [] %] + [% FOREACH h IN biblioloo.reserveloop %] + [% IF h.branchcode == b %] + [% holds_by_branch.push( h ) %] + [% END %] + [% END %] +
+ [% Branches.GetName( b ) %] + [% INCLUDE holds_table.inc holds=holds_by_branch %] +
+ [% END %] + [% ELSIF Koha.Preference('HoldsSplitQueue') == 'itemtype' %] + [% SET itemtypes = [] %] - - Go bottom - + [% FOREACH h IN biblioloo.reserveloop %] + [% itemtypes.push( h.itemtype ) %] + [% END %] + [% itemtypes = itemtypes.unique %] - - Go down - - 		- - [% IF ( reserveloo.hidename ) %] - [% reserveloo.cardnumber (reserveloo.borrowernumber) %] - [% ELSE %] - [% reserveloo.firstname %] [% reserveloo.surname %] - [% END %] - - [% reserveloo.notes %][% reserveloo.date %][% reserveloo.expirationdate %] - [% IF ( reserveloo.found ) %] - [% IF ( reserveloo.atdestination ) %] - [% IF ( reserveloo.found ) %] - Item waiting at [% reserveloo.wbrname %] since [% reserveloo.waiting_date | $KohaDates %] - [% ELSE %] - Waiting to be pulled - [% END %] - [% ELSE %] - Item being transferred to [% reserveloo.wbrname %] - [% END %] - [% ELSE %] - [% IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 %] - [% Branches.GetName(reserveloo.branchcode) %] - [% ELSE %] - - [% END %] - [% END %] - - [% IF ( reserveloo.found ) %] - - [% IF ( reserveloo.barcodenumber ) %] - [% reserveloo.barcodenumber %] - - [% ELSE %] - No barcode - [% END %] - - [% ELSE %] - [% IF ( reserveloo.item_level_hold ) %] - - Only item - - [% IF ( reserveloo.barcodenumber ) %] - [% reserveloo.barcodenumber %] - - [% ELSE %] - No barcode - [% END %] - - - [% ELSE %] - [% IF reserveloo.itemtype %] - Next available [% ItemTypes.GetDescription( reserveloo.itemtype ) %] item - [% ELSE %] - Next available - [% END %] +
+ [% IF i %] + [% ItemTypes.GetDescription( i ) %] + [% ELSE %] + Any item type + [% END %] + [% INCLUDE holds_table.inc holds=holds_by_itemtype %] +
+ [% END %] + [% ELSIF Koha.Preference('HoldsSplitQueue') == 'branch_itemtype' %] + [% SET branchcodes = [] %] - - [% END %] + [% FOREACH h IN biblioloo.reserveloop %] + [% branchcodes.push( h.branchcode ) %] + [% END %] + [% branchcodes = branchcodes.unique %] + + [% FOREACH b IN branchcodes.sort %] +
+ [% Branches.GetName( b ) %] + [% SET holds_by_branch = [] %] + [% FOREACH h IN biblioloo.reserveloop %] + [% IF h.branchcode == b %] + [% holds_by_branch.push( h ) %] [% END %] -
- - [% IF ( reserveloo.lowestPriority ) %] - Unset lowest priority - [% ELSE %] - Set to lowest priority - [% END %] - - - - Cancel - - - [% UNLESS ( reserveloo.found ) %] - - - [% IF AutoResumeSuspendedHolds %] - - - Clear date - [% ELSE %] - - [% END %] + [% SET itemtypes = [] %] + [% FOREACH h IN holds_by_branch %] + [% itemtypes.push( h.itemtype ) %] + [% END %] + [% itemtypes = itemtypes.unique %] + [% FOREACH i IN itemtypes.sort %] +
+ [% IF i %] + [% ItemTypes.GetDescription( i ) %] [% ELSE %] - + Any item type [% END %] -
[% END %] [% END %] diff --git a/reserve/request.pl b/reserve/request.pl index 2902de09ae..40d35f33eb 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -85,9 +85,13 @@ my $action = $input->param('action'); $action ||= q{}; if ( $action eq 'move' ) { - my $where = $input->param('where'); - my $reserve_id = $input->param('reserve_id'); - AlterPriority( $where, $reserve_id ); + my $where = $input->param('where'); + my $reserve_id = $input->param('reserve_id'); + my $prev_priority = $input->param('prev_priority'); + my $next_priority = $input->param('next_priority'); + my $first_priority = $input->param('first_priority'); + my $last_priority = $input->param('last_priority'); + AlterPriority( $where, $reserve_id, $prev_priority, $next_priority, $first_priority, $last_priority ); } elsif ( $action eq 'cancel' ) { my $reserve_id = $input->param('reserve_id'); my $hold = Koha::Holds->find( $reserve_id ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 1e6c234487..e9bf70953c 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -210,19 +210,19 @@ is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" ); $holds = $biblio->holds; $hold = $holds->next; -AlterPriority( 'top', $hold->reserve_id ); +AlterPriority( 'top', $hold->reserve_id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '1', "Test AlterPriority(), move to top" ); -AlterPriority( 'down', $hold->reserve_id ); +AlterPriority( 'down', $hold->reserve_id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '2', "Test AlterPriority(), move down" ); -AlterPriority( 'up', $hold->reserve_id ); +AlterPriority( 'up', $hold->reserve_id, 1, 3, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '1', "Test AlterPriority(), move up" ); -AlterPriority( 'bottom', $hold->reserve_id ); +AlterPriority( 'bottom', $hold->reserve_id, undef, 2, 1, 6 ); $hold = Koha::Holds->find( $reserveid ); is( $hold->priority, '6', "Test AlterPriority(), move to bottom" ); -- 2.15.1 (Apple Git-101)