Bug 15565 - Place multiple item-level holds at once for the same record
Summary: Place multiple item-level holds at once for the same record
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement with 40 votes (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
: 24309 (view as bug list)
Depends on: 14695 15486
Blocks: 15567
  Show dependency treegraph
 
Reported: 2016-01-13 13:10 UTC by Julian Maurice
Modified: 2024-03-12 15:28 UTC (History)
35 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Allow borrowers to place multiple item-level holds on a record in the staff interface and OPAC. Item radio buttons are replaced with checkboxes. This feature is enabled/disabled via the system preference "DisplayMultiItemHolds".
Version(s) released in:


Attachments
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (36.02 KB, patch)
2016-02-01 17:23 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.00 KB, patch)
2016-02-02 13:12 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - CSS changes (80.75 KB, patch)
2016-02-02 13:12 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (36.99 KB, patch)
2016-04-08 11:14 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bz 15565 - CSS changes (80.77 KB, patch)
2016-04-08 11:15 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.23 KB, patch)
2016-04-19 10:16 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.41 KB, patch)
2016-04-19 10:17 UTC, Julian Maurice
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-12 08:17 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - CSS changes (80.77 KB, patch)
2016-10-12 08:17 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.26 KB, patch)
2016-10-12 08:17 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.41 KB, patch)
2016-10-12 08:17 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (941 bytes, patch)
2016-10-14 07:56 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (995 bytes, patch)
2016-10-14 12:20 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-14 12:26 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - CSS changes (80.77 KB, patch)
2016-10-14 12:26 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.26 KB, patch)
2016-10-14 12:26 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.41 KB, patch)
2016-10-14 12:26 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (995 bytes, patch)
2016-10-14 12:26 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-14 12:51 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-14 12:53 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-14 12:58 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.65 KB, patch)
2016-10-14 13:03 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - CSS changes (80.77 KB, patch)
2016-10-14 13:04 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.26 KB, patch)
2016-10-14 13:06 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.46 KB, patch)
2016-10-14 13:06 UTC, Danielle Elder
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.03 KB, patch)
2016-10-14 13:08 UTC, Danielle Elder
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.35 KB, patch)
2017-03-20 08:09 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - CSS changes (81.47 KB, patch)
2017-03-20 08:09 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.26 KB, patch)
2017-03-20 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.46 KB, patch)
2017-03-20 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.03 KB, patch)
2017-03-20 08:10 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.50 KB, patch)
2017-03-23 16:17 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bz 15565 - CSS changes (81.62 KB, patch)
2017-03-23 16:17 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.40 KB, patch)
2017-03-23 16:17 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.61 KB, patch)
2017-03-23 16:17 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.24 KB, patch)
2017-03-23 16:17 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.57 KB, patch)
2017-06-23 12:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - CSS changes (82.01 KB, patch)
2017-06-23 12:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.40 KB, patch)
2017-06-23 12:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.61 KB, patch)
2017-06-23 12:48 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.24 KB, patch)
2017-06-23 12:49 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bz 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.40 KB, patch)
2017-08-29 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bz 15565 - CSS changes (82.01 KB, patch)
2017-08-29 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.40 KB, patch)
2017-08-29 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.61 KB, patch)
2017-08-29 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.24 KB, patch)
2017-08-29 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.46 KB, patch)
2017-10-27 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15565 - CSS changes (82.07 KB, patch)
2017-10-27 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.46 KB, patch)
2017-10-27 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.67 KB, patch)
2017-10-27 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.29 KB, patch)
2017-10-27 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.47 KB, patch)
2018-02-15 11:13 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565 - CSS changes (84.98 KB, patch)
2018-02-15 11:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.46 KB, patch)
2018-02-15 11:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.67 KB, patch)
2018-02-15 11:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.29 KB, patch)
2018-02-15 11:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565 - remove use of GetBiblionumberFromItemnumber (968 bytes, patch)
2018-02-15 11:14 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 15565 - Place multiple holds at once for the same record on staff and OPAC. (37.53 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565 - CSS changes (85.04 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.35 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565 - remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-03-12 11:04 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (94.43 KB, patch)
2018-08-29 05:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: CSS changes (77.18 KB, patch)
2018-08-29 05:59 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-08-29 06:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-08-29 06:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.74 KB, patch)
2018-08-29 06:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-08-29 06:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.41 KB, patch)
2018-08-29 06:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.29 KB, patch)
2018-08-29 13:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.37 KB, patch)
2018-08-30 06:18 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: CSS changes (78.41 KB, patch)
2018-09-04 21:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-09-04 21:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: CSS changes (78.41 KB, patch)
2018-09-04 21:10 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-09-04 21:11 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-09-04 21:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.74 KB, patch)
2018-09-04 21:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-09-04 21:13 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.41 KB, patch)
2018-09-04 21:13 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.29 KB, patch)
2018-09-04 21:14 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.37 KB, patch)
2018-09-04 21:15 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (94.51 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: CSS changes (78.06 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.53 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.75 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.42 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.30 KB, patch)
2018-09-14 19:27 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.38 KB, patch)
2018-09-14 19:28 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (94.48 KB, patch)
2018-09-14 22:34 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: CSS changes (78.04 KB, patch)
2018-09-14 22:34 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-09-14 22:34 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.74 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.41 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.29 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.42 KB, patch)
2018-09-14 22:35 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: CSS changes (89.73 KB, patch)
2018-10-28 18:42 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.52 KB, patch)
2018-10-28 18:43 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.73 KB, patch)
2018-10-28 18:43 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.74 KB, patch)
2018-10-28 18:44 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.01 KB, patch)
2018-10-28 18:44 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.41 KB, patch)
2018-10-28 18:45 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.29 KB, patch)
2018-10-28 18:46 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.42 KB, patch)
2018-10-28 18:46 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (15.59 KB, patch)
2018-10-28 18:47 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.56 KB, patch)
2018-10-28 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic (3.46 KB, patch)
2018-10-28 23:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (94.56 KB, patch)
2018-10-29 21:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: CSS changes (89.80 KB, patch)
2018-10-29 21:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.58 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.79 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.80 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.07 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.48 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.36 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.49 KB, patch)
2018-10-29 21:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (15.67 KB, patch)
2018-10-29 21:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.62 KB, patch)
2018-10-29 21:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic (3.52 KB, patch)
2018-10-29 21:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.58 KB, patch)
2018-11-10 12:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.78 KB, patch)
2018-11-10 12:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.79 KB, patch)
2018-11-10 12:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.06 KB, patch)
2018-11-10 12:01 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.47 KB, patch)
2018-11-10 12:02 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.35 KB, patch)
2018-11-10 12:02 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.48 KB, patch)
2018-11-10 12:03 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (15.65 KB, patch)
2018-11-10 12:04 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.61 KB, patch)
2018-11-10 12:05 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic (3.51 KB, patch)
2018-11-10 12:05 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Added handling for holds_per_day and respecting AllowHoldPolicyOverride (99.16 KB, patch)
2018-11-10 12:06 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565 (follow-up): Removed leftover JS debug (1.51 KB, patch)
2018-11-10 12:19 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio (12.90 KB, patch)
2018-11-12 10:11 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio (12.89 KB, patch)
2018-11-12 10:16 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (39.25 KB, patch)
2018-12-18 01:54 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.58 KB, patch)
2018-12-18 01:55 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.78 KB, patch)
2018-12-18 01:56 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.79 KB, patch)
2018-12-18 01:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.06 KB, patch)
2018-12-18 01:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.47 KB, patch)
2018-12-18 01:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.35 KB, patch)
2018-12-18 01:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.48 KB, patch)
2018-12-18 01:59 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (15.65 KB, patch)
2018-12-18 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.61 KB, patch)
2018-12-18 02:00 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio (24.66 KB, patch)
2018-12-18 02:17 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Changes to maximum hold allowed logic (24.79 KB, patch)
2018-12-18 02:20 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Place multiple holds from opac-search.pl (5.69 KB, patch)
2018-12-19 00:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Changes to maximum hold allowed logic (25.77 KB, patch)
2018-12-19 03:31 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: (follow-up) Place multiple holds from opac-search.pl (6.95 KB, patch)
2018-12-19 03:32 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (39.25 KB, patch)
2019-01-31 01:03 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.58 KB, patch)
2019-01-31 01:03 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.78 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.79 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.06 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.47 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.35 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.48 KB, patch)
2019-01-31 01:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (14.92 KB, patch)
2019-01-31 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.61 KB, patch)
2019-01-31 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Changes to maximum hold allowed logic (24.67 KB, patch)
2019-01-31 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Place multiple holds from opac-search.pl (5.57 KB, patch)
2019-01-31 01:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) fixing typo that breaks placing a hold (674 bytes, patch)
2019-01-31 02:13 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Place multiple holds at once for the same record on staff and OPAC. (39.81 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Use item's biblionumber only on item-level request (1.58 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.78 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Alert librarian that maximum holds per record is reached (1.63 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Remove use of GetBiblionumberFromItemnumber (1.06 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC (6.47 KB, patch)
2019-04-15 02:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl (2.35 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds (5.48 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Removed leftover debug code and added missing filters (15.04 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (QA follow-up) Fix some last filters (1.61 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Changes to maximum hold allowed logic (24.68 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) Place multiple holds from opac-search.pl (5.57 KB, patch)
2019-04-15 02:48 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: (follow-up) fixing typo that breaks placing a hold (674 bytes, patch)
2019-04-15 02:49 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Koha::Biblio->allowed_holds and tests (4.43 KB, patch)
2021-02-22 05:10 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff client (14.83 KB, patch)
2021-02-22 05:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on OPAC (8.00 KB, patch)
2021-02-22 05:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Koha::Biblio->allowed_holds and tests (4.48 KB, patch)
2021-02-22 06:42 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff client (14.88 KB, patch)
2021-02-22 06:43 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on OPAC (8.06 KB, patch)
2021-02-22 06:43 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Koha::Biblio->allowed_holds and tests (4.47 KB, patch)
2021-05-03 01:32 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff client (14.91 KB, patch)
2021-05-03 01:32 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on OPAC (8.06 KB, patch)
2021-05-03 01:32 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Koha::Biblio->allowed_holds and tests (4.52 KB, patch)
2021-05-06 19:14 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on OPAC (7.25 KB, patch)
2021-05-06 19:15 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff client (15.14 KB, patch)
2021-05-06 19:15 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 15565: Work in progress, calculating holds remaining (13.97 KB, patch)
2021-06-15 01:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15565: Allow multiple hold for multiple bib in OPAC (2.91 KB, patch)
2021-06-18 06:09 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff interface (13.59 KB, patch)
2024-02-13 00:55 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Tidy reserve/placerequest.pl (3.08 KB, patch)
2024-02-13 00:55 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one or more records in OPAC (3.84 KB, patch)
2024-02-13 00:55 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Add 'DisplayMultiItemHolds' system preference to toggle display of multi item holds (7.78 KB, patch)
2024-02-13 00:55 UTC, David Cook
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff interface (13.77 KB, patch)
2024-03-11 19:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15565: Tidy reserve/placerequest.pl (3.20 KB, patch)
2024-03-11 19:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one or more records in OPAC (3.84 KB, patch)
2024-03-11 19:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15565: Add 'DisplayMultiItemHolds' system preference to toggle display of multi item holds (7.79 KB, patch)
2024-03-11 19:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Attachment to Bug 15565 - Place multiple item-level holds at once for the same record (13.83 KB, patch)
2024-03-12 15:26 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 15565: Place multiple holds on one record on staff interface (13.83 KB, patch)
2024-03-12 15:27 UTC, Lisette Scheer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2016-01-13 13:10:53 UTC
Bug 14695 allows to place multiple item-level holds for the same record.
It would be nice to also have the ability to place multiple title-level holds for the same record.
An example use case could be a teacher that wants to borrow multiple copies of the same book for his students.
Comment 1 Alex Arnaud 2016-01-27 16:53:33 UTC
Bug 14695 allows to place multiple holds for the same record. It would be nice to choose the numbers of items (or select several items) at once instead of making X times the same operation. We want this behavior on staff and OPAC.
Comment 2 Alex Arnaud 2016-02-01 17:23:40 UTC Comment hidden (obsolete)
Comment 3 Alex Arnaud 2016-02-02 13:12:42 UTC Comment hidden (obsolete)
Comment 4 Alex Arnaud 2016-02-02 13:12:48 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens 2016-03-04 22:12:29 UTC
/home/vagrant/kohaclone/.git/rebase-apply/patch:432: trailing whitespace.
my @biblionumbers = ref($query->param('biblionumbers')) eq 'Array' 
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 6 Benjamin Rokseth 2016-04-08 11:14:52 UTC Comment hidden (obsolete)
Comment 7 Benjamin Rokseth 2016-04-08 11:15:01 UTC Comment hidden (obsolete)
Comment 8 Benjamin Rokseth 2016-04-08 11:15:32 UTC
Rebased against master
Comment 9 Benjamin Rokseth 2016-04-08 12:18:51 UTC
Fails in intra on placing any number of holds with:

/cgi-bin/koha/reserve/placerequest.pl
DBIx::Class::Storage::DBI::_dbh_execute(): Column 'biblionumber' cannot be null at /kohadev/kohaclone/Koha/Object.pm line 109

I notice that in the request, param multi_holds is not present
Comment 10 Julian Maurice 2016-04-19 10:16:59 UTC Comment hidden (obsolete)
Comment 11 Julian Maurice 2016-04-19 10:17:03 UTC Comment hidden (obsolete)
Comment 12 Julian Maurice 2016-04-19 10:17:51 UTC
(In reply to Benjamin Rokseth from comment #9)
> Fails in intra on placing any number of holds with:
> 
> /cgi-bin/koha/reserve/placerequest.pl
> DBIx::Class::Storage::DBI::_dbh_execute(): Column 'biblionumber' cannot be
> null at /kohadev/kohaclone/Koha/Object.pm line 109
attachment 50392 [details] [review] fixes that
Comment 13 Benjamin Rokseth 2016-05-06 11:20:53 UTC
1 out of 7 hunks FAILED -- saving rejects to file opac/opac-reserve.pl.rej
...
2 out of 3 hunks FAILED -- saving rejects to file reserve/placerequest.pl.rej

seems to conflict with Bug 15533 - Allow patrons and librarians to select itemtype when placing holds pushed recently?
Comment 14 Jonathan Druart 2016-06-08 14:58:53 UTC
Alex, could you confirm that you will abandon your patches in favor of the ones on bug 14695?
Comment 15 Jonathan Druart 2016-06-08 14:59:55 UTC
(In reply to Jonathan Druart from comment #14)
> Alex, could you confirm that you will abandon your patches in favor of the
> ones on bug 14695?

Sorry, misread, forget that.
Comment 16 Alex Arnaud 2016-10-12 08:17:01 UTC Comment hidden (obsolete)
Comment 17 Alex Arnaud 2016-10-12 08:17:11 UTC Comment hidden (obsolete)
Comment 18 Alex Arnaud 2016-10-12 08:17:21 UTC Comment hidden (obsolete)
Comment 19 Alex Arnaud 2016-10-12 08:17:30 UTC Comment hidden (obsolete)
Comment 20 Alex Arnaud 2016-10-12 13:04:24 UTC
Patches have been rebased on master. Please test.
Comment 21 Séverine Queune 2016-10-12 13:33:55 UTC
There is no alert when exceeding the maximum of holds per record, but Koha places holds only in the number of items fixed in the circ rules.
I think it should react like when maximum holds allowed is reached : checkbox can't be selected.
Comment 22 Alex Arnaud 2016-10-14 07:56:47 UTC Comment hidden (obsolete)
Comment 23 Danielle Elder 2016-10-14 12:20:15 UTC Comment hidden (obsolete)
Comment 24 Danielle Elder 2016-10-14 12:26:11 UTC Comment hidden (obsolete)
Comment 25 Danielle Elder 2016-10-14 12:26:19 UTC Comment hidden (obsolete)
Comment 26 Danielle Elder 2016-10-14 12:26:25 UTC Comment hidden (obsolete)
Comment 27 Danielle Elder 2016-10-14 12:26:31 UTC Comment hidden (obsolete)
Comment 28 Danielle Elder 2016-10-14 12:26:39 UTC Comment hidden (obsolete)
Comment 29 Danielle Elder 2016-10-14 12:51:23 UTC Comment hidden (obsolete)
Comment 30 Danielle Elder 2016-10-14 12:53:20 UTC Comment hidden (obsolete)
Comment 31 Danielle Elder 2016-10-14 12:58:20 UTC Comment hidden (obsolete)
Comment 32 Danielle Elder 2016-10-14 13:03:23 UTC Comment hidden (obsolete)
Comment 33 Danielle Elder 2016-10-14 13:04:39 UTC Comment hidden (obsolete)
Comment 34 Danielle Elder 2016-10-14 13:06:03 UTC Comment hidden (obsolete)
Comment 35 Danielle Elder 2016-10-14 13:06:29 UTC Comment hidden (obsolete)
Comment 36 Danielle Elder 2016-10-14 13:08:13 UTC
Sorry, devbox trouble with multiple sign-offs.  I did test and received the alert when I tried too many holds per record, but was able to place holds on multiple items per bib.
Comment 37 Danielle Elder 2016-10-14 13:08:28 UTC Comment hidden (obsolete)
Comment 38 Alex Arnaud 2017-03-20 08:09:42 UTC Comment hidden (obsolete)
Comment 39 Alex Arnaud 2017-03-20 08:09:52 UTC Comment hidden (obsolete)
Comment 40 Alex Arnaud 2017-03-20 08:10:05 UTC Comment hidden (obsolete)
Comment 41 Alex Arnaud 2017-03-20 08:10:16 UTC Comment hidden (obsolete)
Comment 42 Alex Arnaud 2017-03-20 08:10:30 UTC Comment hidden (obsolete)
Comment 43 Biblibre Sandboxes 2017-03-23 16:16:14 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 44 Biblibre Sandboxes 2017-03-23 16:17:19 UTC Comment hidden (obsolete)
Comment 45 Biblibre Sandboxes 2017-03-23 16:17:24 UTC Comment hidden (obsolete)
Comment 46 Biblibre Sandboxes 2017-03-23 16:17:28 UTC Comment hidden (obsolete)
Comment 47 Biblibre Sandboxes 2017-03-23 16:17:32 UTC Comment hidden (obsolete)
Comment 48 Biblibre Sandboxes 2017-03-23 16:17:36 UTC Comment hidden (obsolete)
Comment 49 Alex Arnaud 2017-06-23 12:48:23 UTC Comment hidden (obsolete)
Comment 50 Alex Arnaud 2017-06-23 12:48:34 UTC Comment hidden (obsolete)
Comment 51 Alex Arnaud 2017-06-23 12:48:45 UTC Comment hidden (obsolete)
Comment 52 Alex Arnaud 2017-06-23 12:48:55 UTC Comment hidden (obsolete)
Comment 53 Alex Arnaud 2017-06-23 12:49:04 UTC Comment hidden (obsolete)
Comment 54 Owen Leonard 2017-08-29 17:52:24 UTC Comment hidden (obsolete)
Comment 55 Owen Leonard 2017-08-29 17:52:29 UTC Comment hidden (obsolete)
Comment 56 Owen Leonard 2017-08-29 17:52:33 UTC Comment hidden (obsolete)
Comment 57 Owen Leonard 2017-08-29 17:52:38 UTC Comment hidden (obsolete)
Comment 58 Owen Leonard 2017-08-29 17:52:43 UTC Comment hidden (obsolete)
Comment 59 Liz Rea 2017-10-04 02:38:59 UTC
This still applies, could we get a QA to look at it please? Really good feature.

Cheers,
Liz
Comment 60 Kyle M Hall 2017-10-27 15:59:52 UTC Comment hidden (obsolete)
Comment 61 Kyle M Hall 2017-10-27 16:00:02 UTC Comment hidden (obsolete)
Comment 62 Kyle M Hall 2017-10-27 16:00:07 UTC Comment hidden (obsolete)
Comment 63 Kyle M Hall 2017-10-27 16:00:12 UTC Comment hidden (obsolete)
Comment 64 Kyle M Hall 2017-10-27 16:00:16 UTC Comment hidden (obsolete)
Comment 65 Jonathan Druart 2017-10-27 20:19:06 UTC
Undefined subroutine &CGI::Compile::ROOT::home_vagrant_kohaclone_opac_opac_2dreserve_2epl::GetBiblionumberFromItemnumber called at /home/vagrant/kohaclone/opac/opac-reserve.pl line 244.


This subroutine has been removed on bug 18277.
Comment 66 Liz Rea 2018-02-01 20:30:15 UTC
Hi,

Lots of conflicts applying this now.

Cheers,
Liz
Comment 67 Chris Cormack 2018-02-01 22:11:46 UTC
We'd really like this patch in, does Biblibre plan to work on it more, if not please let us know so we can perhaps try to get it fixed to pass QA ourselves
Comment 68 Alex Arnaud 2018-02-15 11:13:57 UTC Comment hidden (obsolete)
Comment 69 Alex Arnaud 2018-02-15 11:14:09 UTC Comment hidden (obsolete)
Comment 70 Alex Arnaud 2018-02-15 11:14:20 UTC Comment hidden (obsolete)
Comment 71 Alex Arnaud 2018-02-15 11:14:30 UTC Comment hidden (obsolete)
Comment 72 Alex Arnaud 2018-02-15 11:14:42 UTC Comment hidden (obsolete)
Comment 73 Alex Arnaud 2018-02-15 11:14:52 UTC Comment hidden (obsolete)
Comment 74 Benjamin Rokseth 2018-03-12 11:04:02 UTC Comment hidden (obsolete)
Comment 75 Benjamin Rokseth 2018-03-12 11:04:13 UTC Comment hidden (obsolete)
Comment 76 Benjamin Rokseth 2018-03-12 11:04:23 UTC Comment hidden (obsolete)
Comment 77 Benjamin Rokseth 2018-03-12 11:04:31 UTC Comment hidden (obsolete)
Comment 78 Benjamin Rokseth 2018-03-12 11:04:41 UTC Comment hidden (obsolete)
Comment 79 Benjamin Rokseth 2018-03-12 11:04:51 UTC Comment hidden (obsolete)
Comment 80 Katrin Fischer 2018-04-16 22:51:20 UTC
Hi Alex,

patch applies, QA script passes. Bit baffled there are no changes to C4/Koha and no unit tests.

Can you please amend the first patch with a proper test plan and fix the commit messages? (Bug XXXX:)
Comment 81 Katrin Fischer 2018-04-19 06:17:10 UTC
Ping?
Comment 82 Alex Buckley 2018-08-29 05:27:16 UTC
I have done the following changes to the patches on this bug report to try and get it moving along again:

(#1 and #2 address Katrin's notes in comment 80)
 
1. Amended the first patch to include a proper test plan

2. Fixed the commit messages of all commits to be in the format: (Bug: XXXX)

3. Rebased all commits against master

4. Reworked the css in the second patch because since that commit was written we have switched to scss.

5. Added an additional patch which fixes a few bugs when placing an item level hold from the OPAC. 

I will now attach these commits to the bug report.
Comment 83 Alex Buckley 2018-08-29 05:58:17 UTC Comment hidden (obsolete)
Comment 84 Alex Buckley 2018-08-29 05:59:26 UTC Comment hidden (obsolete)
Comment 85 Alex Buckley 2018-08-29 06:00:04 UTC Comment hidden (obsolete)
Comment 86 Alex Buckley 2018-08-29 06:00:28 UTC Comment hidden (obsolete)
Comment 87 Alex Buckley 2018-08-29 06:00:51 UTC Comment hidden (obsolete)
Comment 88 Alex Buckley 2018-08-29 06:01:17 UTC Comment hidden (obsolete)
Comment 89 Alex Buckley 2018-08-29 06:01:45 UTC Comment hidden (obsolete)
Comment 90 Alex Buckley 2018-08-29 13:26:01 UTC Comment hidden (obsolete)
Comment 91 Alex Buckley 2018-08-30 06:18:33 UTC Comment hidden (obsolete)
Comment 92 Cab Vinton 2018-09-02 15:15:30 UTC
Patch failed on PTFS sanbdbox 0:

Some problems occurred applying patches from bug 15565:
<h1>Something went wrong !</h1>Applying: Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
Applying: Bug 15565: CSS changes
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M       koha-tmpl/opac-tmpl/bootstrap/css/opac.css
M       koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/opac.css
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/css/opac.css
Patch failed at 0001 Bug 15565: CSS changes
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 15565 - Place multiple holds at once for the same record

78242 - Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
78243 - Bug 15565: CSS changes
78244 - Bug 15565: Use item's biblionumber only on item-level request
78245 - Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off
78246 - Bug 15565: Alert librarian that maximum holds per record is reached
78247 - Bug 15565: Remove use of GetBiblionumberFromItemnumber
78248 - Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC
78266 - Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl
78305 - Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-15565-CSS-changes-i7sD5P.patch .
Comment 93 Alex Buckley 2018-09-04 21:00:37 UTC Comment hidden (obsolete)
Comment 94 Alex Buckley 2018-09-04 21:01:32 UTC Comment hidden (obsolete)
Comment 95 Alex Buckley 2018-09-04 21:06:27 UTC
I am going to go through obsoleting the existing patches and attach rebased patches onto this bug report.
Comment 96 Alex Buckley 2018-09-04 21:10:15 UTC Comment hidden (obsolete)
Comment 97 Alex Buckley 2018-09-04 21:11:58 UTC Comment hidden (obsolete)
Comment 98 Alex Buckley 2018-09-04 21:12:31 UTC Comment hidden (obsolete)
Comment 99 Alex Buckley 2018-09-04 21:12:58 UTC Comment hidden (obsolete)
Comment 100 Alex Buckley 2018-09-04 21:13:24 UTC Comment hidden (obsolete)
Comment 101 Alex Buckley 2018-09-04 21:13:53 UTC Comment hidden (obsolete)
Comment 102 Alex Buckley 2018-09-04 21:14:40 UTC Comment hidden (obsolete)
Comment 103 Alex Buckley 2018-09-04 21:15:23 UTC Comment hidden (obsolete)
Comment 104 Alex Buckley 2018-09-04 21:17:54 UTC
The rebased patches should now apply cleanly on master (as of 5/9/2018), as shown below:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
Applying: Bug 15565: CSS changes
Applying: Bug 15565: Use item's biblionumber only on item-level request
Applying: Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off
Applying: Bug 15565: Alert librarian that maximum holds per record is reached
Applying: Bug 15565: Remove use of GetBiblionumberFromItemnumber
Applying: Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC
Applying: Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl
Applying: Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds
Comment 105 Lisette Scheer 2018-09-14 19:13:18 UTC
Applying: Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
Using index info to reconstruct a base tree...
M       koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
M       koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
M       opac/opac-reserve.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-reserve.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-15565-Place-multiple-holds-at-once-for-the-sam-kXB0M4.patch


Lisette
Comment 106 Chris Cormack 2018-09-14 19:27:08 UTC Comment hidden (obsolete)
Comment 107 Chris Cormack 2018-09-14 19:27:14 UTC Comment hidden (obsolete)
Comment 108 Chris Cormack 2018-09-14 19:27:21 UTC Comment hidden (obsolete)
Comment 109 Chris Cormack 2018-09-14 19:27:27 UTC Comment hidden (obsolete)
Comment 110 Chris Cormack 2018-09-14 19:27:33 UTC Comment hidden (obsolete)
Comment 111 Chris Cormack 2018-09-14 19:27:39 UTC Comment hidden (obsolete)
Comment 112 Chris Cormack 2018-09-14 19:27:47 UTC Comment hidden (obsolete)
Comment 113 Chris Cormack 2018-09-14 19:27:55 UTC Comment hidden (obsolete)
Comment 114 Chris Cormack 2018-09-14 19:28:01 UTC Comment hidden (obsolete)
Comment 115 Lisette Scheer 2018-09-14 22:34:42 UTC Comment hidden (obsolete)
Comment 116 Lisette Scheer 2018-09-14 22:34:48 UTC Comment hidden (obsolete)
Comment 117 Lisette Scheer 2018-09-14 22:34:54 UTC Comment hidden (obsolete)
Comment 118 Lisette Scheer 2018-09-14 22:35:02 UTC Comment hidden (obsolete)
Comment 119 Lisette Scheer 2018-09-14 22:35:12 UTC Comment hidden (obsolete)
Comment 120 Lisette Scheer 2018-09-14 22:35:21 UTC Comment hidden (obsolete)
Comment 121 Lisette Scheer 2018-09-14 22:35:28 UTC Comment hidden (obsolete)
Comment 122 Lisette Scheer 2018-09-14 22:35:35 UTC Comment hidden (obsolete)
Comment 123 Lisette Scheer 2018-09-14 22:35:41 UTC Comment hidden (obsolete)
Comment 124 Katrin Fischer 2018-10-28 10:40:36 UTC
Trying to work through this:

- I've rebased the patches (generated updated CSS file for the second patch)
- I've tested the current behaviour and am a little confused: It appears this feature is already available for the staff side, so this is about adding it to the OPAC side. I've adjusted the bug title. Correct? 

TOOD:

1) QA script fails:

- Tons of missing filters :(

2) Code review

Left over debug code:
use Data::Dumper;
warn $itemnumber;
warn $canreserve;
if ($canreserve) {
 warn $canreserve;
...

Little typo:
+        # if we have an item selectionned,
selected :)

3) Add text for release notes (not blocker, can be done later)

Please clean up the patches quickly for testing this week! First tests are looking good.
Comment 125 Alex Buckley 2018-10-28 18:42:51 UTC Comment hidden (obsolete)
Comment 126 Alex Buckley 2018-10-28 18:43:28 UTC Comment hidden (obsolete)
Comment 127 Alex Buckley 2018-10-28 18:43:54 UTC Comment hidden (obsolete)
Comment 128 Alex Buckley 2018-10-28 18:44:23 UTC Comment hidden (obsolete)
Comment 129 Alex Buckley 2018-10-28 18:44:51 UTC Comment hidden (obsolete)
Comment 130 Alex Buckley 2018-10-28 18:45:39 UTC Comment hidden (obsolete)
Comment 131 Alex Buckley 2018-10-28 18:46:01 UTC Comment hidden (obsolete)
Comment 132 Alex Buckley 2018-10-28 18:46:51 UTC Comment hidden (obsolete)
Comment 133 Alex Buckley 2018-10-28 18:47:17 UTC Comment hidden (obsolete)
Comment 134 Alex Buckley 2018-10-28 18:51:46 UTC
Hi there

I have fixed the apply conflict on the second patch, and have attached a follow-up patch which:

* Adds the missing filters to request.tt, opac-reserve.tt 
* Corrects typo in code comment
* Removes left-over debug code 

Cheers,
Alex
Comment 135 Katrin Fischer 2018-10-28 21:16:17 UTC
Treating last patch as a QA follow-up ;)
Comment 136 Katrin Fischer 2018-10-28 21:39:56 UTC Comment hidden (obsolete)
Comment 137 Katrin Fischer 2018-10-28 21:50:25 UTC
QA script is green now with the latest follow-up.

There is a problem in the way that item level holds are handled:

1) Circulation condition: Item level holds not allowed

Without this patch, the radio buttons for "next available" and "item specific" are not shown, which is the correct behavior.
With this patch, they always show.

2) Circulation condition: Enforce item level holds

It should only be showing "specific hold" here. The radio boxes are greyed out, but it might be nicer to restore former behaviour.

3) Holds per record allowed: 2, forced item holds as above

Go to a detail page for a record with more than 2 items.
Select 3 items for hold (item level)
Save
You get redirected to the account, but no holds have been placed.
Comment 138 Alex Buckley 2018-10-28 23:12:09 UTC Comment hidden (obsolete)
Comment 139 Alex Buckley 2018-10-28 23:21:37 UTC
(In reply to Katrin Fischer in comment 137):

Hi Katrin

I have addressed points 1 and 2 in the follow-up patch just attached. 

I have been unable to replicate issue 3; when 'Holds per record (count)'=2 and 'Item level hold'=Force I have found on opac-reserve.pl that after selecting 2 item checkboxes in the OPAC reservation interface the remaining checkboxes are disabled by JS preventing you from placing more than set in 'Holds per record (count)'. 

Are you able to elaborate a bit more on what you are seeing?

Cheers,
Alex
Comment 140 Katrin Fischer 2018-10-29 21:17:45 UTC Comment hidden (obsolete)
Comment 141 Katrin Fischer 2018-10-29 21:17:54 UTC Comment hidden (obsolete)
Comment 142 Katrin Fischer 2018-10-29 21:18:02 UTC Comment hidden (obsolete)
Comment 143 Katrin Fischer 2018-10-29 21:18:10 UTC Comment hidden (obsolete)
Comment 144 Katrin Fischer 2018-10-29 21:18:19 UTC Comment hidden (obsolete)
Comment 145 Katrin Fischer 2018-10-29 21:18:27 UTC Comment hidden (obsolete)
Comment 146 Katrin Fischer 2018-10-29 21:18:35 UTC Comment hidden (obsolete)
Comment 147 Katrin Fischer 2018-10-29 21:18:44 UTC Comment hidden (obsolete)
Comment 148 Katrin Fischer 2018-10-29 21:18:52 UTC Comment hidden (obsolete)
Comment 149 Katrin Fischer 2018-10-29 21:19:01 UTC Comment hidden (obsolete)
Comment 150 Katrin Fischer 2018-10-29 21:19:10 UTC Comment hidden (obsolete)
Comment 151 Katrin Fischer 2018-10-29 21:19:18 UTC Comment hidden (obsolete)
Comment 152 Nick Clemens 2018-11-08 11:10:00 UTC
CSS changes do not apply - should not include the css for staff side?

Staff side:
1 - Warning box when excedding maxreserves, however, I can place holds beyond the warning whether  AllowHoldPolicyOverride  is set or not
2 - Holds Per day setting not taken into account while placing holds (item level or next available)

OPAC:
1 - When placing item level holds the user is warned of exceeding maxreserves, however, if holds per day is less there is no warning, but holds are not placed beyond the limit with no warning (maxreserves=5 holds per day = 3, check 5 boxes, only 3 holds are placed)
Comment 153 Katrin Fischer 2018-11-08 11:13:57 UTC
Just a note: holds per day was not in yet when I started testing this.
Comment 154 Katrin Fischer 2018-11-09 16:47:19 UTC
Hi Alex, 
any chance to fix this before the final freeze?
Comment 155 Alex Buckley 2018-11-10 01:33:52 UTC
Hi Katrin,

Sure thing working on fix right now.
Comment 156 Alex Buckley 2018-11-10 12:00:04 UTC Comment hidden (obsolete)
Comment 157 Alex Buckley 2018-11-10 12:00:33 UTC Comment hidden (obsolete)
Comment 158 Alex Buckley 2018-11-10 12:01:10 UTC Comment hidden (obsolete)
Comment 159 Alex Buckley 2018-11-10 12:01:34 UTC Comment hidden (obsolete)
Comment 160 Alex Buckley 2018-11-10 12:02:07 UTC Comment hidden (obsolete)
Comment 161 Alex Buckley 2018-11-10 12:02:40 UTC Comment hidden (obsolete)
Comment 162 Alex Buckley 2018-11-10 12:03:48 UTC Comment hidden (obsolete)
Comment 163 Alex Buckley 2018-11-10 12:04:43 UTC Comment hidden (obsolete)
Comment 164 Alex Buckley 2018-11-10 12:05:10 UTC Comment hidden (obsolete)
Comment 165 Alex Buckley 2018-11-10 12:05:45 UTC Comment hidden (obsolete)
Comment 166 Alex Buckley 2018-11-10 12:06:44 UTC Comment hidden (obsolete)
Comment 167 Alex Buckley 2018-11-10 12:15:23 UTC
(In reply to Katrin Fischer in comment 154 and Nick Clemens in comment 152):

Hi both,

I have written a follow-up patch which does the following:

OPAC: 
The MaxReserves syspref, holds per day and holds per record circ setting values are all respected in determining how many items can be reserved. 

* Once the lowest of these 3 values is reached all un-checked item checkboxes on opac-reserve are disabled.

Staff client:

Similar to the OPAC in that once the lowest of the 3 values is reached all un-checked item checkboxes on request.tt are disabled except is the AllowHoldPolicyOverride is set to 'Allow'.

If AllowHoldPolicyOverride is enabled then additional checkboxes can be selected with the warning modal boxes loading on each checkbox click. 

Also added function to C4::Reserves named GetAllowedHoldsForPatronToday() which returns the holds per day value for the patron and item type combination. 

I have added unit tests for this new function in t/db_dependent/Reserves/MultiplePerRecord.t

Can you please test this?
Comment 168 Alex Buckley 2018-11-10 12:19:52 UTC Comment hidden (obsolete)
Comment 169 Tomás Cohen Arazi 2018-11-11 22:01:56 UTC
I'm not happy with adding things to the C4:: namespace, what about adding

my $biblio = Koha::Biblios->find( $biblio_id );
my $holds_allowed_on_record_today = $biblio->allowed_holds({ patron => $patron_obj });

holds are usually added in context in which we already have the $patron object, and a biblionumber.
Comment 170 Alex Buckley 2018-11-11 22:03:56 UTC
Thanks Tomas I will try that in a follow-up patch a bit later today when I have some time.
Comment 171 Alex Buckley 2018-11-12 10:11:57 UTC Comment hidden (obsolete)
Comment 172 Alex Buckley 2018-11-12 10:16:34 UTC Comment hidden (obsolete)
Comment 173 Alex Buckley 2018-11-12 10:20:25 UTC
Hi all,

I have just added another follow-up which moves the function out of C4/Reserves into Koha/Biblio (as Tomás suggested in comment 169)

The caller for this function is now: my $holds_allowed_on_record_today = $biblio->allowed_holds( $patron_obj );

I have amended the t/db_dependent/Reserves/MultiplePerRecord.t unit tests. 

Can this please be tested and if you could let me know of further required changes that would be great.

Many thanks,
Alex
Comment 174 Katrin Fischer 2018-11-12 20:12:43 UTC
Some first comments here:

1) CSS
Something is off here, there is no more less directory on master:

http://git.koha-community.org/gitweb/?p=koha.git;a=tree;f=koha-tmpl/opac-tmpl/bootstrap;hb=e112c6f117d4b6fd6c6160e8be9cba4186db1279


koha-tmpl/opac-tmpl/bootstrap/less/opac.less               | 2644 ++++++++++++++++++++++++++++++++++++++++++++

Also I think generated .css files should not be included.


2) Asking Joubu to glance at the code, 2 more things (non-blocker):

foreach my $item ( $self->items()->as_list() ) {

would be better as:

while ( my $item = $items->next ) {

And tests should be in their own subtest.
Comment 175 Katrin Fischer 2018-11-12 20:28:26 UTC
3) The logic appears not correct too:
I haven't placed any hold today.
I allow one daily hold, 2 holds per record.

Record level hold: let's me select 2 from pull down. On saving I am redirected to the patron account but no hold has been placed.

Item level hold: When switching to specific item I immediately get the error:

Maximum number of reserves allowed today reached.
(reserves has to be holds here!)

One checkbox is selected and I can save the hold -> so the warning should not have displayed.
Comment 176 Katrin Fischer 2018-11-12 20:35:42 UTC
Agreed with Alex that this will need some more work and clean-up before going in. Will come back to this!
Comment 177 Alex Buckley 2018-12-18 01:54:22 UTC
Created attachment 83331 [details] [review]
Bug 15565: Place multiple holds at once for the same record on staff and OPAC.

Test plan:
1. Create an circulation rule to use for testing the patches on this bug
report. Ensure it has a value greater than 1 for 'Holds
allowed', 'Holds per record (count)', and 'Item level holds' is enabled.

2. Also ensure the 'RequestOnOpac' syspref is set to 'Allow'.

3. Go to the reservation page (request.pl) of a biblio in the staff client with more
than one item.

4. Notice beside each item a radio button is displayed preventing you
from being able to select multiple items to place a hold on.

5. Go to the reservation page (opac-reserve.pl) of the same biblio in the OPAC
also notice a radio button is displayed beside every item.

6. Apply patches

7. Repeat step 3 and 5 and notice in both the staff client and OPAC
checkboxes rather than radio buttons are displayed beside each item so
you can select multiple items for reservation at a time.

8. From staff interface, place several item-level holds for the same user
on the same record at once:

  - check that you are warned if you exceed "Holds per record (count)"
(circulation rules) for this record.
  - check also maxReserves system preference.

9. From opac, place several item-level and record level holds
for the same user on the same record at once:

  - check that can't exceed "Holds per record (count)" for
  this record.
  This is enforced by all unticked item checkboxes being
  disabled after the number of ticked checkboxes equals the 'Holds per record (count)' (circulation rules) value.

  - check also maxReserves system preference.

Do the same in OPAC and staff by selecting several
record (multi-hold).

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 178 Alex Buckley 2018-12-18 01:55:53 UTC
Created attachment 83332 [details] [review]
Bug 15565: Use item's biblionumber only on item-level request

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 179 Alex Buckley 2018-12-18 01:56:24 UTC
Created attachment 83333 [details] [review]
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 180 Alex Buckley 2018-12-18 01:57:03 UTC
Created attachment 83334 [details] [review]
Bug 15565: Alert librarian that maximum holds per record is reached

Signed-off-by: Dani Elder <dani@bywatersolutions.com>

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 181 Alex Buckley 2018-12-18 01:57:36 UTC
Created attachment 83335 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 182 Alex Buckley 2018-12-18 01:58:04 UTC
Created attachment 83336 [details] [review]
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC

This commit fixes two bugs:

1. C4::Reserves->CanItemBeReserved() returns an output in a hash in the format: status => 'output'

However opac-reserve.pl is not accessing th output of this function
correctly consequently no item is ever reservable.

And so after a patron submits the reservation form on opac-reserve.pl it
looks like the item-level reservation has successfully been placed
however it has not and no error message is displayed to them.

This commit stores the output of the aforementioned function in a hash
and then checks the value of the status key. As a result opac-reserve.pl
now correctly accesses the value returned and item level holds are
successfully placed from the OPAC.

2. If a biblio has previously been reserved then the opac-reserve.pl page
shows the 'Next available item' and 'A specific item' radio buttons to be
disabled to ensure patrons cannot place a reservation of a different
type on the biblio.

With both of these radio buttons being disabled it results in item level holds
placed in the OPAC on a previously reserved item not being stored in the
database. This is very confusing for users.

I have implemented a conditional which checks what the forced_hold_type
of the previous hold was. If it was 'item' then the $reqtype =
'Specific' and if it was 'record' then $reqtype is 'Any'.

This means there will always be a value for $reqtype, and this variable
is checked if it is equal to 'Specific' and if it is then an item level
hold is placed.

Test plan:
1. Ensure in your circulation rules that 'Item level holds' is enabled

2. Apply all patches on this bug report except this commit

3. In the OPAC try placing a item level hold (and notice you can submit
the opac-reserve.pl page and no error messages are displayed. However
the hold(s) are not being placed.

4. Place an item level hold(s) on the biblio from staff client interface
and then return to the opac-reserve.pl page for the biblio in the OPAC
and notice both the 'Next available' and 'A specific item' radio buttons
are disabled.

5. Again try placing an item level hold in the OPAC and notice dispite
no error/warning message being displayed the hold is not being placed.

6. Apply this patch

7. Restart memcached, and plack

8. In the staff client remove the reservation you placed from the staff
client.

9. Repeat step 3 and notice when you go to your opac summary
(opac-user.pl) page the holds are displayed. .i.e. You can now
successfully place item level holds in the OPAC.

10. Return to the opac-reserve.pl page for the biblio and notice that
because you have already placed a hold on this biblio the 'Next
available item' and 'A specific item' radiobuttons are disabled.

11. Select several of the items checkboxes to place more item level
holds and press the 'Confirm hold' button.

12. Go to your opac summary (opac-user.pl) page again and notice the
holds you placed in step 11 are shown there.

i.e. you can place item level holds from the OPAC on biblios which have
previously had reserves placed on them.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 183 Alex Buckley 2018-12-18 01:58:46 UTC
Created attachment 83337 [details] [review]
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl

Previously the selected branch dropdown value was not being successfully retrieved in
opac-reserve.pl, this resulted in opac holds being placed with no pickup
branch being set.

Consequently as no branch was associated with the hold an Ajax error was
thrown when viewing the holds-table in the patron page in the staff
client.

This commit successfully retrieves the selected branch value and so
fixing the Ajax error.

Test plan:
1. Apply all patches on bug report 15565 except this commit

2. Place a hold(s) from opac-reserve.pl.

3. Go to your opac patron summary page (opac-user.pl) and notice
although the holds are successfully displayed they have no 'Pickup
location' value

4. Go to the patron page in the staff client and click on the 'Hold(s)'
tab and notice a popup explaining the datatable Ajax error is loaded

5. Apply this patch

6. Repeat step 3 noticing this time in the
opac-user.pl a 'Pickup location' is displayed for the hold you just
created.

6. Repeat step 4 and notice the hold datatable is successfully loaded
and populated without an Ajax error popup being loaded.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 184 Alex Buckley 2018-12-18 01:59:18 UTC
Created attachment 83338 [details] [review]
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds

This commit implements changes to opac-reserve.tt and opac-reserve.pl.

1. The change to opac-reserve.tt JS fixes the total_requested() function which returns an output which is always 2 digits higher than the actual number of selected item checkboxes.

i.e. if the 'maxreserves' syspref (limit for the overall
number of reserves allowed by a user) is 4 and a user has no previous
holds and ticks 3 item checkboxes and submits the form.

The 'Maximum number of reserve exceded.' popup message is displayed,
however as they have selected 3 items for reservation this is less than
the maxreserves value of 4.

The popup is displayed because total_requested() JS function is
returning 5 (i.e. 2 on top of the actual number of requested holds).

This commit usbtracts 2 from the number of holds calculated by
total_requested() so that the accurate number of requested holds is
being used to determine whether or not to display the excess holds
popup.

2. When the 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref are the same value (e.g. 4) you can tick 4 item
checkboxes in opac-reserve.pl and submit the form.

The form will submit successfully and to the user everything looks to
have worked however the holds will not be
successfully created.

This is because after the form is submitted to opac-reserve.pl a check is
done:

If 'number of submitted hold requests' + 'number of previous existing holds'
greater than or equal to maxreserves syspref then set the variable
'$canreserve' to 0.

If the $canreserve variable is 0 the form will submit, no warning/error
message is displayed to the user but the holds are not created.

Users should be able to place 4 holds if the maxreserves syspref is 4
(just not more than 4 holds). This commit changes this check to:

If 'number of submitted hold requests' + 'number of previous existing
holds' greater than maxreserves syspref then set the variable '$canreserve' to 0..

i.e. notice we only check for greater than (not equal to).

Which solves the bug of the form submitting and no holds being created
when the maxreserves syspref and 'Holds per record (count)' (circulation
rule) are the same.

Test plan (in two parts to test fix #1 and #2 (above)):

Test plan for fix #1:
1. Apply all patches on this bug report (15565) and follow and confirm
the feature works as described in the test plan in the first commit

2. Set the 'maxreserves' syspref to 3 (and 'Holds per record (count)'
(circulation rule) to 5)

3. Ensure your logged in patron account has no existing reserves

4. In the OPAC go to a biblio page with 3 or more items

5. In the reservation page (opac-reserve.pl) for that biblio select 2
item checkboes to place 2 item level holds and notice the 'Maximum
number of reserve exceded.' popup message is displayed.

As the maxreserves syspref value is 3 you should be able to place 2
holds without exceeding that value

6. Apply this patch

7. Repeat step 5 and notice the form submits without the exceeding popup
message and the holds are successfully placed.

Test plan for fix #2:
1. Apply all patches attached to this bug report (bug 15565) and perform
the test plan in the first commit and verify it works.

2. Set your 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref to the same value 3

3. Go to a biblio in the OPAC with 3 or more items

4. In the opac-reserve.pl reservation interface for that biblio tick 3
item checkboxes to place 3 item level holds and submit the form.

5. Notice the form submits and no warning/error message is displayed

6. In the opac user summary page that is displayed after the reserve
form is submitted notice the holds are not displayed this is because
they have not been placed

7. Apply this patch

8. Repeat step 4 and 5 and notice the holds have been successfully
placed and are displayed in your opac user summary page.

Sponsored-By: Brimbank Library
Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 185 Alex Buckley 2018-12-18 02:00:28 UTC
Created attachment 83339 [details] [review]
Bug 15565: (follow-up) Removed leftover debug code and added missing filters

Sponsord-By: Brimbank Library, Australia

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 186 Alex Buckley 2018-12-18 02:00:53 UTC
Created attachment 83340 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 187 Alex Buckley 2018-12-18 02:17:28 UTC
Created attachment 83341 [details] [review]
Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio

Also updated t/db_dependen/Reserves/MultiplePerRecord.t test to reflect
moving function.

Added the display of the JS modal box informing OPAC borrower that
they have exceeded hold limit after they click 'A specific item'
radiobutton on opac-reserve if holds per day, holds per record or
maxreserves is 1

Test plan:
1. Apply patches
2. Restart memcached and plack
3. Run t/db_dependent/Reserves/MultiplePerRecord.t
4. Set your maxreserves syspref to 3, and a circ rules holds per record
to 2 and holds per day to 1
5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
specific item' radiobutton and observe the first item checkbox is
checked and other unchecked checkboxes are disabled.
6. Repeat step 4 this time setting holds per day to 5
7. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays
8. Select a second item checkbox and observe a modal saying you have reached
the maximum number of holds for the record is loaded - notice the
wording 'hold' not 'reserve' in use in the modal box
9. Click 'OK' on modal and observe all unchecked item checkboxes are
automatically disabled to prevent additional holds being placed
10. Select 'Confirm hold'
11. On your holds summary page confirm both holds are placed
12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
"Don't allow"
13. Visit a biblio reservation interface in the staff client
14. Select 2 item checkboxes and observe the modal box explaining you
have reached maximum holds for the record is displayed - notice the
wording of 'hold' not 'reserve' is used in the modal box.
15. Select 'OK' on the modal and notice all un-checked item checkboxes
are automatically disabled
16. Repeat steps 12-15 this time with the value of
'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
you have clicked on the second item checkbox although the modal still
loads the item checkboxes do not disable - you are able to override and
continue selecting checkboxes each time with modal warning loading.

17. In both the OPAC and staff client try placing a record level hold,
selecting a value from the 'Holds to place (count)' dropdown. Notice
that the maximum value of this is the lowest value out of the 'maximum
number of holds per record', 'maximum number of holds per day' and the
'maxreserves' system preference so in this case it will be 2
18. Save and notice 2 holds are placed

19. Observe the allow_holds() function is in the Koha/Biblio.pm file and
the GetAllowedHoldsForPatronToday() function has been removed from
C4/Reserves.pm

Sponsored-By: Brimbank Library, Australia
Comment 188 Alex Buckley 2018-12-18 02:20:30 UTC
Created attachment 83342 [details] [review]
Bug 15565: (follow-up) Changes to maximum hold allowed logic

Implemented changes to the logic to address Katrin Fischer's points in
comment #175, also changed the modal box wording to use 'hold' not
'reserve'

Also updated t/db_dependent/Reserves/MultiplePerRecord.t test to reflect
moving function.

Added the display of the JS modal box informing OPAC borrower that
they have exceeded hold limit after they click 'A specific item'
radiobutton on opac-reserve if holds per day, holds per record or
maxreserves is 1

Test plan:
1. Apply patches
2. Restart memcached and plack
3. Run t/db_dependent/Reserves/MultiplePerRecord.t
4. Set your maxreserves syspref to 3, and a circ rules holds per record
to 2 and holds per day to 1
5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
specific item' radiobutton and observe the first item checkbox is
checked and other unchecked checkboxes are disabled.
6. Repeat step 4 this time setting holds per day to 5
7. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays
8. Select a second item checkbox and observe a modal saying you have reached
the maximum number of holds for the record is loaded - notice the
wording 'hold' not 'reserve' in use in the modal box
9. Click 'OK' on modal and observe all unchecked item checkboxes are
automatically disabled to prevent additional holds being placed
10. Select 'Confirm hold'
11. On your holds summary page confirm both holds are placed
12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
"Don't allow"
13. Visit a biblio reservation interface in the staff client
14. Select 2 item checkboxes and observe the modal box explaining you
have reached maximum holds for the record is displayed - notice the
wording of 'hold' not 'reserve' is used in the modal box.
15. Select 'OK' on the modal and notice all un-checked item checkboxes
are automatically disabled
16. Repeat steps 12-15 this time with the value of
'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
you have clicked on the second item checkbox although the modal still
loads the item checkboxes do not disable - you are able to override and
continue selecting checkboxes each time with modal warning loading.

17. In both the OPAC and staff client try placing a record level hold,
selecting a value from the 'Holds to place (count)' dropdown. Notice
that the maximum value of this is the lowest value out of the 'maximum
number of holds per record', 'maximum number of holds per day' and the
'maxreserves' system preference so in this case it will be 2
18. Save and notice 2 holds are placed

19. Observe the allow_holds() function is in the Koha/Biblio.pm file and
the GetAllowedHoldsForPatronToday() function has been removed from
C4/Reserves.pm

Sponsored-By: Brimbank Library, Australia
Comment 189 Alex Buckley 2018-12-19 00:49:05 UTC
Created attachment 83378 [details] [review]
Bug 15565: (follow-up) Place multiple holds from opac-search.pl

Previously before this patch if you selected multiple bibliographic
results from opac-search.pl and selected 'Place hold' and confirmed the
hold only 1 hold on the first of the selected biblios would be placed.

This patch fixes this bug and enables users to be able to place holds on
multiple bibliographic records in opac-search.pl, select 'Place hold', select if they want
each of these holds to be on 'Next available', 'Next available from
group', or 'A specific item' and place their holds and all the holds
they placed will be placed correctly.

Test plan:
1. Follow test plan on the last patch - (Don't apply this patch)
2. Try to select multiple bibliographic record checkboxes and select
'Place hold'
3. Confirm holds and notice you are re-directed to opac-user.pl and only
the first hold has been placed

4. Apply this patch
5. Restart memcached and plack if running
6. Repeat steps 2 and 3 and notice all holds were placed
7. Repeat step 2 and then select a variety of 'A specific item' and 'Next
available item' holds for each of the bibliographic holds and
confirm they are all placed correctly
8. Confirm you can still place multiple holds correctly in opac-reserve
accessing from a OPAC biblio detail page

Sponsored-By: Brimbank Library, Australia
Comment 190 Alex Buckley 2018-12-19 03:31:36 UTC
Created attachment 83380 [details] [review]
Bug 15565: (follow-up) Changes to maximum hold allowed logic

Implemented changes to the logic to address Katrin Fischer's points in
comment #175, also changed the modal box wording to use 'hold' not
'reserve'

Also updated t/db_dependent/Reserves/MultiplePerRecord.t test to reflect
moving function.

Added the display of the JS modal box informing OPAC borrower that
they have exceeded hold limit after they click 'A specific item'
radiobutton on opac-reserve if holds per day, holds per record or
maxreserves is 1

Test plan:
1. Apply patches
2. Restart memcached and plack
3. Run t/db_dependent/Reserves/MultiplePerRecord.t
4. Set your maxreserves syspref to 3, and a circ rules holds per record
to 2 and holds per day to 1
5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
specific item' radiobutton and observe the first item checkbox is
checked and other unchecked checkboxes are disabled.
6. Repeat step 4 this time setting holds per day to 5
7. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays
8. Select a second item checkbox and observe a modal saying you have reached
the maximum number of holds for the record is loaded - notice the
wording 'hold' not 'reserve' in use in the modal box
9. Click 'OK' on modal and observe all unchecked item checkboxes are
automatically disabled to prevent additional holds being placed
10. Select 'Confirm hold'
11. On your holds summary page confirm both holds are placed
12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
"Don't allow"
13. Visit a biblio reservation interface in the staff client
14. Select 2 item checkboxes and observe the modal box explaining you
have reached maximum holds for the record is displayed - notice the
wording of 'hold' not 'reserve' is used in the modal box.
15. Select 'OK' on the modal and notice all un-checked item checkboxes
are automatically disabled
16. Repeat steps 12-15 this time with the value of
'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
you have clicked on the second item checkbox although the modal still
loads the item checkboxes do not disable - you are able to override and
continue selecting checkboxes each time with modal warning loading.

17. In both the OPAC and staff client try placing a record level hold,
selecting a value from the 'Holds to place (count)' dropdown. Notice
that the maximum value of this is the lowest value out of the 'maximum
number of holds per record', 'maximum number of holds per day' and the
'maxreserves' system preference so in this case it will be 2
18. Save and notice 2 holds are placed

19. Observe the allow_holds() function is in the Koha/Biblio.pm file and
the GetAllowedHoldsForPatronToday() function has been removed from
C4/Reserves.pm

Sponsored-By: Brimbank Library, Australia
Comment 191 Alex Buckley 2018-12-19 03:32:27 UTC
Created attachment 83381 [details] [review]
Bug 15565: (follow-up) Place multiple holds from opac-search.pl

Previously before this patch if you selected multiple bibliographic
results from opac-search.pl and selected 'Place hold' and confirmed the
hold only 1 hold on the first of the selected biblios would be placed.

This patch fixes this bug and enables users to be able to place holds on
multiple bibliographic records in opac-search.pl, select 'Place hold', select if they want
each of these holds to be on 'Next available', 'Next available from
group', or 'A specific item' and place their holds and all the holds
they placed will be placed correctly.

Test plan:
1. Follow test plan on the last patch - (Don't apply this patch)
2. Try to select multiple bibliographic record checkboxes and select
'Place hold'
3. Confirm holds and notice you are re-directed to opac-user.pl and only
the first hold has been placed

4. Apply this patch
5. Restart memcached and plack if running
6. Repeat steps 2 and 3 and notice all holds were placed
7. Repeat step 2 and then select a variety of 'A specific item' and 'Next
available item' holds for each of the bibliographic holds and
confirm they are all placed correctly
8. Confirm you can still place multiple holds correctly in opac-reserve
accessing from a OPAC biblio detail page

Sponsored-By: Brimbank Library, Australia
Comment 192 Alex Buckley 2018-12-19 03:49:03 UTC
Over the last little while in amongst lots of other stuff I have made changes to fix the issues Katrin outlined in comments 174 and #175.

Please retest :)
Comment 193 Alex Buckley 2019-01-29 23:52:43 UTC
Aleisha is going to manage/work on the upstreaming of the patches on this bug report, as I am unlikely to have enough time to do this for the next while.
Comment 194 Aleisha Amohia 2019-01-31 01:03:44 UTC
Created attachment 84541 [details] [review]
Bug 15565: Place multiple holds at once for the same record on staff and OPAC.

Test plan:
1. Create an circulation rule to use for testing the patches on this bug
report. Ensure it has a value greater than 1 for 'Holds
allowed', 'Holds per record (count)', and 'Item level holds' is enabled.

2. Also ensure the 'RequestOnOpac' syspref is set to 'Allow'.

3. Go to the reservation page (request.pl) of a biblio in the staff client with more
than one item.

4. Notice beside each item a radio button is displayed preventing you
from being able to select multiple items to place a hold on.

5. Go to the reservation page (opac-reserve.pl) of the same biblio in the OPAC
also notice a radio button is displayed beside every item.

6. Apply patches

7. Repeat step 3 and 5 and notice in both the staff client and OPAC
checkboxes rather than radio buttons are displayed beside each item so
you can select multiple items for reservation at a time.

8. From staff interface, place several item-level holds for the same user
on the same record at once:

  - check that you are warned if you exceed "Holds per record (count)"
(circulation rules) for this record.
  - check also maxReserves system preference.

9. From opac, place several item-level and record level holds
for the same user on the same record at once:

  - check that can't exceed "Holds per record (count)" for
  this record.
  This is enforced by all unticked item checkboxes being
  disabled after the number of ticked checkboxes equals the 'Holds per record (count)' (circulation rules) value.

  - check also maxReserves system preference.

Do the same in OPAC and staff by selecting several
record (multi-hold).

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 195 Aleisha Amohia 2019-01-31 01:03:56 UTC
Created attachment 84542 [details] [review]
Bug 15565: Use item's biblionumber only on item-level request

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 196 Aleisha Amohia 2019-01-31 01:04:05 UTC
Created attachment 84543 [details] [review]
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 197 Aleisha Amohia 2019-01-31 01:04:15 UTC
Created attachment 84544 [details] [review]
Bug 15565: Alert librarian that maximum holds per record is reached

Signed-off-by: Dani Elder <dani@bywatersolutions.com>

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 198 Aleisha Amohia 2019-01-31 01:04:24 UTC
Created attachment 84545 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 199 Aleisha Amohia 2019-01-31 01:04:33 UTC
Created attachment 84546 [details] [review]
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC

This commit fixes two bugs:

1. C4::Reserves->CanItemBeReserved() returns an output in a hash in the format: status => 'output'

However opac-reserve.pl is not accessing th output of this function
correctly consequently no item is ever reservable.

And so after a patron submits the reservation form on opac-reserve.pl it
looks like the item-level reservation has successfully been placed
however it has not and no error message is displayed to them.

This commit stores the output of the aforementioned function in a hash
and then checks the value of the status key. As a result opac-reserve.pl
now correctly accesses the value returned and item level holds are
successfully placed from the OPAC.

2. If a biblio has previously been reserved then the opac-reserve.pl page
shows the 'Next available item' and 'A specific item' radio buttons to be
disabled to ensure patrons cannot place a reservation of a different
type on the biblio.

With both of these radio buttons being disabled it results in item level holds
placed in the OPAC on a previously reserved item not being stored in the
database. This is very confusing for users.

I have implemented a conditional which checks what the forced_hold_type
of the previous hold was. If it was 'item' then the $reqtype =
'Specific' and if it was 'record' then $reqtype is 'Any'.

This means there will always be a value for $reqtype, and this variable
is checked if it is equal to 'Specific' and if it is then an item level
hold is placed.

Test plan:
1. Ensure in your circulation rules that 'Item level holds' is enabled

2. Apply all patches on this bug report except this commit

3. In the OPAC try placing a item level hold (and notice you can submit
the opac-reserve.pl page and no error messages are displayed. However
the hold(s) are not being placed.

4. Place an item level hold(s) on the biblio from staff client interface
and then return to the opac-reserve.pl page for the biblio in the OPAC
and notice both the 'Next available' and 'A specific item' radio buttons
are disabled.

5. Again try placing an item level hold in the OPAC and notice dispite
no error/warning message being displayed the hold is not being placed.

6. Apply this patch

7. Restart memcached, and plack

8. In the staff client remove the reservation you placed from the staff
client.

9. Repeat step 3 and notice when you go to your opac summary
(opac-user.pl) page the holds are displayed. .i.e. You can now
successfully place item level holds in the OPAC.

10. Return to the opac-reserve.pl page for the biblio and notice that
because you have already placed a hold on this biblio the 'Next
available item' and 'A specific item' radiobuttons are disabled.

11. Select several of the items checkboxes to place more item level
holds and press the 'Confirm hold' button.

12. Go to your opac summary (opac-user.pl) page again and notice the
holds you placed in step 11 are shown there.

i.e. you can place item level holds from the OPAC on biblios which have
previously had reserves placed on them.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 200 Aleisha Amohia 2019-01-31 01:04:42 UTC
Created attachment 84547 [details] [review]
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl

Previously the selected branch dropdown value was not being successfully retrieved in
opac-reserve.pl, this resulted in opac holds being placed with no pickup
branch being set.

Consequently as no branch was associated with the hold an Ajax error was
thrown when viewing the holds-table in the patron page in the staff
client.

This commit successfully retrieves the selected branch value and so
fixing the Ajax error.

Test plan:
1. Apply all patches on bug report 15565 except this commit

2. Place a hold(s) from opac-reserve.pl.

3. Go to your opac patron summary page (opac-user.pl) and notice
although the holds are successfully displayed they have no 'Pickup
location' value

4. Go to the patron page in the staff client and click on the 'Hold(s)'
tab and notice a popup explaining the datatable Ajax error is loaded

5. Apply this patch

6. Repeat step 3 noticing this time in the
opac-user.pl a 'Pickup location' is displayed for the hold you just
created.

6. Repeat step 4 and notice the hold datatable is successfully loaded
and populated without an Ajax error popup being loaded.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 201 Aleisha Amohia 2019-01-31 01:04:51 UTC
Created attachment 84548 [details] [review]
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds

This commit implements changes to opac-reserve.tt and opac-reserve.pl.

1. The change to opac-reserve.tt JS fixes the total_requested() function which returns an output which is always 2 digits higher than the actual number of selected item checkboxes.

i.e. if the 'maxreserves' syspref (limit for the overall
number of reserves allowed by a user) is 4 and a user has no previous
holds and ticks 3 item checkboxes and submits the form.

The 'Maximum number of reserve exceded.' popup message is displayed,
however as they have selected 3 items for reservation this is less than
the maxreserves value of 4.

The popup is displayed because total_requested() JS function is
returning 5 (i.e. 2 on top of the actual number of requested holds).

This commit usbtracts 2 from the number of holds calculated by
total_requested() so that the accurate number of requested holds is
being used to determine whether or not to display the excess holds
popup.

2. When the 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref are the same value (e.g. 4) you can tick 4 item
checkboxes in opac-reserve.pl and submit the form.

The form will submit successfully and to the user everything looks to
have worked however the holds will not be
successfully created.

This is because after the form is submitted to opac-reserve.pl a check is
done:

If 'number of submitted hold requests' + 'number of previous existing holds'
greater than or equal to maxreserves syspref then set the variable
'$canreserve' to 0.

If the $canreserve variable is 0 the form will submit, no warning/error
message is displayed to the user but the holds are not created.

Users should be able to place 4 holds if the maxreserves syspref is 4
(just not more than 4 holds). This commit changes this check to:

If 'number of submitted hold requests' + 'number of previous existing
holds' greater than maxreserves syspref then set the variable '$canreserve' to 0..

i.e. notice we only check for greater than (not equal to).

Which solves the bug of the form submitting and no holds being created
when the maxreserves syspref and 'Holds per record (count)' (circulation
rule) are the same.

Test plan (in two parts to test fix #1 and #2 (above)):

Test plan for fix #1:
1. Apply all patches on this bug report (15565) and follow and confirm
the feature works as described in the test plan in the first commit

2. Set the 'maxreserves' syspref to 3 (and 'Holds per record (count)'
(circulation rule) to 5)

3. Ensure your logged in patron account has no existing reserves

4. In the OPAC go to a biblio page with 3 or more items

5. In the reservation page (opac-reserve.pl) for that biblio select 2
item checkboes to place 2 item level holds and notice the 'Maximum
number of reserve exceded.' popup message is displayed.

As the maxreserves syspref value is 3 you should be able to place 2
holds without exceeding that value

6. Apply this patch

7. Repeat step 5 and notice the form submits without the exceeding popup
message and the holds are successfully placed.

Test plan for fix #2:
1. Apply all patches attached to this bug report (bug 15565) and perform
the test plan in the first commit and verify it works.

2. Set your 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref to the same value 3

3. Go to a biblio in the OPAC with 3 or more items

4. In the opac-reserve.pl reservation interface for that biblio tick 3
item checkboxes to place 3 item level holds and submit the form.

5. Notice the form submits and no warning/error message is displayed

6. In the opac user summary page that is displayed after the reserve
form is submitted notice the holds are not displayed this is because
they have not been placed

7. Apply this patch

8. Repeat step 4 and 5 and notice the holds have been successfully
placed and are displayed in your opac user summary page.

Sponsored-By: Brimbank Library
Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 202 Aleisha Amohia 2019-01-31 01:05:01 UTC
Created attachment 84549 [details] [review]
Bug 15565: (follow-up) Removed leftover debug code and added missing filters

Sponsord-By: Brimbank Library, Australia

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 203 Aleisha Amohia 2019-01-31 01:05:18 UTC
Created attachment 84550 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 204 Aleisha Amohia 2019-01-31 01:05:32 UTC
Created attachment 84551 [details] [review]
Bug 15565: (follow-up) Changes to maximum hold allowed logic

Implemented changes to the logic to address Katrin Fischer's points in
comment #175, also changed the modal box wording to use 'hold' not
'reserve'

Also updated t/db_dependent/Reserves/MultiplePerRecord.t test to reflect
moving function.

Added the display of the JS modal box informing OPAC borrower that
they have exceeded hold limit after they click 'A specific item'
radiobutton on opac-reserve if holds per day, holds per record or
maxreserves is 1

Test plan:
1. Apply patches
2. Restart memcached and plack
3. Run t/db_dependent/Reserves/MultiplePerRecord.t
4. Set your maxreserves syspref to 3, and a circ rules holds per record
to 2 and holds per day to 1
5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
specific item' radiobutton and observe the first item checkbox is
checked and other unchecked checkboxes are disabled.
6. Repeat step 4 this time setting holds per day to 5
7. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays
8. Select a second item checkbox and observe a modal saying you have reached
the maximum number of holds for the record is loaded - notice the
wording 'hold' not 'reserve' in use in the modal box
9. Click 'OK' on modal and observe all unchecked item checkboxes are
automatically disabled to prevent additional holds being placed
10. Select 'Confirm hold'
11. On your holds summary page confirm both holds are placed
12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
"Don't allow"
13. Visit a biblio reservation interface in the staff client
14. Select 2 item checkboxes and observe the modal box explaining you
have reached maximum holds for the record is displayed - notice the
wording of 'hold' not 'reserve' is used in the modal box.
15. Select 'OK' on the modal and notice all un-checked item checkboxes
are automatically disabled
16. Repeat steps 12-15 this time with the value of
'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
you have clicked on the second item checkbox although the modal still
loads the item checkboxes do not disable - you are able to override and
continue selecting checkboxes each time with modal warning loading.

17. In both the OPAC and staff client try placing a record level hold,
selecting a value from the 'Holds to place (count)' dropdown. Notice
that the maximum value of this is the lowest value out of the 'maximum
number of holds per record', 'maximum number of holds per day' and the
'maxreserves' system preference so in this case it will be 2
18. Save and notice 2 holds are placed

19. Observe the allow_holds() function is in the Koha/Biblio.pm file and
the GetAllowedHoldsForPatronToday() function has been removed from
C4/Reserves.pm

Sponsored-By: Brimbank Library, Australia
Comment 205 Aleisha Amohia 2019-01-31 01:05:41 UTC
Created attachment 84552 [details] [review]
Bug 15565: (follow-up) Place multiple holds from opac-search.pl

Previously before this patch if you selected multiple bibliographic
results from opac-search.pl and selected 'Place hold' and confirmed the
hold only 1 hold on the first of the selected biblios would be placed.

This patch fixes this bug and enables users to be able to place holds on
multiple bibliographic records in opac-search.pl, select 'Place hold', select if they want
each of these holds to be on 'Next available', 'Next available from
group', or 'A specific item' and place their holds and all the holds
they placed will be placed correctly.

Test plan:
1. Follow test plan on the last patch - (Don't apply this patch)
2. Try to select multiple bibliographic record checkboxes and select
'Place hold'
3. Confirm holds and notice you are re-directed to opac-user.pl and only
the first hold has been placed

4. Apply this patch
5. Restart memcached and plack if running
6. Repeat steps 2 and 3 and notice all holds were placed
7. Repeat step 2 and then select a variety of 'A specific item' and 'Next
available item' holds for each of the bibliographic holds and
confirm they are all placed correctly
8. Confirm you can still place multiple holds correctly in opac-reserve
accessing from a OPAC biblio detail page

Sponsored-By: Brimbank Library, Australia
Comment 206 Aleisha Amohia 2019-01-31 01:14:39 UTC
Patches have been rebased. Working on improving the code and including design/layout changes that were requested by Brimbank.
Comment 207 Aleisha Amohia 2019-01-31 02:13:08 UTC
Created attachment 84553 [details] [review]
Bug 15565: (follow-up) fixing typo that breaks placing a hold
Comment 208 Aleisha Amohia 2019-04-15 02:47:13 UTC
Created attachment 87979 [details] [review]
Bug 15565: Place multiple holds at once for the same record on staff and OPAC.

Test plan:
1. Create an circulation rule to use for testing the patches on this bug
report. Ensure it has a value greater than 1 for 'Holds
allowed', 'Holds per record (count)', and 'Item level holds' is enabled.

2. Also ensure the 'RequestOnOpac' syspref is set to 'Allow'.

3. Go to the reservation page (request.pl) of a biblio in the staff client with more
than one item.

4. Notice beside each item a radio button is displayed preventing you
from being able to select multiple items to place a hold on.

5. Go to the reservation page (opac-reserve.pl) of the same biblio in the OPAC
also notice a radio button is displayed beside every item.

6. Apply patches

7. Repeat step 3 and 5 and notice in both the staff client and OPAC
checkboxes rather than radio buttons are displayed beside each item so
you can select multiple items for reservation at a time.

8. From staff interface, place several item-level holds for the same user
on the same record at once:

  - check that you are warned if you exceed "Holds per record (count)"
(circulation rules) for this record.
  - check also maxReserves system preference.

9. From opac, place several item-level and record level holds
for the same user on the same record at once:

  - check that can't exceed "Holds per record (count)" for
  this record.
  This is enforced by all unticked item checkboxes being
  disabled after the number of ticked checkboxes equals the 'Holds per record (count)' (circulation rules) value.

  - check also maxReserves system preference.

Do the same in OPAC and staff by selecting several
record (multi-hold).

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 209 Aleisha Amohia 2019-04-15 02:47:22 UTC
Created attachment 87980 [details] [review]
Bug 15565: Use item's biblionumber only on item-level request

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 210 Aleisha Amohia 2019-04-15 02:47:31 UTC
Created attachment 87981 [details] [review]
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 211 Aleisha Amohia 2019-04-15 02:47:40 UTC
Created attachment 87982 [details] [review]
Bug 15565: Alert librarian that maximum holds per record is reached

Signed-off-by: Dani Elder <dani@bywatersolutions.com>

Signed-off-by: Danielle Elder <danielle@bywatersolution.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 212 Aleisha Amohia 2019-04-15 02:47:49 UTC
Created attachment 87983 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 213 Aleisha Amohia 2019-04-15 02:47:57 UTC
Created attachment 87984 [details] [review]
Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC

This commit fixes two bugs:

1. C4::Reserves->CanItemBeReserved() returns an output in a hash in the format: status => 'output'

However opac-reserve.pl is not accessing th output of this function
correctly consequently no item is ever reservable.

And so after a patron submits the reservation form on opac-reserve.pl it
looks like the item-level reservation has successfully been placed
however it has not and no error message is displayed to them.

This commit stores the output of the aforementioned function in a hash
and then checks the value of the status key. As a result opac-reserve.pl
now correctly accesses the value returned and item level holds are
successfully placed from the OPAC.

2. If a biblio has previously been reserved then the opac-reserve.pl page
shows the 'Next available item' and 'A specific item' radio buttons to be
disabled to ensure patrons cannot place a reservation of a different
type on the biblio.

With both of these radio buttons being disabled it results in item level holds
placed in the OPAC on a previously reserved item not being stored in the
database. This is very confusing for users.

I have implemented a conditional which checks what the forced_hold_type
of the previous hold was. If it was 'item' then the $reqtype =
'Specific' and if it was 'record' then $reqtype is 'Any'.

This means there will always be a value for $reqtype, and this variable
is checked if it is equal to 'Specific' and if it is then an item level
hold is placed.

Test plan:
1. Ensure in your circulation rules that 'Item level holds' is enabled

2. Apply all patches on this bug report except this commit

3. In the OPAC try placing a item level hold (and notice you can submit
the opac-reserve.pl page and no error messages are displayed. However
the hold(s) are not being placed.

4. Place an item level hold(s) on the biblio from staff client interface
and then return to the opac-reserve.pl page for the biblio in the OPAC
and notice both the 'Next available' and 'A specific item' radio buttons
are disabled.

5. Again try placing an item level hold in the OPAC and notice dispite
no error/warning message being displayed the hold is not being placed.

6. Apply this patch

7. Restart memcached, and plack

8. In the staff client remove the reservation you placed from the staff
client.

9. Repeat step 3 and notice when you go to your opac summary
(opac-user.pl) page the holds are displayed. .i.e. You can now
successfully place item level holds in the OPAC.

10. Return to the opac-reserve.pl page for the biblio and notice that
because you have already placed a hold on this biblio the 'Next
available item' and 'A specific item' radiobuttons are disabled.

11. Select several of the items checkboxes to place more item level
holds and press the 'Confirm hold' button.

12. Go to your opac summary (opac-user.pl) page again and notice the
holds you placed in step 11 are shown there.

i.e. you can place item level holds from the OPAC on biblios which have
previously had reserves placed on them.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 214 Aleisha Amohia 2019-04-15 02:48:06 UTC
Created attachment 87985 [details] [review]
Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl

Previously the selected branch dropdown value was not being successfully retrieved in
opac-reserve.pl, this resulted in opac holds being placed with no pickup
branch being set.

Consequently as no branch was associated with the hold an Ajax error was
thrown when viewing the holds-table in the patron page in the staff
client.

This commit successfully retrieves the selected branch value and so
fixing the Ajax error.

Test plan:
1. Apply all patches on bug report 15565 except this commit

2. Place a hold(s) from opac-reserve.pl.

3. Go to your opac patron summary page (opac-user.pl) and notice
although the holds are successfully displayed they have no 'Pickup
location' value

4. Go to the patron page in the staff client and click on the 'Hold(s)'
tab and notice a popup explaining the datatable Ajax error is loaded

5. Apply this patch

6. Repeat step 3 noticing this time in the
opac-user.pl a 'Pickup location' is displayed for the hold you just
created.

6. Repeat step 4 and notice the hold datatable is successfully loaded
and populated without an Ajax error popup being loaded.

Sponsored-By: Brimbank Library

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 215 Aleisha Amohia 2019-04-15 02:48:15 UTC
Created attachment 87986 [details] [review]
Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds

This commit implements changes to opac-reserve.tt and opac-reserve.pl.

1. The change to opac-reserve.tt JS fixes the total_requested() function which returns an output which is always 2 digits higher than the actual number of selected item checkboxes.

i.e. if the 'maxreserves' syspref (limit for the overall
number of reserves allowed by a user) is 4 and a user has no previous
holds and ticks 3 item checkboxes and submits the form.

The 'Maximum number of reserve exceded.' popup message is displayed,
however as they have selected 3 items for reservation this is less than
the maxreserves value of 4.

The popup is displayed because total_requested() JS function is
returning 5 (i.e. 2 on top of the actual number of requested holds).

This commit usbtracts 2 from the number of holds calculated by
total_requested() so that the accurate number of requested holds is
being used to determine whether or not to display the excess holds
popup.

2. When the 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref are the same value (e.g. 4) you can tick 4 item
checkboxes in opac-reserve.pl and submit the form.

The form will submit successfully and to the user everything looks to
have worked however the holds will not be
successfully created.

This is because after the form is submitted to opac-reserve.pl a check is
done:

If 'number of submitted hold requests' + 'number of previous existing holds'
greater than or equal to maxreserves syspref then set the variable
'$canreserve' to 0.

If the $canreserve variable is 0 the form will submit, no warning/error
message is displayed to the user but the holds are not created.

Users should be able to place 4 holds if the maxreserves syspref is 4
(just not more than 4 holds). This commit changes this check to:

If 'number of submitted hold requests' + 'number of previous existing
holds' greater than maxreserves syspref then set the variable '$canreserve' to 0..

i.e. notice we only check for greater than (not equal to).

Which solves the bug of the form submitting and no holds being created
when the maxreserves syspref and 'Holds per record (count)' (circulation
rule) are the same.

Test plan (in two parts to test fix #1 and #2 (above)):

Test plan for fix #1:
1. Apply all patches on this bug report (15565) and follow and confirm
the feature works as described in the test plan in the first commit

2. Set the 'maxreserves' syspref to 3 (and 'Holds per record (count)'
(circulation rule) to 5)

3. Ensure your logged in patron account has no existing reserves

4. In the OPAC go to a biblio page with 3 or more items

5. In the reservation page (opac-reserve.pl) for that biblio select 2
item checkboes to place 2 item level holds and notice the 'Maximum
number of reserve exceded.' popup message is displayed.

As the maxreserves syspref value is 3 you should be able to place 2
holds without exceeding that value

6. Apply this patch

7. Repeat step 5 and notice the form submits without the exceeding popup
message and the holds are successfully placed.

Test plan for fix #2:
1. Apply all patches attached to this bug report (bug 15565) and perform
the test plan in the first commit and verify it works.

2. Set your 'Holds per record (count)' (circulation rule) and
'maxreserves' syspref to the same value 3

3. Go to a biblio in the OPAC with 3 or more items

4. In the opac-reserve.pl reservation interface for that biblio tick 3
item checkboxes to place 3 item level holds and submit the form.

5. Notice the form submits and no warning/error message is displayed

6. In the opac user summary page that is displayed after the reserve
form is submitted notice the holds are not displayed this is because
they have not been placed

7. Apply this patch

8. Repeat step 4 and 5 and notice the holds have been successfully
placed and are displayed in your opac user summary page.

Sponsored-By: Brimbank Library
Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 216 Aleisha Amohia 2019-04-15 02:48:24 UTC
Created attachment 87987 [details] [review]
Bug 15565: (follow-up) Removed leftover debug code and added missing filters

Sponsord-By: Brimbank Library, Australia

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 217 Aleisha Amohia 2019-04-15 02:48:34 UTC
Created attachment 87988 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 218 Aleisha Amohia 2019-04-15 02:48:43 UTC
Created attachment 87989 [details] [review]
Bug 15565: (follow-up) Changes to maximum hold allowed logic

Implemented changes to the logic to address Katrin Fischer's points in
comment #175, also changed the modal box wording to use 'hold' not
'reserve'

Also updated t/db_dependent/Reserves/MultiplePerRecord.t test to reflect
moving function.

Added the display of the JS modal box informing OPAC borrower that
they have exceeded hold limit after they click 'A specific item'
radiobutton on opac-reserve if holds per day, holds per record or
maxreserves is 1

Test plan:
1. Apply patches
2. Restart memcached and plack
3. Run t/db_dependent/Reserves/MultiplePerRecord.t
4. Set your maxreserves syspref to 3, and a circ rules holds per record
to 2 and holds per day to 1
5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
specific item' radiobutton and observe the first item checkbox is
checked and other unchecked checkboxes are disabled.
6. Repeat step 4 this time setting holds per day to 5
7. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays
8. Select a second item checkbox and observe a modal saying you have reached
the maximum number of holds for the record is loaded - notice the
wording 'hold' not 'reserve' in use in the modal box
9. Click 'OK' on modal and observe all unchecked item checkboxes are
automatically disabled to prevent additional holds being placed
10. Select 'Confirm hold'
11. On your holds summary page confirm both holds are placed
12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
"Don't allow"
13. Visit a biblio reservation interface in the staff client
14. Select 2 item checkboxes and observe the modal box explaining you
have reached maximum holds for the record is displayed - notice the
wording of 'hold' not 'reserve' is used in the modal box.
15. Select 'OK' on the modal and notice all un-checked item checkboxes
are automatically disabled
16. Repeat steps 12-15 this time with the value of
'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
you have clicked on the second item checkbox although the modal still
loads the item checkboxes do not disable - you are able to override and
continue selecting checkboxes each time with modal warning loading.

17. In both the OPAC and staff client try placing a record level hold,
selecting a value from the 'Holds to place (count)' dropdown. Notice
that the maximum value of this is the lowest value out of the 'maximum
number of holds per record', 'maximum number of holds per day' and the
'maxreserves' system preference so in this case it will be 2
18. Save and notice 2 holds are placed

19. Observe the allow_holds() function is in the Koha/Biblio.pm file and
the GetAllowedHoldsForPatronToday() function has been removed from
C4/Reserves.pm

Sponsored-By: Brimbank Library, Australia
Comment 219 Aleisha Amohia 2019-04-15 02:48:52 UTC
Created attachment 87990 [details] [review]
Bug 15565: (follow-up) Place multiple holds from opac-search.pl

Previously before this patch if you selected multiple bibliographic
results from opac-search.pl and selected 'Place hold' and confirmed the
hold only 1 hold on the first of the selected biblios would be placed.

This patch fixes this bug and enables users to be able to place holds on
multiple bibliographic records in opac-search.pl, select 'Place hold', select if they want
each of these holds to be on 'Next available', 'Next available from
group', or 'A specific item' and place their holds and all the holds
they placed will be placed correctly.

Test plan:
1. Follow test plan on the last patch - (Don't apply this patch)
2. Try to select multiple bibliographic record checkboxes and select
'Place hold'
3. Confirm holds and notice you are re-directed to opac-user.pl and only
the first hold has been placed

4. Apply this patch
5. Restart memcached and plack if running
6. Repeat steps 2 and 3 and notice all holds were placed
7. Repeat step 2 and then select a variety of 'A specific item' and 'Next
available item' holds for each of the bibliographic holds and
confirm they are all placed correctly
8. Confirm you can still place multiple holds correctly in opac-reserve
accessing from a OPAC biblio detail page

Sponsored-By: Brimbank Library, Australia
Comment 220 Aleisha Amohia 2019-04-15 02:49:01 UTC
Created attachment 87991 [details] [review]
Bug 15565: (follow-up) fixing typo that breaks placing a hold
Comment 221 Aleisha Amohia 2019-04-15 22:16:24 UTC
Rebased, ready for testing
Comment 222 David Cook 2020-12-18 05:55:04 UTC
Looks like Owen set this to "Patch doesn't apply" back on 2019-07-31, and I can confirm that is the case now in 2020-12-18.
Comment 223 David Cook 2020-12-18 05:59:06 UTC
Would it be possible to get squash these patches? I think rebasing 13 patches where the first one is failing is going to be not a very pleasurable task.

It looks like these are the files changed:

Biblio.pm
MultiplePerRecord.t
Reserves.pm
opac-reserve.pl
opac-reserve.tt
opac.scss
placerequest.pl
request.pl
request.tt
Comment 224 David Cook 2020-12-18 06:01:55 UTC
(In reply to David Cook from comment #223)
> Would it be possible to get squash these patches? I think rebasing 13
> patches where the first one is failing is going to be not a very pleasurable
> task.
> 
> It looks like these are the files changed:
> 
> Koha::Biblio.pm
> MultiplePerRecord.t
> Reserves.pm
> opac-reserve.pl
> opac-reserve.tt
> opac.scss
> placerequest.pl
> request.pl
> request.tt
Comment 225 David Cook 2020-12-18 06:34:52 UTC
Trying to find a base to merge these into easily so I can squash them and it's tough. 18.11, 19.05, 19.11... I never knew that Koha's holds code was changed so frequently. 

Aleisha, even if you don't squash them, could you let me know the parent commit for these patches from your last rebase?
Comment 226 Jonathan Druart 2020-12-18 10:05:35 UTC
(In reply to David Cook from comment #225)
> Aleisha, even if you don't squash them, could you let me know the parent
> commit for these patches from your last rebase?

See https://gist.github.com/jajm/f0b77da1a41b6b1c5ab1
Comment 227 David Cook 2020-12-22 02:27:51 UTC
(In reply to Jonathan Druart from comment #226)
> (In reply to David Cook from comment #225)
> > Aleisha, even if you don't squash them, could you let me know the parent
> > commit for these patches from your last rebase?
> 
> See https://gist.github.com/jajm/f0b77da1a41b6b1c5ab1

I've reviewed it based on your suggestion, but it doesn't actually work. At least not with git 2.16.4. 

The git rev-list appears to be random, so returning the first result is like picking a name out of a hat. This can be verified by comparing the results of the following command:

`git rev-list -n 15 --before="'Tue, 16 Apr 2019 23:00:05 +0000'" origin/master | less`

Even if the list weren't random, the dates are from patch creation time rather than merge time, so it would still be a shot in the dark.
Comment 228 David Cook 2020-12-22 02:36:17 UTC
It's actually quite interesting...

I just tried `git log --before="2019-04-16"`, which looks good at a glance, but it doesn't include f668fecec3771b633068f89dcead2312471f9425. Yet, f668fecec3771b633068f89dcead2312471f9425 should be before "2019-04-16".

Something weird going on there.
Comment 229 David Cook 2020-12-22 02:57:05 UTC
We can trust that commit 1b5635bebd75c418fdda68036e1e91f857cb1e4d was probably made and applied on May 30 2019 as it includes the release notes for 19.05.00.

Aleisha posted a comment about patches being rebased on April 15 2019, but the patches conflict with commits from April 11 2019...

But then if I go before those I'm still getting lots of conflicts.

Without that parent commit or a tonne of work, I don't think anyone can rebase these except Aleisha.
Comment 230 Aleisha Amohia 2020-12-22 03:04:01 UTC
I don't have time to get to this right now and probably won't for a while, but it is on my list.
Comment 231 David Cook 2020-12-22 03:44:22 UTC
(In reply to Aleisha Amohia from comment #230)
> I don't have time to get to this right now and probably won't for a while,
> but it is on my list.

I'm happy to collaborate on it as it's increased in priority here. 

I was just being grumpy before, but I can try rebasing it now against master. 

I'm off from Dec 25 through Jan 3, so I suppose there is no rush, but I'm hoping to tick this one off sooner rather than later.
Comment 232 David Cook 2020-12-22 05:58:58 UTC
I've put in some effort into rebasing these patches (and skipping many of them), but there's so many different changes that it's difficult to keep it all straight and the existing test plans are chaos.

Rather than proceeding with this patch, I reckon that it's smarter to split this patch into separate patches that deal with 1 change at a time (e.g. adding checkboxes to the staff client, adding checkboxes to the OPAC, changing maximum hold logic, etc.)

I think that Bug 23457 is a good candidate for adding checkboxes to the staff client, and I think that's probably where I'm going to focus my energy.
Comment 233 David Cook 2020-12-22 06:01:46 UTC
On the plus side, rebasing actually exposed a bug in master! So yay!
Comment 234 David Cook 2020-12-22 06:50:02 UTC
(Apologies if my above comments came across as rude or disrespectful. I'm not trying to criticize anyone. I'm just really keen to move forward, as I have a library that really wants multiple holds at once functionality ASAP, and I think breaking up the patches might be a good way to move forward.)
Comment 235 Jonathan Druart 2021-01-04 10:39:40 UTC
Patches applied correctly for me:

% git log --before="2019-04-16" --oneline|head -n1
7e218b81e6 Bug 21953: (follow-up) Fix test count

% git checkout -b bug_15565 7e218b81e6
% yes|git bz apply 15565

Bug 15565 - Place multiple holds at once for the same record in OPAC

87979 - Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
87980 - Bug 15565: Use item's biblionumber only on item-level request
87981 - Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off
87982 - Bug 15565: Alert librarian that maximum holds per record is reached
87983 - Bug 15565: Remove use of GetBiblionumberFromItemnumber
87984 - Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC
87985 - Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl
87986 - Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds
87987 - Bug 15565: (follow-up) Removed leftover debug code and added missing filters
87988 - Bug 15565: (QA follow-up) Fix some last filters
87989 - Bug 15565: (follow-up) Changes to maximum hold allowed logic
87990 - Bug 15565: (follow-up) Place multiple holds from opac-search.pl
87991 - Bug 15565: (follow-up) fixing typo that breaks placing a hold

Applying: Bug 15565: Place multiple holds at once for the same record on staff and OPAC.
Applying: Bug 15565: Use item's biblionumber only on item-level request
Applying: Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off
Applying: Bug 15565: Alert librarian that maximum holds per record is reached
Applying: Bug 15565: Remove use of GetBiblionumberFromItemnumber
Applying: Bug 15565: Fixed issues preventing any item-level holds being placed in OPAC
Applying: Bug 15565: Changed the retrieval of selected hold pickup branch in opac-reserve.pl
Applying: Bug 15565: Allow up to (and including) maxreserves number of OPAC item levelholds
Applying: Bug 15565: (follow-up) Removed leftover debug code and added missing filters
Applying: Bug 15565: (QA follow-up) Fix some last filters
Applying: Bug 15565: (follow-up) Changes to maximum hold allowed logic
Applying: Bug 15565: (follow-up) Place multiple holds from opac-search.pl
Applying: Bug 15565: (follow-up) fixing typo that breaks placing a hold
Apply? [(y)es, (n)o, (i)nteractive] %             


Pushed to a remote branch if needed: https://gitlab.com/joubu/Koha/-/tree/bug_15565
Comment 236 David Cook 2021-02-01 03:40:50 UTC
Thanks, Jonathan. I haven't looked at it yet, but I'm hoping I'll get the time to do so at some point.
Comment 237 Aleisha Amohia 2021-02-10 20:12:03 UTC
I have a bit of time now, tackling this again. I'll try and fix the patches so they are easier to work with.
Comment 238 Martha Fuerst 2021-02-10 21:44:04 UTC
Thanks, Aleisha! We're very interested in this.

-Martu Fuerst, Huntsville-Madison County Public Library
Comment 239 David Cook 2021-02-10 22:33:26 UTC
(In reply to Aleisha Amohia from comment #237)
> I have a bit of time now, tackling this again. I'll try and fix the patches
> so they are easier to work with.

Awesome! Feel free to ping me when you're ready and I'll happily test.
Comment 240 Aleisha Amohia 2021-02-22 05:10:55 UTC
Created attachment 117128 [details] [review]
Bug 15565: Koha::Biblio->allowed_holds and tests
Comment 241 Aleisha Amohia 2021-02-22 05:11:03 UTC
Created attachment 117129 [details] [review]
Bug 15565: Place multiple holds on one record on staff client

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT
Comment 242 Aleisha Amohia 2021-02-22 05:11:11 UTC
Created attachment 117130 [details] [review]
Bug 15565: Place multiple holds on one record on OPAC

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT
Comment 243 David Cook 2021-02-22 06:42:55 UTC
Created attachment 117131 [details] [review]
Bug 15565: Koha::Biblio->allowed_holds and tests

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 244 David Cook 2021-02-22 06:43:03 UTC
Created attachment 117132 [details] [review]
Bug 15565: Place multiple holds on one record on staff client

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 245 David Cook 2021-02-22 06:43:10 UTC
Created attachment 117133 [details] [review]
Bug 15565: Place multiple holds on one record on OPAC

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 246 David Cook 2021-02-22 06:43:45 UTC
Looking good, Aleisha!
Comment 247 David Cook 2021-02-25 01:28:16 UTC
If someone else signs this off, I could potentially QA it...
Comment 248 Martha Fuerst 2021-02-25 15:56:32 UTC
I've tried testing this using the Bywater sandboxes, but I can't get the pop-up to show up with the current test plan (step 6). When I set maxreserves to 3, I still don't get an alert when trying to place 4 holds, but only 3 holds are placed.
Comment 249 David Cook 2021-03-17 23:11:04 UTC
(In reply to Martha Fuerst from comment #248)
> I've tried testing this using the Bywater sandboxes, but I can't get the
> pop-up to show up with the current test plan (step 6). When I set
> maxreserves to 3, I still don't get an alert when trying to place 4 holds,
> but only 3 holds are placed.

I'll test this one again when I have a chance.
Comment 250 David Cook 2021-05-03 01:32:35 UTC
Created attachment 120365 [details] [review]
Bug 15565: Koha::Biblio->allowed_holds and tests

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 251 David Cook 2021-05-03 01:32:44 UTC
Created attachment 120366 [details] [review]
Bug 15565: Place multiple holds on one record on staff client

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 252 David Cook 2021-05-03 01:32:52 UTC
Created attachment 120367 [details] [review]
Bug 15565: Place multiple holds on one record on OPAC

Test plan:
1) Apply patch and restart services
2) Set the maxreserves system preference to 3
3) Set the following circulation rules:
holds allowed (total) = 10
holds allowed (daily) = 10
holds per record (count) = 10
3) Set up a record with 4 items
4) Go to place a hold on this record
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox unchecks itself.
7) Try setting different values for the above circulation rules that are
lower than the number of items on this record. Confirm the alert pops up
as expected.
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Confirm that item-level holds are placed on all selected items.

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 253 David Cook 2021-05-03 01:33:59 UTC
Slightly updated test plan. Everything is working well in koha-testing-docker.

Test plan:
0) git bz apply 15565
1) restart_all
2) Set the maxreserves system preference to 3 (from 50)
3) Set the following circulation rules:
holds allowed (total) = 10 (from unlimited)
holds allowed (daily) = 10 (from unlimited)
holds per record (count) = 10 (from 2)
3) Set up a record with 4 items
E.g. add 3 items to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4) Go to place a hold on this record in Staff Interface
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox is not checked.
7) Set the following circulation rules:
holds allowed (total) = 2 (from 10)
holds allowed (daily) = 2 (from 10)
holds per record (count) = 2 (from 10)
8) Perform the same above actions on the OPAC
Comment 254 David Cook 2021-05-06 03:01:40 UTC
Going to set this back to "Needs Signoff" so it's clearer that someone other than me should test it.
Comment 255 David Cook 2021-05-06 07:03:33 UTC
(In reply to David Cook from comment #254)
> Going to set this back to "Needs Signoff" so it's clearer that someone other
> than me should test it.

But FYI it is passing the QA tools:

root@kohadevbox:koha(bug_15565)$ qa
testing 3 commit(s) (applied to 640ba47 'cf Bug 28288: Prevent XSISBN.t to fai')

Processing files before patches
|========================>| 7 / 7 (100.00%)
Processing files after patches
|========================>| 7 / 7 (100.00%)

 OK     Koha/Biblio.pm
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
 OK     koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
 OK     opac/opac-reserve.pl
 OK     reserve/placerequest.pl
 OK     reserve/request.pl
 OK     t/db_dependent/Reserves/MultiplePerRecord.t

Processing additional checks OK!
Comment 256 Victor Grousset/tuxayo 2021-05-06 19:14:52 UTC
Created attachment 120639 [details] [review]
Bug 15565: Koha::Biblio->allowed_holds and tests

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 257 Victor Grousset/tuxayo 2021-05-06 19:15:02 UTC
Created attachment 120640 [details] [review]
Bug 15565: Place multiple holds on one record on OPAC

Test plan: See previous commit about the staff interface

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 258 Victor Grousset/tuxayo 2021-05-06 19:15:12 UTC
Created attachment 120641 [details] [review]
Bug 15565: Place multiple holds on one record on staff client

Test plan:
0) git bz apply 15565
1) restart_all
2) Set the maxreserves system preference to 3 (from 50)
3) Set the following circulation rules:
holds allowed (total) = 10 (from unlimited)
holds allowed (daily) = 10 (from unlimited)
holds per record (count) = 10 (from 2)
3) Set up a record with 4 items
E.g. add 3 items to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29
4) Go to place a hold on this record in Staff Interface
5) Notice that the items table under 'Place a hold on a specific item'
now uses checkboxes instead of radio buttons
6) Check all 4 checkboxes. Confirm that when you check the 4th checkbox,
    an alert pops up warning the you've reached the maximum. Confirm
    that when you close the alert, the checkbox is not checked.
7) Set the following circulation rules one by one and test that each
one alone can trigger the alert:
holds allowed (total) = 2 (from 10)
holds allowed (daily) = 2 (from 10)
holds per record (count) = 2 (from 10)
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Perform the same above actions on the OPAC

Sponsored-by: Catalyst IT

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 259 Victor Grousset/tuxayo 2021-05-06 19:18:02 UTC
It works! :D

===

Is that expected that Holds allowed (total) isn't taken into account?
        my $lowestvalue = min( $maxreserves, $remaining_holds_for_record, $remaining_holds_allowed_today );

===

I my 1st test, I had a situation where
maxreserves 3
holds allowed (total) = 2
holds allowed (daily) = 2
holds per record (count) = 2
And when I ticked one tick, the alert came.
The patron had 0 holds and so the record.
I did a reset_all and couldn't reproduce.
So there might be something wrong, in case this symptom gives you an hint. If not, let's move on I would say. Can't be blocked by a ghost bug that might not actually exist.

===

Took David's test plan and put it in the commit.
And did the following changes:

7) Set the following circulation rules:
↓↓↓
7) Set the following circulation rules one by one and test that each
one alone can trigger the alert:

8) Perform the same above actions on the OPAC
↓↓↓
8) Set the circulation rules and syspref all to 4+. Check some items and
confirm the holds.
9) Perform the same above actions on the OPAC

===

oops, inverted two patches during a rebase. Anyway, still applies on top of master.
Comment 260 David Cook 2021-05-10 00:00:36 UTC
(In reply to Victor Grousset/tuxayo from comment #259)
> Is that expected that Holds allowed (total) isn't taken into account?
>         my $lowestvalue = min( $maxreserves, $remaining_holds_for_record,
> $remaining_holds_allowed_today );

Sounds like a bug to me.

Aleisha?
Comment 261 David Cook 2021-05-13 01:57:45 UTC
*** Bug 24309 has been marked as a duplicate of this bug. ***
Comment 262 David Cook 2021-05-13 01:59:03 UTC
Updating title from "Place multiple holds at once for the same record in OPAC" to "Place multiple item-level holds at once for the same record in OPAC and staff interface" since I think that's more accurate.
Comment 263 David Cook 2021-05-13 02:01:30 UTC
Marking as Failed QA due to Victor's comment about "Holds allowed (total)".

I think I should be more thorough in my testing of the different hold counts too, even if it means making the test plan longer.
Comment 264 David Cook 2021-05-27 06:06:31 UTC
Test plan:
0) git bz apply 15565
1) restart_all
2) Set up a record with 4 items
E.g. add 3 items to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29

Staff Interface:
3) Set the maxreserves system preference to 2 (from 50) 
4) Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?borrowernumber=51&biblionumber=29
5) Try to place a hold on all the items

Unfortunately, as soon as I choose 1 item, it says "Patron reached the maximum number of holds". 

It looks like these patches don't work with "Unlimited" holds, so this functionality is fundamentally broken...
Comment 265 David Cook 2021-05-27 06:38:30 UTC
Ok so there are 4 different settings that control the allowable number of holds:

maxreserves (global system preference)
"Holds allowed (total)" (circ rule: reservesallowed)
"Holds allowed (daily)" (circ rule: holds_per_day)
"Holds per record (count)" (circ rule: holds_per_record)

C4::Reserves is a mess...

If the following are blank/empty (notably not null), then 15565 doesn't work. 
select * from circulation_rules where rule_name IN ('reservesallowed', 'holds_per_day', 'holds_per_record');
Comment 266 David Cook 2021-05-27 06:51:58 UTC
I need to finish up for the day, but... I reckon we need something like this:

Koha::Holds->remaining_reserves_total({ patron => $patron, });
maxreserves (global system preference)
"Holds allowed (total)" (circ rule: reservesallowed)

Koha::Holds->remaining_reserves_record({ patron => $patron, record => $biblio, });
"Holds per record (count)" (circ rule: holds_per_record)

Koha::Holds->remaining_reserves_today({ patron => $patron, record => $biblio, });
"Holds allowed (daily)" (circ rule: holds_per_day)

And each needs to consider an unlimited number (although in theory maxreserves should always be a positive integer).

Then when you're checking boxes you check each of those totals in turn. (The reason for having separate functions is so that you can tell the user what limit they've exceeded...). 

An unlimited rule should probably return a null response, so that you only consider counts that are digits.
Comment 267 Aleisha Amohia 2021-06-15 01:50:46 UTC
Created attachment 121941 [details] [review]
Bug 15565: Work in progress, calculating holds remaining

Not ready for testing!
Comment 268 David Cook 2021-06-18 02:23:11 UTC
I think I found another problem.

From the results page, it looks like you can place multiple item holds for multiple bibs, but it doesn't work. It just places a hold on the first item for each bib.
Comment 269 David Cook 2021-06-18 02:26:50 UTC
It looks like the "Unlimited" bug happens on the staff interface but not the OPAC...
Comment 270 David Cook 2021-06-18 05:26:21 UTC
In the OPAC, you can select multiple specific items for multiple records at the same time from the result page, but not all of your selections make it into the holds. It looks like just the first selection. 

I'm looking at patching this now.
Comment 271 David Cook 2021-06-18 06:09:17 UTC
Created attachment 122098 [details] [review]
Bug 15565: Allow multiple hold for multiple bib in OPAC

This patch allows an OPAC user to select multiple item holds
from multiple bibs when placing holds/reserves from the OPAC
search result page.
Comment 272 David Cook 2021-07-08 02:22:34 UTC
Getting a merge conflict for "Place multiple holds on one record on staff client"

I'll see if I can work it out..
Comment 273 David Cook 2021-07-08 04:30:52 UTC
I fixed the merge conflict but the Javascript in the staff interface doesn't work anymore. Probably due to other changes that have happened to the templates on master.
Comment 274 David Cook 2021-09-21 00:59:26 UTC
(In reply to David Cook from comment #273)
> I fixed the merge conflict but the Javascript in the staff interface doesn't
> work anymore. Probably due to other changes that have happened to the
> templates on master.

This is so weird... I swear that I successfully backported to 21.05 but now it's not working...
Comment 275 David Cook 2021-09-21 01:40:59 UTC
(In reply to David Cook from comment #274)
> (In reply to David Cook from comment #273)
> > I fixed the merge conflict but the Javascript in the staff interface doesn't
> > work anymore. Probably due to other changes that have happened to the
> > templates on master.
> 
> This is so weird... I swear that I successfully backported to 21.05 but now
> it's not working...

Ah, those Javascript issues are with hold limits. The problem I encountered was due to Bug 28338...
Comment 276 David Cook 2022-02-28 03:03:11 UTC
As per my comment at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14695#c242, it looks like multi-hold doesn't properly process Pickup location at the item-level...
Comment 277 David Cook 2022-11-15 05:12:34 UTC
Aleisha, do you have clients using this functionality?
Comment 278 Aleisha Amohia 2022-11-15 19:59:17 UTC
(In reply to David Cook from comment #277)
> Aleisha, do you have clients using this functionality?

Catalyst had received sponsorship to work on this for a library years ago, they may still be using a version of this in production but I'm not sure as we don't work with that library anymore and sponsorship has run out.
Comment 279 David Cook 2024-02-12 05:44:41 UTC
Looking at this code again, and I think one of the reasons it never got accepted was that it was trying to fix too many sins in the Reserves/Holds area. 

We'd be better off getting rid of placerequest.pl all together. We do have the /api/v1/holds endpoint these days, so it is tempting to actually replace it with some Javascript.

I think I might try to do a quick and dirty server-side version locally, but I think the API-driven workflow is the future for the community.
Comment 280 David Cook 2024-02-12 05:45:06 UTC
(In reply to David Cook from comment #279)
> Looking at this code again, and I think one of the reasons it never got
> accepted was that it was trying to fix too many sins in the Reserves/Holds
> area. 
> 
> We'd be better off getting rid of placerequest.pl all together. We do have
> the /api/v1/holds endpoint these days, so it is tempting to actually replace
> it with some Javascript.
> 
> I think I might try to do a quick and dirty server-side version locally, but
> I think the API-driven workflow is the future for the community.

Of course, that's only for the staff-side. We don't have the API endpoint for the OPAC side.
Comment 281 David Cook 2024-02-12 23:14:00 UTC
I've rewritten the staff-interface code to be much more minimal yet still effective. Its error handling is patterned off the OPAC, which makes it consistent across the interfaces as well.
Comment 282 David Cook 2024-02-12 23:44:31 UTC
(In reply to David Cook from comment #281)
> I've rewritten the staff-interface code to be much more minimal yet still
> effective. Its error handling is patterned off the OPAC, which makes it
> consistent across the interfaces as well.

The difference with the past patches is that we don't get the warnings/restrictions in the UI *before* placing the hold. We only get the warnings about failures *after* trying to place the hold.

I think warning *before* the hold is actually a separate feature.
Comment 283 David Cook 2024-02-13 00:25:37 UTC
(In reply to David Cook from comment #282)
> The difference with the past patches is that we don't get the
> warnings/restrictions in the UI *before* placing the hold. We only get the
> warnings about failures *after* trying to place the hold.
> 
> I think warning *before* the hold is actually a separate feature.

I actually don't know if there is a practical bug-free way of doing it *before*, since each individual item can have different rules. There's a complex interplay of rules. 

For instance, item A could have 1 "Holds per record", while item B could have 10 "Holds per record". Trying to pre-emptively calculate holdability on selected holds rather than placed holds... is tough to do without potential for errors.

One could argue that it's unlikely a bib record would have items of different types, but I see it all the time. 

That said, only warning *after* trying to place the holds isn't great, because it might not be obvious which holds were placed and which weren't placed. 

But we can't do much about that while our hold placing scripts use redirects. Replacing those redirects with API calls would work better, because the user would be able to see immediately which holds were placed and which weren't. (But that also involves messing around with the DOM in unfun ways - unless we use Vue.)
Comment 284 David Cook 2024-02-13 00:55:36 UTC
Created attachment 162071 [details] [review]
Bug 15565: Place multiple holds on one record on staff interface
Comment 285 David Cook 2024-02-13 00:55:43 UTC
Created attachment 162072 [details] [review]
Bug 15565: Tidy reserve/placerequest.pl
Comment 286 David Cook 2024-02-13 00:55:50 UTC
Created attachment 162073 [details] [review]
Bug 15565: Place multiple holds on one or more records in OPAC
Comment 287 David Cook 2024-02-13 00:55:56 UTC
Created attachment 162074 [details] [review]
Bug 15565: Add 'DisplayMultiItemHolds' system preference to toggle display of multi item holds

These patches allow the display of checkboxes instead of radio buttons for placing specific
item holds in the OPAC and staff interface. This display is controlled by the system preference
'DisplayMultiItemHolds'. When disabled, the hold display is the same as in the past. When enabled,
the user can select multiple items to place holds on at the same time.

If there is an error placing one or more holds, an error will display on the following screen.

Test plan:
0. Apply the patches and koha-plack --reload kohadev
1. Enable 'DisplayMultiItemHolds' syspref
2. In staff interface, go to a record with multiple holdable items
3. Check the boxes next to 2+ items and click the button to place the holds
4. Note that the holds are all placed (unless there is a failure)
5. Cancel the holds, and try again (but this time make Holds per record (count) only
1 in the circ rules)
6. Note that one of the holds is placed, but the others are not and an error message
is displayed saying that one or more holds could not be placed because the holds per
record was exceeded

7. Perform the same above procedure on the OPAC
8. If DisplayMultiPlaceHold is enabled, you can test placing multiple specific item holds
across multiple bibs in the OPAC via the search results
9. If EnableItemGroups and EnableItemGroupHolds is enabled, you can test that you can
either place item group holds or specific item holds. Toggling the checkboxes in the
staff interface will uncheck the item group hold radio button.
Comment 288 David Cook 2024-02-13 01:09:01 UTC
I noticed there is a FAIL in the QA tools, but it's not related to this change:

--

testing 4 commit(s) (applied to d130a6c '6b Bug 35962: (bug 35843 follow-up 2)')

Processing files before patches
The PO directory has not been found. There is a problem in your Koha installation. at /kohadevbox/koha/Koha/I18N.pm line 197.
|========================>| 5 / 5 (100.00%)
Processing files after patches
|========================>| 5 / 5 (100.00%)

 FAIL   installer/data/mysql/mandatory/sysprefs.sql
   FAIL   sysprefs_order
                Not blocker: Sysprefs hidelostitems is bad placed (see bug 10610)

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt

 OK     koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt

 OK     reserve/placerequest.pl

 OK     reserve/request.pl


Processing additional checks OK!
Comment 289 Lisette Scheer 2024-03-07 23:27:47 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Comment 290 Nick Clemens 2024-03-11 19:06:10 UTC
Created attachment 163052 [details] [review]
Bug 15565: Place multiple holds on one record on staff interface
Comment 291 Nick Clemens 2024-03-11 19:06:16 UTC
Created attachment 163053 [details] [review]
Bug 15565: Tidy reserve/placerequest.pl
Comment 292 Nick Clemens 2024-03-11 19:06:22 UTC
Created attachment 163054 [details] [review]
Bug 15565: Place multiple holds on one or more records in OPAC
Comment 293 Nick Clemens 2024-03-11 19:06:28 UTC
Created attachment 163055 [details] [review]
Bug 15565: Add 'DisplayMultiItemHolds' system preference to toggle display of multi item holds

These patches allow the display of checkboxes instead of radio buttons for placing specific
item holds in the OPAC and staff interface. This display is controlled by the system preference
'DisplayMultiItemHolds'. When disabled, the hold display is the same as in the past. When enabled,
the user can select multiple items to place holds on at the same time.

If there is an error placing one or more holds, an error will display on the following screen.

Test plan:
0. Apply the patches and koha-plack --reload kohadev
1. Enable 'DisplayMultiItemHolds' syspref
2. In staff interface, go to a record with multiple holdable items
3. Check the boxes next to 2+ items and click the button to place the holds
4. Note that the holds are all placed (unless there is a failure)
5. Cancel the holds, and try again (but this time make Holds per record (count) only
1 in the circ rules)
6. Note that one of the holds is placed, but the others are not and an error message
is displayed saying that one or more holds could not be placed because the holds per
record was exceeded

7. Perform the same above procedure on the OPAC
8. If DisplayMultiPlaceHold is enabled, you can test placing multiple specific item holds
across multiple bibs in the OPAC via the search results
9. If EnableItemGroups and EnableItemGroupHolds is enabled, you can test that you can
either place item group holds or specific item holds. Toggling the checkboxes in the
staff interface will uncheck the item group hold radio button.
Comment 294 Lisette Scheer 2024-03-12 15:26:01 UTC
Created attachment 163092 [details] [review]
Attachment to Bug 15565 - Place multiple item-level holds at once for the same record

Bug 15565: Place multiple holds on one record on staff interface

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 295 Lisette Scheer 2024-03-12 15:27:40 UTC
Created attachment 163093 [details] [review]
Bug 15565: Place multiple holds on one record on staff interface

Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 296 Lisette Scheer 2024-03-12 15:28:55 UTC
Worked great in all cases. Thanks David!