Bug 30579 - When placing item level hold, some options that are not used are not disabled
Summary: When placing item level hold, some options that are not used are not disabled
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Nick Clemens (kidclamp)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
: 30564 (view as bug list)
Depends on: 30577
Blocks: 36864 36899
  Show dependency treegraph
 
Reported: 2022-04-21 12:32 UTC by Nick Clemens (kidclamp)
Modified: 2024-11-18 16:17 UTC (History)
18 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This is part of improvements to placing holds from the record page in the staff interface. It improves the structure of the page to make it clear that the different type of holds ("Hold next available item", "Hold next available item from an item group" (when enabled), and "Hold a specific item") are mutually-exclusive options, including: - Simplifying the hold details section at the start of the page - Putting the information and options for each type of hold in their own selectable section of the page Note: additional improvements to the design were made in bugs 36864 and 36899.
Version(s) released in:
24.05.00
Circulation function:


Attachments
Bug 30579: Disentangle multi-hold and single bib forms (51.50 KB, patch)
2022-04-21 12:33 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Group next available hold options, and disable when item is chosen (10.21 KB, patch)
2022-04-21 12:33 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (51.57 KB, patch)
2022-04-21 15:16 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 30579: Group next available hold options, and disable when item is chosen (10.27 KB, patch)
2022-04-21 15:16 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (51.62 KB, patch)
2022-04-22 15:03 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 30579: Group next available hold options, and disable when item is chosen (10.32 KB, patch)
2022-04-22 15:03 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Screenshot of place hold page with patch applied (141.48 KB, image/png)
2022-04-28 14:19 UTC, Katrin Fischer
Details
Suggested update (137.40 KB, image/gif)
2022-05-02 14:27 UTC, Owen Leonard
Details
Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer (19.22 KB, patch)
2022-05-02 16:14 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (51.62 KB, patch)
2022-05-03 10:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Group next available hold options, and disable when item is chosen (10.31 KB, patch)
2022-05-03 10:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer (19.21 KB, patch)
2022-05-03 10:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer (19.15 KB, patch)
2022-05-11 12:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30579: (follow-up) Expand range of controls which are disabled/enabled (4.62 KB, patch)
2022-05-11 12:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (9.19 KB, patch)
2022-09-23 15:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Whitespace (27.31 KB, patch)
2022-09-23 15:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Group next available hold options, and disable when item is chosen (9.77 KB, patch)
2022-09-23 15:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer (10.64 KB, patch)
2022-09-23 15:52 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (48.94 KB, patch)
2023-03-28 14:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (50.16 KB, patch)
2023-03-29 10:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Update form checker for each form (3.10 KB, patch)
2023-04-07 12:07 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (53.03 KB, patch)
2023-05-05 14:56 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (53.08 KB, patch)
2023-05-05 18:25 UTC, Hinemoea Viault
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (53.17 KB, patch)
2023-05-19 15:54 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (53.21 KB, patch)
2023-05-19 18:44 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.16 KB, patch)
2023-05-23 09:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (54.04 KB, patch)
2023-08-28 12:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.16 KB, patch)
2023-08-28 12:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.28 KB, patch)
2023-08-28 12:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (55.33 KB, patch)
2023-08-28 14:46 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (54.01 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.16 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.28 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (55.33 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (1.91 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.80 KB, patch)
2023-10-25 12:15 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (54.08 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.23 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.35 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (55.40 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (1.98 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.88 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.12 KB, patch)
2023-10-26 17:28 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (53.43 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.23 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.35 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (54.84 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (1.98 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.88 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.12 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Remove unnecessary nested form elements (1.38 KB, patch)
2024-03-15 18:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (56.93 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.23 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.35 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (55.24 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (1.98 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.88 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.12 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Remove unnecessary nested form elements (1.38 KB, patch)
2024-05-01 16:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (57.00 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.29 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.41 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (55.31 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (2.04 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.94 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.18 KB, patch)
2024-05-02 09:39 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Remove unnecessary nested form elements (1.44 KB, patch)
2024-05-02 09:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (56.90 KB, patch)
2024-05-02 16:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.29 KB, patch)
2024-05-02 16:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.41 KB, patch)
2024-05-02 16:10 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (70.38 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (2.04 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.94 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.18 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Remove unnecessary nested form elements (1.40 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Fix rebase issue (3.54 KB, patch)
2024-05-02 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Disentangle multi-hold and single bib forms (56.90 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Add missing for attribute on label (2.29 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Remove doubled notes field (1.41 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add fieldsets and style disabled elements (70.38 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options (2.04 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Add required label and warning when hold levels required (2.94 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) fix button id (2.18 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Remove unnecessary nested form elements (1.40 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: Fix rebase issue (3.54 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Fix rebase of multi holds form and add CSRF (4.96 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Standardize labels: (1.38 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30579: (follow-up) Style selections as buttons (7.79 KB, patch)
2024-05-07 12:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2022-04-21 12:32:05 UTC

    
Comment 1 Nick Clemens (kidclamp) 2022-04-21 12:33:09 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2022-04-21 12:33:14 UTC Comment hidden (obsolete)
Comment 3 Andrew Fuerste-Henry 2022-04-21 15:08:44 UTC Comment hidden (obsolete)
Comment 4 Andrew Fuerste-Henry 2022-04-21 15:16:52 UTC Comment hidden (obsolete)
Comment 5 Andrew Fuerste-Henry 2022-04-21 15:16:57 UTC Comment hidden (obsolete)
Comment 6 Andrew Fuerste-Henry 2022-04-22 15:03:05 UTC Comment hidden (obsolete)
Comment 7 Andrew Fuerste-Henry 2022-04-22 15:03:09 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2022-04-28 14:19:59 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2022-05-02 14:27:40 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2022-05-02 16:14:09 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens (kidclamp) 2022-05-03 10:22:44 UTC Comment hidden (obsolete)
Comment 12 Nick Clemens (kidclamp) 2022-05-03 10:22:48 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens (kidclamp) 2022-05-03 10:22:52 UTC Comment hidden (obsolete)
Comment 14 Nick Clemens (kidclamp) 2022-05-03 10:23:56 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2022-05-11 12:23:22 UTC Comment hidden (obsolete)
Comment 16 Owen Leonard 2022-05-11 12:23:27 UTC Comment hidden (obsolete)
Comment 17 Sally 2022-05-17 15:19:46 UTC Comment hidden (obsolete)
Comment 18 Nick Clemens (kidclamp) 2022-09-23 15:52:29 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens (kidclamp) 2022-09-23 15:52:33 UTC Comment hidden (obsolete)
Comment 20 Nick Clemens (kidclamp) 2022-09-23 15:52:37 UTC Comment hidden (obsolete)
Comment 21 Nick Clemens (kidclamp) 2022-09-23 15:52:41 UTC Comment hidden (obsolete)
Comment 22 Nick Clemens (kidclamp) 2022-09-23 15:53:26 UTC Comment hidden (obsolete)
Comment 23 Nick Clemens (kidclamp) 2023-03-28 14:27:51 UTC
Created attachment 148835 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly
Comment 24 Matt Blenkinsop 2023-03-29 08:27:20 UTC
Discussed at hackfest - next available item from an item group needs a pickup location as an error is being thrown
Comment 25 Nick Clemens (kidclamp) 2023-03-29 10:28:19 UTC
Created attachment 148898 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly
Comment 26 Matt Blenkinsop 2023-04-05 10:12:16 UTC
Hi Nick - I'm getting an alert asking me to select a pickup location when placing a hold on next available item, even when a location is selected. Could you check again at your end and see if you're seeing the same? Looks like the alert is being thrown by the else block in lines 1570 - 1575
Comment 27 Nick Clemens (kidclamp) 2023-04-07 12:07:32 UTC
Created attachment 149271 [details] [review]
Bug 30579: (follow-up) Update form checker for each form
Comment 28 Matt Blenkinsop 2023-04-17 09:29:50 UTC
Still getting an error, this time when placing a hold on next available item in an item group - I'm getting an alert saying "Please select an item group to place hold", even though an item group is selected
Comment 29 Magnus Enger 2023-05-05 13:21:08 UTC
I don't think this works as intended for me. 

1. When I load the page, the radio button to the right of "Hold next available item" is active/selected. 
- Under "Hold next available item from an item group", the dropdown and the radio button is disabled. The dropdown is "greyed out" and nothing happens if I click on either of them. 
- But under "Place a hold on a specific item" the dropdown for "Allowed pickup locations" is not "greyed out" and I can interact with it. I can click on the radio buttons under "Hold", and if I do, the radio button to the right of "Hold next available item" is no longer active/selected. In fact, none of the three radio buttons to the right of the headings are active/selected.

2. If i click on the radio button for "Hold next available item from an item group"
- I can interact with the "Hold" radio button and the "Allowed pickup locations" dropdown under "Place a hold on a specific item". 
- If i click on a radio button under "Hold", then the radio button to the right of "Hold next available item from an item group" remains active/selected (this is different from what happens under 1 above).
Comment 30 Magnus Enger 2023-05-05 13:21:53 UTC
PS. Tested on 112.0.5615.49 snap (64-bit) on Ubuntu
Comment 31 Nick Clemens (kidclamp) 2023-05-05 14:56:20 UTC
Created attachment 150767 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly
Comment 32 Hinemoea Viault 2023-05-05 18:25:03 UTC
Created attachment 150786 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly

Signed-off-by: Hinemoea Viault <hinemoea.viault@inlibro.com>
Comment 33 David Nind 2023-05-09 01:33:52 UTC
Hi Hinemoea.

As you have signed off this bug, could you update the bug status to Signed Off?

That way, you will get the credit on the dashboard (https://dashboard.koha-community.org/).

(It will also move in to the Needs QA queue, for the QA Team to do their thing!)

David
Comment 34 Sam Lau 2023-05-18 19:23:35 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Comment 35 Nick Clemens (kidclamp) 2023-05-19 15:54:04 UTC
Created attachment 151483 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Comment 36 Sam Lau 2023-05-19 18:44:30 UTC
Created attachment 151493 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Place a hold for a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Confirm you can successfully place all 3 types of holds
10 - Confirm if placing a second hold that the type is forced correctly

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 37 Jonathan Druart 2023-05-23 09:36:58 UTC
Created attachment 151558 [details] [review]
Bug 30579: Add missing for attribute on label
Comment 38 Jonathan Druart 2023-05-23 09:41:30 UTC
That's better but we should do better in my opinion. First would be to have only 1 "place hold" button maybe? And hide the form when the related radio is not checked?

Owen, what do you think?
Comment 39 Marcel de Rooy 2023-06-30 08:00:10 UTC
This is a very brave effort to get this template more clear. But as it seems, not without any discussion. There was no feedback on the previous comment.

The number of adjustments
 .../prog/en/modules/reserve/request.tt        | 587 ++++++++++--------
Does not make it easier.
Holds is very bug prone. Not to mention recent additions like item groups etc.

I am not sure if this (simple) test plan covers all edge cases. Hard to verify. Does this need some broader consensus/discussion?
Should this be marked as an enhancment?
Comment 40 Owen Leonard 2023-06-30 14:44:09 UTC
I could see us using a Bootstrap Collapse to show the options, with the "next available" section displayed by default.

I could also see us keeping this version with the addition of making sure that all form fields are disabled when the section isn't "active." We could even set the opacity of the inactive fieldsets to .5 or something like that in order to indicate that the controls are not available.

I find that the pickup location dropdowns are not populating, and the form doesn't prevent me from placing the hold without selecting one. That must be resolved.

I agree that a more detailed test plan would be helpful.
Comment 41 Nick Clemens (kidclamp) 2023-08-28 12:34:44 UTC
Created attachment 154881 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 42 Nick Clemens (kidclamp) 2023-08-28 12:34:46 UTC
Created attachment 154882 [details] [review]
Bug 30579: Add missing for attribute on label
Comment 43 Nick Clemens (kidclamp) 2023-08-28 12:34:49 UTC
Created attachment 154883 [details] [review]
Bug 30579: Remove doubled notes field
Comment 44 Nick Clemens (kidclamp) 2023-08-28 14:43:40 UTC
(In reply to Marcel de Rooy from comment #39)
> I am not sure if this (simple) test plan covers all edge cases. 

Test plan updated

> Should this be marked as an enhancment?

Done

(In reply to Owen Leonard from comment #40)
> I could see us using a Bootstrap Collapse to show the options, with the
> "next available" section displayed by default.
> 
> I could also see us keeping this version with the addition of making sure
> that all form fields are disabled when the section isn't "active." We could
> even set the opacity of the inactive fieldsets to .5 or something like that
> in order to indicate that the controls are not available.

Follow-up patch will be attached

> I find that the pickup location dropdowns are not populating, and the form
> doesn't prevent me from placing the hold without selecting one. That must be
> resolved.

I cannot recreate, can you retest? Can you confirm API is loading content on other pages like http://localhost:8081/cgi-bin/koha/admin/branches.pl
Comment 45 Nick Clemens (kidclamp) 2023-08-28 14:46:28 UTC
Created attachment 154886 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)
Comment 46 Emily Lamancusa (emlam) 2023-09-14 21:04:29 UTC
I really like where this is going! I found a few more bugs in testing, but it seems to be nearly there.

The only two that are definite blockers for me are some odd behavior with hold policy limits:

- When trying to place a hold on an item group, I always got the prompt: "None of these items can normally be put on hold for this patron. Place hold?" although there should be nothing forbidding that hold. (with item group holds enabled and regardless of patron or AllowHoldPolicyOverride value)

- The circ rule for maximum number of items allowed per record was not enforced for item-level holds. i.e. with a limit of 2, it was possible to place 3 item-level holds for the same patron even if overrides were not allowed. If overrides are allowed, there should be a warning at the top of the page, but the warning does not appear for a patron with the max number of item-level holds.


Not necessarily blockers but would be nice to fix:

- The select2's for pickup location are not actually disabled when the fieldset is disabled. (They are greyed out, and the data selected is not used, but they're still clickable.) Is this an oversight, or is it an unfortunate characteristic of select2's? (not necessarily a blocker for me because the visual cue and actual processing of data are correct)

- When enforcing that a patron with an existing item-level hold may only place item-level holds, the heading for item-level holds says (Required) in bright red. Same thing for item group holds. However, the heading for record-level holds does not say (Required). (this is the case in master, but if it's easy to add in here for consistency, I'd say do it)
Comment 47 Nick Clemens (kidclamp) 2023-10-25 12:15:03 UTC
Created attachment 157787 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 48 Nick Clemens (kidclamp) 2023-10-25 12:15:06 UTC
Created attachment 157788 [details] [review]
Bug 30579: Add missing for attribute on label
Comment 49 Nick Clemens (kidclamp) 2023-10-25 12:15:09 UTC
Created attachment 157789 [details] [review]
Bug 30579: Remove doubled notes field
Comment 50 Nick Clemens (kidclamp) 2023-10-25 12:15:12 UTC
Created attachment 157790 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)
Comment 51 Nick Clemens (kidclamp) 2023-10-25 12:15:15 UTC
Created attachment 157791 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options
Comment 52 Nick Clemens (kidclamp) 2023-10-25 12:15:18 UTC
Created attachment 157792 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required
Comment 53 Nick Clemens (kidclamp) 2023-10-25 12:19:31 UTC
(In reply to Emily Lamancusa from comment #46)
> I really like where this is going! I found a few more bugs in testing, but
> it seems to be nearly there.
> 
> The only two that are definite blockers for me are some odd behavior with
> hold policy limits:
> 
> - When trying to place a hold on an item group, I always got the prompt:
> "None of these items can normally be put on hold for this patron. Place
> hold?" although there should be nothing forbidding that hold. (with item
> group holds enabled and regardless of patron or AllowHoldPolicyOverride
> value)

Button updated to fix this
> 
> - The circ rule for maximum number of items allowed per record was not
> enforced for item-level holds. i.e. with a limit of 2, it was possible to
> place 3 item-level holds for the same patron even if overrides were not
> allowed. If overrides are allowed, there should be a warning at the top of
> the page, but the warning does not appear for a patron with the max number
> of item-level holds.

I don't recreate this one
> 
> 
> Not necessarily blockers but would be nice to fix:
> 
> - The select2's for pickup location are not actually disabled when the
> fieldset is disabled. 

I don't see a way to accomplish this, I think it's just select2

> - When enforcing that a patron with an existing item-level hold may only
> place item-level holds, the heading for item-level holds says (Required) in
> bright red. 

Added
Comment 54 Emily Lamancusa (emlam) 2023-10-26 17:28:42 UTC
Created attachment 157937 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 55 Emily Lamancusa (emlam) 2023-10-26 17:28:45 UTC
Created attachment 157938 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 56 Emily Lamancusa (emlam) 2023-10-26 17:28:48 UTC
Created attachment 157939 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 57 Emily Lamancusa (emlam) 2023-10-26 17:28:51 UTC
Created attachment 157940 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 58 Emily Lamancusa (emlam) 2023-10-26 17:28:53 UTC
Created attachment 157941 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 59 Emily Lamancusa (emlam) 2023-10-26 17:28:56 UTC
Created attachment 157942 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 60 Emily Lamancusa (emlam) 2023-10-26 17:28:59 UTC
Created attachment 157943 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 61 Emily Lamancusa (emlam) 2023-10-26 17:29:21 UTC
Awesome, just tested the fixes and they all seem to be working!
Adding a tiny follow-up patch to fix what I assume is a copy/paste error, though.

In the "Use same conditions..." follow-up, shouldn't the button id still be "hold_grp_btn" though? (the patch changed it to "hold_any_btn")

> > - The circ rule for maximum number of items allowed per record was not
> > enforced for item-level holds. i.e. with a limit of 2, it was possible to
> > place 3 item-level holds for the same patron even if overrides were not
> > allowed. If overrides are allowed, there should be a warning at the top of
> > the page, but the warning does not appear for a patron with the max number
> > of item-level holds.
> 
> I don't recreate this one

I'm not recreating it anymore either. Maybe it was just something weird on my end.

I don't follow Koha's template logic well enough yet to be confident QAing this one, but I added an additional sign-off since I confirm everything worked as expected in my testing.
Comment 62 Nick Clemens (kidclamp) 2023-10-27 13:32:38 UTC
(In reply to Emily Lamancusa from comment #61)
> Awesome, just tested the fixes and they all seem to be working!
> Adding a tiny follow-up patch to fix what I assume is a copy/paste error,
> though.
> 
> In the "Use same conditions..." follow-up, shouldn't the button id still be
> "hold_grp_btn" though? (the patch changed it to "hold_any_btn")

Yes, lazy copy/pasta error, thank you for follow-up :-)
Comment 63 Nick Clemens (kidclamp) 2024-03-15 18:04:13 UTC
Created attachment 163260 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 64 Nick Clemens (kidclamp) 2024-03-15 18:04:16 UTC
Created attachment 163261 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 65 Nick Clemens (kidclamp) 2024-03-15 18:04:20 UTC
Created attachment 163262 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 66 Nick Clemens (kidclamp) 2024-03-15 18:04:23 UTC
Created attachment 163263 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 67 Nick Clemens (kidclamp) 2024-03-15 18:04:26 UTC
Created attachment 163264 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 68 Nick Clemens (kidclamp) 2024-03-15 18:04:29 UTC
Created attachment 163265 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 69 Nick Clemens (kidclamp) 2024-03-15 18:04:32 UTC
Created attachment 163266 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 70 Nick Clemens (kidclamp) 2024-03-15 18:04:35 UTC
Created attachment 163267 [details] [review]
Bug 30579: (follow-up) Remove unnecessary nested form elements
Comment 71 Hammat wele 2024-04-17 04:26:40 UTC
This patch and bz31579 have different logics

This patch completely changes the form, with this patch you can either use “Hold next available item” (containing the “Pickup at” field), or “Place to hold on a specific item” since selecting one disables the other.

However, bz31579 needs the “Pickup at” and “Place a hold on a specific item” fields to all be active since we configure the “Place a hold on a specific item” fields according to the value chosen in the field “Pickup at” which means that it is impossible to use both patches at the same time, we will have to make a choice between the two.
Comment 72 Martin Renvoize (ashimema) 2024-05-01 15:45:56 UTC
Poke me in mattermost or slack when this is rebased and I'll jump straight on it.
Comment 73 Nick Clemens (kidclamp) 2024-05-01 16:12:04 UTC
Created attachment 166012 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 74 Nick Clemens (kidclamp) 2024-05-01 16:12:07 UTC
Created attachment 166013 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 75 Nick Clemens (kidclamp) 2024-05-01 16:12:10 UTC
Created attachment 166014 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 76 Nick Clemens (kidclamp) 2024-05-01 16:12:14 UTC
Created attachment 166015 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 77 Nick Clemens (kidclamp) 2024-05-01 16:12:17 UTC
Created attachment 166016 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 78 Nick Clemens (kidclamp) 2024-05-01 16:12:20 UTC
Created attachment 166017 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 79 Nick Clemens (kidclamp) 2024-05-01 16:12:23 UTC
Created attachment 166018 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 80 Nick Clemens (kidclamp) 2024-05-01 16:12:26 UTC
Created attachment 166019 [details] [review]
Bug 30579: (follow-up) Remove unnecessary nested form elements
Comment 81 Nick Clemens (kidclamp) 2024-05-01 16:13:19 UTC
Looks like there was an addition when using item groups to list the items and to list (disabled) groups with no items - so should define an item group with no items for testing
Comment 82 Martin Renvoize (ashimema) 2024-05-02 09:39:34 UTC
Created attachment 166034 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 83 Martin Renvoize (ashimema) 2024-05-02 09:39:38 UTC
Created attachment 166035 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 84 Martin Renvoize (ashimema) 2024-05-02 09:39:42 UTC
Created attachment 166036 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 85 Martin Renvoize (ashimema) 2024-05-02 09:39:46 UTC
Created attachment 166037 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 86 Martin Renvoize (ashimema) 2024-05-02 09:39:51 UTC
Created attachment 166038 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Martin Renvoize (ashimema) 2024-05-02 09:39:55 UTC
Created attachment 166039 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Martin Renvoize (ashimema) 2024-05-02 09:39:59 UTC
Created attachment 166040 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Martin Renvoize (ashimema) 2024-05-02 09:40:03 UTC
Created attachment 166041 [details] [review]
Bug 30579: (follow-up) Remove unnecessary nested form elements

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Martin Renvoize (ashimema) 2024-05-02 09:40:49 UTC
This is a great improvement. All working as described and no regressions found.

Tests all passing, QA script happy

Passing QA
Comment 91 Nick Clemens (kidclamp) 2024-05-02 16:10:49 UTC
Created attachment 166074 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Nick Clemens (kidclamp) 2024-05-02 16:10:53 UTC
Created attachment 166075 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Nick Clemens (kidclamp) 2024-05-02 16:10:56 UTC
Created attachment 166076 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 94 Nick Clemens (kidclamp) 2024-05-02 16:11:00 UTC
Created attachment 166077 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 95 Nick Clemens (kidclamp) 2024-05-02 16:11:04 UTC
Created attachment 166078 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 96 Nick Clemens (kidclamp) 2024-05-02 16:11:07 UTC
Created attachment 166079 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 97 Nick Clemens (kidclamp) 2024-05-02 16:11:11 UTC
Created attachment 166080 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 98 Nick Clemens (kidclamp) 2024-05-02 16:11:14 UTC
Created attachment 166081 [details] [review]
Bug 30579: (follow-up) Remove unnecessary nested form elements

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 99 Nick Clemens (kidclamp) 2024-05-02 16:11:18 UTC
Created attachment 166082 [details] [review]
Bug 30579: Fix rebase issue
Comment 100 Katrin Fischer 2024-05-03 15:24:42 UTC
This one is for Nick's hair...
Comment 101 Katrin Fischer 2024-05-03 15:42:46 UTC
Tiny bit torn about this one, it makes things clearer, but it adds a click... but yes, I can see why we are doing this. Still... sadly found some blockers that prevent me from pushing this right now. Please follow-up quickly!

If not marked as blocker, it's also good for a later follow-up.

1) Standardize headings

We have:
Hold next available item
Place a hold on a specific item 

We could do:
Hold on next available item
Hold on a specific item

OR
Place hold on next available item
Place hold on a specific item

Tend to the first, because it's a little shorter.

2) Usability

I know that I can click on the text instead of into the radio box, but from the formatting that is not visually clear. Could we think about replacing the checkboxes for an element that makes for a bigger "click target" visually?

3) Multi-holds result in a CSRF error (blocker)

Programming error - No CSRF token passed for POST http://localhost:8081/intranet/reserve/placerequest.pl (referer: http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=262&amp;biblionumber=5&amp;biblionumber=13&amp;biblionumber=146&amp;biblionumber=191&amp;biblionumber=11&amp;biblionumber=126&amp;biblionumber=139&amp;biblionumber=165&amp;biblionumber=255&amp;biblionumber=115&amp;multi_hold=1&borrowernumber=19)

4) Multi-holds missing pick-up location and other options (blocker)

If you do a lot of holds at once, the pick-up location at the top allowed you to set them all at once. now this option and others, like "hold expires on" and "notes" are completely gone.
Comment 102 Andrew Fuerste-Henry 2024-05-03 16:01:54 UTC
(In reply to Katrin Fischer from comment #101)
> 4) Multi-holds missing pick-up location and other options (blocker)
> 
> If you do a lot of holds at once, the pick-up location at the top allowed
> you to set them all at once. now this option and others, like "hold expires
> on" and "notes" are completely gone.

I can't recreate this. Or maybe it's more a question of clarity of interface? Anything I set in the top "Hold details" section of the page is applied to all holds created, regardless of choices lower in the page. When I entered notes and hold expiration date, those values were applied to all holds created via multi-hold, just as they were applied to individual bib- or item-level holds.
Comment 103 Katrin Fischer 2024-05-03 16:05:29 UTC
(In reply to Andrew Fuerste-Henry from comment #102)
> (In reply to Katrin Fischer from comment #101)
> > 4) Multi-holds missing pick-up location and other options (blocker)
> > 
> > If you do a lot of holds at once, the pick-up location at the top allowed
> > you to set them all at once. now this option and others, like "hold expires
> > on" and "notes" are completely gone.
> 
> I can't recreate this. Or maybe it's more a question of clarity of
> interface? Anything I set in the top "Hold details" section of the page is
> applied to all holds created, regardless of choices lower in the page. When
> I entered notes and hold expiration date, those values were applied to all
> holds created via multi-hold, just as they were applied to individual bib-
> or item-level holds.

Maybe I should have been more clear: I am placing multiple holds on different records by selecting them in the staff interface result list.
The hold details section is completely missing on top of the table.

Can you check if that is the same thing you were looking at?
Comment 104 Andrew Fuerste-Henry 2024-05-03 16:12:25 UTC
(In reply to Katrin Fischer from comment #103)
> Can you check if that is the same thing you were looking at?

It is not! I thought you meant placing more than one hold at a time on the same record.

I can confirm that the interface to place holds on multiple different records at once currently includes notes, a selection for pickup branch, expiration date, and non-priority toggle. With this patch, none of those things show.
Comment 105 Katrin Fischer 2024-05-03 16:35:43 UTC
I know people are keen on this from the activity - it's still on the table, but we will need to hurry a little.
Comment 106 Nick Clemens (kidclamp) 2024-05-07 12:36:02 UTC
Created attachment 166264 [details] [review]
Bug 30579: Disentangle multi-hold and single bib forms

This patch alters the structure of the hold request page to make it clear
that "Hold next available", "Hold item group", and "Hold specific item" are
mutually-exclusive options.

While there is some duplication from this, it makes the sections easier to read
and allows for more variation in the two forms

To test:
 1 - Find a bib with multiple items
 2 - Enable item groups and item group holds in system preferences
 3 - Load the records detail page
 4 - Add an item group on the item groups tab
 5 - Select two items and add to the group
 6 - Click the 'Holds' tab and search for/select a patron
 7 - Confirm the three levels of holds are clear
 8 - Confirm checking the radio next to one disables the others
 9 - Check 'Hold next available item from an item group'
10 - Do not select an item group
11 - Click 'Place hold' and confirm you are notified of need to select an item group
     NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold
12 - Click 'Place hold on a specific item' - but do not select an item
13 - Click place hold and confirm there is an alert and you cannot continue
14 - Click 'Hold next available item' and place hold
15 - Hold is successfully placed
16 - Place another hold for the same patron
17 - Only the 'Hold next available item' form is enabled
18 - Confirm you cannot switch hold type
19 - Place hold
20 - Select a new patron and place an item group hold
21 - Select the same patron and place another hold - you are forced to place an item group hold
22 - Select a new patron and place a hold on a specific item
23 - Place a second hold, confirm you can only place it on a specific item

Signed-off-by: hinemoea <hinemoea@inlibro.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 107 Nick Clemens (kidclamp) 2024-05-07 12:36:06 UTC
Created attachment 166265 [details] [review]
Bug 30579: Add missing for attribute on label

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 108 Nick Clemens (kidclamp) 2024-05-07 12:36:09 UTC
Created attachment 166266 [details] [review]
Bug 30579: Remove doubled notes field

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 109 Nick Clemens (kidclamp) 2024-05-07 12:36:12 UTC
Created attachment 166267 [details] [review]
Bug 30579: (follow-up) Add fieldsets and style disabled elements

This patch surrounds each section with a fieldset which allow enabling/disabling
all the elements in one go. CSS is added to make it clear which fields are disabled.

Button for item specific is moved form the multi-hold section

Javascript is simplified to use the new fieldsets

To test:
1 - Repeate test plan on earlier commit
2 - Additionally, search and select multiple records and place hold
3 - Confirm multi-hold placing works :-)

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Nick Clemens (kidclamp) 2024-05-07 12:36:15 UTC
Created attachment 166268 [details] [review]
Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 111 Nick Clemens (kidclamp) 2024-05-07 12:36:19 UTC
Created attachment 166269 [details] [review]
Bug 30579: (follow-up) Add required label and warning when hold levels required

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 112 Nick Clemens (kidclamp) 2024-05-07 12:36:22 UTC
Created attachment 166270 [details] [review]
Bug 30579: (follow-up) fix button id

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 113 Nick Clemens (kidclamp) 2024-05-07 12:36:25 UTC
Created attachment 166271 [details] [review]
Bug 30579: (follow-up) Remove unnecessary nested form elements

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 114 Nick Clemens (kidclamp) 2024-05-07 12:36:28 UTC
Created attachment 166272 [details] [review]
Bug 30579: Fix rebase issue
Comment 115 Nick Clemens (kidclamp) 2024-05-07 12:36:32 UTC
Created attachment 166273 [details] [review]
Bug 30579: (follow-up) Fix rebase of multi holds form and add CSRF

This patch restores missing elemtns to multi-hold form and adds CSRF variables
Comment 116 Nick Clemens (kidclamp) 2024-05-07 12:36:35 UTC
Created attachment 166274 [details] [review]
Bug 30579: (follow-up) Standardize labels:

This patch changes the heading for specific item holds, so the three options are now:

 Hold next available item
 Hold next available item from an item group
 Hold a specific item
Comment 117 Nick Clemens (kidclamp) 2024-05-07 12:36:39 UTC
Created attachment 166275 [details] [review]
Bug 30579: (follow-up) Style selections as buttons
Comment 118 Katrin Fischer 2024-05-07 13:47:04 UTC
I am not persuaded by the current "design" of the radiobox buttons, but maybe we can get some help there if we ask really nicely as a follow-up?
Comment 119 Katrin Fischer 2024-05-07 13:55:54 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 120 Lucas Gass (lukeg) 2024-05-07 14:01:32 UTC
(In reply to Katrin Fischer from comment #118)
> I am not persuaded by the current "design" of the radiobox buttons, but
> maybe we can get some help there if we ask really nicely as a follow-up?

Agree. Also, inputs as children of a button creates invalid HTML.
Comment 121 Fridolin Somers 2024-05-24 14:45:00 UTC
Not backported to 23.11.x
Comment 122 Martin Renvoize (ashimema) 2024-06-14 08:31:02 UTC
I think the additional pieces were handled in follow-up bugs.. dropping the additional_work_needed flag