Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 29049: Simplify holds priority dropdown logic on request.pl

Browse source 
This patch makes a few changes:
1 - If we are not splitting the queue ( HoldsSplitQueue == 'nothing' ) then
    there are no changes for 'virtual' vs 'actual' numbering
2 - If we are splitting the queue you cannot use the dropdown, so we do not need to process the options
3 - If the hold is 'found' we do not need to process the options
4 - We can simply use the 'last priority' to build the options, we do not need to process in the script
5 - We can use a block to build the options
6 - Remove a stray holds.index

To test:
 1 - Place 5 holds on a bib
 2 - 'Find' two of the holds i.e. check them in to set status to 'transit' or 'waiting'
 3 - View the holds page for the biblio
 4 - Inspect the priorty dropdown for the found holds, note they have options for 1-5 but are disabled
 5 - Note the dropdowns for other holds have options 1-5
 6 - Note options 4&5 in the dropdowns have no effect
 7 - Apply patch
 8 - Inspect the priority dopdowns on the found holds
 9 - Note they only show their found status
10 - Note the other dropdowns only show options 1-3
11 - Test with other HoldsSplitQueue options and HoldsSplitQueueNumbering
12 - When holds queue is split, no dropdowns should be active and should show only their current priority
13 - When  HoldsSplitQueueNumbering is 'virtual' the dropdown should be disabled and correctly count the number of unfound holds in the list

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
JK: Fixed grammar in commit title
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

JD: Remove trailing 'i'

Bug 29049: (QA follow-up) Remove excessive whitespace

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This commit is contained in:
Nick Clemens 2021-09-17 13:59:25 +00:00 committed by Kyle Hall
parent 75a309eaa7
commit e821f50b4e
2 changed files with 34 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
View file

@ -25,8 +25,29 @@
[% SET first_priority = 0 %]
[% SET last_priority = holds.last.priority %]
[% BLOCK priority_dropdown %]
[% SET loop_priority = 1 %]
[% WHILE loop_priority <= last_priority %]
[% IF this_priority == loop_priority %]
<option value="[% loop_priority | html %]" selected="selected">[% loop_priority | html %]</option>
[% ELSE %]
<option value="[% loop_priority | html %]">[% loop_priority | html %]</option>
[% END %]
[% loop_priority = loop_priority + 1 %]
[% END %]
[% END %]
[% FOREACH hold IN holds %]
[% IF !hold.found && first_priority == 0 %][% first_priority = hold.priority %][% END %]
[% IF !hold.found && first_priority == 0 %]
[% first_priority = hold.priority %]
[% found_holds = loop.index() %]
[% END %]
[% IF Koha.Preference('HoldsSplitQueueNumbering') == 'actual' %]
[% this_priority = hold.priority %]
[% ELSE %]
[% this_priority = loop.count() - found_holds %]
[% END %]
<tr>
<td>
<input type="hidden" name="reserve_id" value="[% hold.reserve_id | html %]" />
@ -34,10 +55,17 @@
<input type="hidden" name="biblionumber" value="[% hold.biblionumber | html %]" />
[% IF Koha.Preference('HoldsSplitQueue') == "nothing" && !hold.found %]
<select name="rank-request" class="rank-request" data-hold-id="[% hold.reserve_id | html %]">
[% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
[% PROCESS priority_dropdown %]
[% ELSE %]
<option value="[% hold.priority | html %]" selected="selected">[% this_priority | html %]</option>
[% END %]
<option value="del">del</option>
</select>
[% ELSE %]
<input type="hidden" name="rank-request" class="rank-request" value="[% hold.priority | html %]" data-hold-id="[% hold.reserve_id | html %]">
<select name="rank-request" class="rank-request" disabled="disabled" data-hold-id="[% hold.reserve_id | html %]">
[% END %]
[% IF ( hold.found ) %]
[% IF ( hold.intransit ) %]
<option value="T" selected="selected">In transit</option>
@ -46,42 +74,17 @@
[% ELSE %]
<option value="W" selected="selected">Waiting</option>
[% END %]
[% END %]
[% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
[% IF Koha.Preference('HoldsSplitQueueNumbering') == 'actual' %]
[% FOREACH optionloo IN hold.optionloop %]
[% IF ( optionloo.selected ) %]
<option value="[% optionloo.num | html %]" selected="selected">[% optionloo.num | html %]</option>
[% ELSE %]
<option value="[% optionloo.num | html %]">[% optionloo.num | html %]</option>
<option value="[% hold.priority | html %]" selected="selected">[% this_priority | html %]</option>
[% END %]
[% END %]
[% ELSE %]
[% SET ranker = 1 %]
[% FOREACH h IN holds %]
[% NEXT IF h.found %]
[% IF ( h.priority == hold.priority ) %]
<option value="[% h.priority | html %]" selected="selected">[% ranker | html %]</option>
[% ELSE %]
<option value="[% h.priority | html %]">[% ranker | html %]</option>
[% END %]
[% ranker = ranker + 1 %]
[% END %]
[% END %]
[% ELSIF !hold.found %]
<option value="[% hold.priority | html %]" selected="selected">[% hold.priority | html %]</option>
[% END %]
<option value="del">del</option>
</select>
[% END %]
</td>
[% IF ( CAN_user_reserveforothers_modify_holds_priority ) %]
[% UNLESS hold.found %]
[% SET prev_priority = loop.prev.priority %]
[% SET next_priority = loop.next.priority %]
[% holds.index | html %]
<td style="white-space:nowrap;">
<a title="Move hold up" href="request.pl?action=move&amp;where=up&amp;first_priority=[% first_priority | html %]&amp;last_priority=[% last_priority | html %]&amp;prev_priority=[% prev_priority | html %]&amp;next_priority=[% next_priority | html %]&amp;borrowernumber=[% hold.borrowernumber | html %]&amp;biblionumber=[% hold.biblionumber | html %]&amp;reserve_id=[% hold.reserve_id | html %]&amp;date=[% hold.date | html %]">

12
reserve/request.pl
View file

@ -661,17 +661,6 @@ foreach my $biblionumber (@biblionumbers) {
{
my $priority = $res->priority();
my %reserve;
my @optionloop;
for ( my $i = 1 ; $i <= $totalcount ; $i++ ) {
push(
@optionloop,
{
num => $i,
selected => ( $i == $priority ),
}
);
}
if ( $res->is_found() ) {
$reserve{'holdingbranch'} = $res->item()->holdingbranch();
$reserve{'biblionumber'} = $res->item()->biblionumber();
@ -704,7 +693,6 @@ foreach my $biblionumber (@biblionumbers) {
$reserve{'barcode'} = $res->item() ? $res->item()->barcode() : undef;
$reserve{'priority'} = $res->priority();
$reserve{'lowestPriority'} = $res->lowestPriority();
$reserve{'optionloop'} = \@optionloop;
$reserve{'suspend'} = $res->suspend();
$reserve{'suspend_until'} = $res->suspend_until();
$reserve{'reserve_id'} = $res->reserve_id();