Bug 36775 - Option to place multiple holds on single bib should not be hidden when holds per record is unlimited
Summary: Option to place multiple holds on single bib should not be hidden when holds ...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Andrew Fuerste-Henry
QA Contact: Emily Lamancusa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-03 16:21 UTC by Andrew Fuerste-Henry
Modified: 2024-05-24 15:27 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 36775: Allow multiple holds on record when holds per record is unlimited (1.80 KB, patch)
2024-05-03 16:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36775: Allow multiple holds on record when holds per record is unlimited (1.85 KB, patch)
2024-05-03 18:44 UTC, David Nind
Details | Diff | Splinter Review
Bug 36775: Allow multiple holds on record when holds per record is unlimited (1.84 KB, patch)
2024-05-09 16:03 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36775: Allow multiple holds on record when holds per record is unlimited (1.95 KB, patch)
2024-05-09 16:42 UTC, Emily Lamancusa
Details | Diff | Splinter Review
Bug 36775: (QA follow-up) Remove max_length attribute (1.82 KB, patch)
2024-05-09 16:42 UTC, Emily Lamancusa
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2024-05-03 16:21:04 UTC
In request.tt, we tell the "Holds to place (count)" field to show only if holds_per_record > 1. That means it does not show when holds_per_record is null and therefore unlimited.
Comment 1 Andrew Fuerste-Henry 2024-05-03 16:43:20 UTC
Created attachment 166152 [details] [review]
Bug 36775: Allow multiple holds on record when holds per record is unlimited

To test:
1 set Holds Per Record to Unlimited
2 confirm hold placement interface does not include "Holds to place (count)"
3 apply patch, restart all
4 reload hold placement interface, confirm it does include "Holds to place (count)"
5 set Holds Per Record to 2
6 confirm Holds to place shows when placing a first hold for a patron, but not when placing a second
7 set Holds Per Record to 1
8 confirm Holds to Place does not show
9 sign off and eat a nice snack
Comment 2 David Nind 2024-05-03 18:44:53 UTC
Created attachment 166158 [details] [review]
Bug 36775: Allow multiple holds on record when holds per record is unlimited

To test:
1 set Holds Per Record to Unlimited
2 confirm hold placement interface does not include "Holds to place (count)"
3 apply patch, restart all
4 reload hold placement interface, confirm it does include "Holds to place (count)"
5 set Holds Per Record to 2
6 confirm Holds to place shows when placing a first hold for a patron, but not when placing a second
7 set Holds Per Record to 1
8 confirm Holds to Place does not show
9 sign off and eat a nice snack

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Emily Lamancusa 2024-05-09 15:44:22 UTC
Patch does not apply. :(
Comment 4 Andrew Fuerste-Henry 2024-05-09 16:00:59 UTC
(In reply to Emily Lamancusa from comment #3)
> Patch does not apply. :(

Sorry, you don't get the nice snack from the test plan :(
Comment 5 Andrew Fuerste-Henry 2024-05-09 16:03:35 UTC
Created attachment 166467 [details] [review]
Bug 36775: Allow multiple holds on record when holds per record is unlimited

To test:
1 set Holds Per Record to Unlimited
2 confirm hold placement interface does not include "Holds to place (count)"
3 apply patch, restart all
4 reload hold placement interface, confirm it does include "Holds to place (count)"
5 set Holds Per Record to 2
6 confirm Holds to place shows when placing a first hold for a patron, but not when placing a second
7 set Holds Per Record to 1
8 confirm Holds to Place does not show
9 sign off and eat a nice snack
Comment 6 Emily Lamancusa 2024-05-09 16:42:32 UTC
Created attachment 166468 [details] [review]
Bug 36775: Allow multiple holds on record when holds per record is unlimited

To test:
1 set Holds Per Record to Unlimited
2 confirm hold placement interface does not include "Holds to place (count)"
3 apply patch, restart all
4 reload hold placement interface, confirm it does include "Holds to place (count)"
5 set Holds Per Record to 2
6 confirm Holds to place shows when placing a first hold for a patron, but not when placing a second
7 set Holds Per Record to 1
8 confirm Holds to Place does not show
9 sign off and eat a nice snack

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 7 Emily Lamancusa 2024-05-09 16:42:34 UTC
Created attachment 166469 [details] [review]
Bug 36775: (QA follow-up) Remove max_length attribute

The existing code for "Holds to place (count)" sets the input field's
maxlength to remaining_holds_for_record, which does not do what we want
and makes the field unusable when the circulation rules allow unlimited
holds. Better to remove it.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 8 Emily Lamancusa 2024-05-09 16:47:27 UTC
Does tea count as a snack? :)
Comment 9 Katrin Fischer 2024-05-10 12:13:16 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 10 Andrew Fuerste-Henry 2024-05-10 15:37:33 UTC
(In reply to Emily Lamancusa from comment #8)
> Does tea count as a snack? :)

After some deliberation, I do not feel qualified to answer this question. We'll have to consult the broader community.
Comment 11 Fridolin Somers 2024-05-24 15:27:06 UTC
Does not apply on 23.11.x maybe some dependancy