[Bug 29049] New: Holds page shows too many priority options in pulldown
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Bug ID: 29049 Summary: Holds page shows too many priority options in pulldown Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com To recreate: 1 - Place 5 holds on a biblio 2 - 'Find' two holds i.e. check them in to set them as transit or waiting 3 - Load the holds page for the biblio 4 - Check the pull down for priority 5 - Note holds has priority 1-3 but dropdown is 1-5 6 - Click 4 or 5 and note nothing happens -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 124992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124992&action=edit Bug 29049: Simply holds priorty dropdown logic on request.pl 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125013&action=edit Bug 29049: (follow-up) Reduce number of line returns in page While moving the dropdown to a block, I noticed the page size was very large when there are many holds This patch chomps a large amoutn of whitespace, from the block and elsewhere and drecreased the paeg from about 400k lines to 25k lines for a record with ~200 holds To test: 1 - Apply previous patch 2 - Load holds tab for a biblio 3 - Save the page as html from your browser, note the size of the file 4 - Apply patch this 5 - Reload and repeat 6 - Note the size is much smaller -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124992|0 |1 is obsolete| | --- Comment #3 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 125067 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125067&action=edit Bug 29049: Simply holds priorty dropdown logic on request.pl 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125013|0 |1 is obsolete| | --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 125068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125068&action=edit Bug 29049: (follow-up) Reduce number of line returns in page While moving the dropdown to a block, I noticed the page size was very large when there are many holds This patch chomps a large amoutn of whitespace, from the block and elsewhere and drecreased the paeg from about 400k lines to 25k lines for a record with ~200 holds To test: 1 - Apply previous patch 2 - Load holds tab for a biblio 3 - Save the page as html from your browser, note the size of the file 4 - Apply patch this 5 - Reload and repeat 6 - Note the size is much smaller Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joonas.kylmala@iki.fi --- Comment #5 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- In case anybody else is testing this I was bit confused why i couldn't reproduce the problem at first: turns out we are missing HoldsSplitQueueNumbering from the installer/data/mysql/mandatory/sysprefs.sql file, so on fresh install the bug doesn't happen because we are using the virtual mechanism if none is specified (even though if you go to the systempreferences menu it shows actual). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |joonas.kylmala@iki.fi |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #6 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- This causes the following warning:
Argument "" isn't numeric in subtraction (-) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc line 47.
Let me see if I can come up with a fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125067|0 |1 is obsolete| | Attachment #125068|0 |1 is obsolete| | --- Comment #7 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125306 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125306&action=edit Bug 29049: Simplify holds priority dropdown logic on request.pl 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #8 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125307 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125307&action=edit Bug 29049: (follow-up) Reduce number of line returns in page While moving the dropdown to a block, I noticed the page size was very large when there are many holds This patch chomps a large amoutn of whitespace, from the block and elsewhere and drecreased the paeg from about 400k lines to 25k lines for a record with ~200 holds To test: 1 - Apply previous patch 2 - Load holds tab for a biblio 3 - Save the page as html from your browser, note the size of the file 4 - Apply patch this 5 - Reload and repeat 6 - Note the size is much smaller Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #9 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125308 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125308&action=edit Bug 29049: (QA follow-up) Remove unused variable Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #10 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125309 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125309&action=edit Bug 29049: (QA follow-up) Remove excessive whitespace Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #11 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Created attachment 125310 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125310&action=edit Bug 29049: (QA follow-up) Initialize uninitialized variable to prevent warning Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Joonas Kylmälä from comment #10)
Created attachment 125309 [details] [review] Bug 29049: (QA follow-up) Remove excessive whitespace
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Will be squashed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Also will amend first patch to remove a trailing 'i': diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index daf50d69a76..c48987677eb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -53,7 +53,7 @@ <input type="hidden" name="borrowernumber" value="[% hold.borrowernumber | html %]" /> <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 %]">i + <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 %] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 125397 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125397&action=edit Bug 29049: Remove reference to optionloop It's no longer used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29049 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable Version(s)|21.11.00 |21.11.00,21.05.05 released in| | --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.05 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org