Bug 15565 - Place multiple holds at once for the same record in OPAC
Summary: Place multiple holds at once for the same record in OPAC
Status: ASSIGNED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Alex Buckley
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 14695 15486
Blocks: 15567
  Show dependency treegraph
 
Reported: 2016-01-13 13:10 UTC by Julian Maurice
Modified: 2018-12-18 02:20 UTC (History)
22 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Allow borrowers to place multiple item-level holds on a record in the OPAC. Item radio buttons are replaced with checkboxes, and once a borrower has reached the circulation rules 'Holds per record (count)' value the remaining unticked checkboxes are disabled. This feature was previously only available in the staff client.


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, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bz 15565 - CSS changes (81.62 KB, patch)
2017-03-23 16:17 UTC, sandboxes@biblibre.com
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, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 15565: Set 'found' parameter correctly when ReservesNeedReturns is off (1.61 KB, patch)
2017-03-23 16:17 UTC, sandboxes@biblibre.com
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, sandboxes@biblibre.com
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

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
Created attachment 56512 [details] [review]
Bz 15565 - CSS changes
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 sandboxes@biblibre.com 2017-03-23 16:16:14 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 44 sandboxes@biblibre.com 2017-03-23 16:17:19 UTC Comment hidden (obsolete)
Comment 45 sandboxes@biblibre.com 2017-03-23 16:17:24 UTC Comment hidden (obsolete)
Comment 46 sandboxes@biblibre.com 2017-03-23 16:17:28 UTC Comment hidden (obsolete)
Comment 47 sandboxes@biblibre.com 2017-03-23 16:17:32 UTC Comment hidden (obsolete)
Comment 48 sandboxes@biblibre.com 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
Created attachment 71643 [details] [review]
Bug 15565 - Place multiple holds at once for the same record on staff and OPAC.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 69 Alex Arnaud 2018-02-15 11:14:09 UTC
Created attachment 71644 [details] [review]
Bug 15565 - CSS changes

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 70 Alex Arnaud 2018-02-15 11:14:20 UTC
Created attachment 71645 [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>
Comment 71 Alex Arnaud 2018-02-15 11:14:30 UTC
Created attachment 71646 [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>
Comment 72 Alex Arnaud 2018-02-15 11:14:42 UTC
Created attachment 71647 [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>
Comment 73 Alex Arnaud 2018-02-15 11:14:52 UTC
Created attachment 71648 [details] [review]
Bug 15565 - remove use of GetBiblionumberFromItemnumber
Comment 74 Benjamin Rokseth 2018-03-12 11:04:02 UTC
Created attachment 72644 [details] [review]
Bug 15565 - Place multiple holds at once for the same record on staff and OPAC.

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>
Comment 75 Benjamin Rokseth 2018-03-12 11:04:13 UTC
Created attachment 72645 [details] [review]
Bug 15565 - CSS changes

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>
Comment 76 Benjamin Rokseth 2018-03-12 11:04:23 UTC
Created attachment 72646 [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>
Comment 77 Benjamin Rokseth 2018-03-12 11:04:31 UTC
Created attachment 72647 [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>
Comment 78 Benjamin Rokseth 2018-03-12 11:04:41 UTC
Created attachment 72648 [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>
Comment 79 Benjamin Rokseth 2018-03-12 11:04:51 UTC
Created attachment 72649 [details] [review]
Bug 15565 - remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
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
Created attachment 78242 [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>
Comment 84 Alex Buckley 2018-08-29 05:59:26 UTC
Created attachment 78243 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 85 Alex Buckley 2018-08-29 06:00:04 UTC
Created attachment 78244 [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>
Comment 86 Alex Buckley 2018-08-29 06:00:28 UTC
Created attachment 78245 [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>
Comment 87 Alex Buckley 2018-08-29 06:00:51 UTC
Created attachment 78246 [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>
Comment 88 Alex Buckley 2018-08-29 06:01:17 UTC
Created attachment 78247 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Comment 89 Alex Buckley 2018-08-29 06:01:45 UTC
Created attachment 78248 [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
Comment 90 Alex Buckley 2018-08-29 13:26:01 UTC
Created attachment 78266 [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
Comment 91 Alex Buckley 2018-08-30 06:18:33 UTC
Created attachment 78305 [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
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
Created attachment 78417 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 94 Alex Buckley 2018-09-04 21:01:32 UTC
Created attachment 78418 [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>
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
Created attachment 78419 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 97 Alex Buckley 2018-09-04 21:11:58 UTC
Created attachment 78420 [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>
Comment 98 Alex Buckley 2018-09-04 21:12:31 UTC
Created attachment 78421 [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>
Comment 99 Alex Buckley 2018-09-04 21:12:58 UTC
Created attachment 78422 [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>
Comment 100 Alex Buckley 2018-09-04 21:13:24 UTC
Created attachment 78423 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Comment 101 Alex Buckley 2018-09-04 21:13:53 UTC
Created attachment 78424 [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
Comment 102 Alex Buckley 2018-09-04 21:14:40 UTC
Created attachment 78425 [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
Comment 103 Alex Buckley 2018-09-04 21:15:23 UTC
Created attachment 78426 [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
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
Created attachment 78712 [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>
Comment 107 Chris Cormack 2018-09-14 19:27:14 UTC
Created attachment 78713 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 108 Chris Cormack 2018-09-14 19:27:21 UTC
Created attachment 78714 [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>
Comment 109 Chris Cormack 2018-09-14 19:27:27 UTC
Created attachment 78715 [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>
Comment 110 Chris Cormack 2018-09-14 19:27:33 UTC
Created attachment 78716 [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>
Comment 111 Chris Cormack 2018-09-14 19:27:39 UTC
Created attachment 78717 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Comment 112 Chris Cormack 2018-09-14 19:27:47 UTC
Created attachment 78718 [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
Comment 113 Chris Cormack 2018-09-14 19:27:55 UTC
Created attachment 78719 [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
Comment 114 Chris Cormack 2018-09-14 19:28:01 UTC
Created attachment 78720 [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
Comment 115 Lisette Scheer 2018-09-14 22:34:42 UTC
Created attachment 78815 [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>
Comment 116 Lisette Scheer 2018-09-14 22:34:48 UTC
Created attachment 78816 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 117 Lisette Scheer 2018-09-14 22:34:54 UTC
Created attachment 78817 [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>
Comment 118 Lisette Scheer 2018-09-14 22:35:02 UTC
Created attachment 78818 [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>
Comment 119 Lisette Scheer 2018-09-14 22:35:12 UTC
Created attachment 78819 [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>
Comment 120 Lisette Scheer 2018-09-14 22:35:21 UTC
Created attachment 78820 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Comment 121 Lisette Scheer 2018-09-14 22:35:28 UTC
Created attachment 78821 [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
Comment 122 Lisette Scheer 2018-09-14 22:35:35 UTC
Created attachment 78822 [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
Comment 123 Lisette Scheer 2018-09-14 22:35:41 UTC
Created attachment 78823 [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>
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
Created attachment 81419 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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>
Comment 126 Alex Buckley 2018-10-28 18:43:28 UTC
Created attachment 81420 [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>
Comment 127 Alex Buckley 2018-10-28 18:43:54 UTC
Created attachment 81421 [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>
Comment 128 Alex Buckley 2018-10-28 18:44:23 UTC
Created attachment 81422 [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>
Comment 129 Alex Buckley 2018-10-28 18:44:51 UTC
Created attachment 81423 [details] [review]
Bug 15565: Remove use of GetBiblionumberFromItemnumber

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Comment 130 Alex Buckley 2018-10-28 18:45:39 UTC
Created attachment 81424 [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
Comment 131 Alex Buckley 2018-10-28 18:46:01 UTC
Created attachment 81425 [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
Comment 132 Alex Buckley 2018-10-28 18:46:51 UTC
Created attachment 81426 [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>
Comment 133 Alex Buckley 2018-10-28 18:47:17 UTC
Created attachment 81427 [details] [review]
Bug 15565: (follow-up) Removed leftover debug code and added missing filters

Sponsord-By: Brimbank Library, Australia
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
Created attachment 81431 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters
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
Created attachment 81449 [details] [review]
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic

Sponsored-By: Brimbank Library, Australia
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
Created attachment 81570 [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 141 Katrin Fischer 2018-10-29 21:17:54 UTC
Created attachment 81571 [details] [review]
Bug 15565: CSS changes

Note: Reworked as we have switched to CSS since this commit was first written.
This commit now contains 2 files:

* koha-tmpl/opac-tmpl/bootstrap/css/opac.css
* koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss

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 142 Katrin Fischer 2018-10-29 21:18:02 UTC
Created attachment 81572 [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 143 Katrin Fischer 2018-10-29 21:18:10 UTC
Created attachment 81573 [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 144 Katrin Fischer 2018-10-29 21:18:19 UTC
Created attachment 81574 [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 145 Katrin Fischer 2018-10-29 21:18:27 UTC
Created attachment 81575 [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 146 Katrin Fischer 2018-10-29 21:18:35 UTC
Created attachment 81576 [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 147 Katrin Fischer 2018-10-29 21:18:44 UTC
Created attachment 81577 [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 148 Katrin Fischer 2018-10-29 21:18:52 UTC
Created attachment 81578 [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 149 Katrin Fischer 2018-10-29 21:19:01 UTC
Created attachment 81579 [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 150 Katrin Fischer 2018-10-29 21:19:10 UTC
Created attachment 81580 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 151 Katrin Fischer 2018-10-29 21:19:18 UTC
Created attachment 81581 [details] [review]
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic

Sponsored-By: Brimbank Library, Australia

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
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
Created attachment 82205 [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 157 Alex Buckley 2018-11-10 12:00:33 UTC
Created attachment 82206 [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 158 Alex Buckley 2018-11-10 12:01:10 UTC
Created attachment 82207 [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 159 Alex Buckley 2018-11-10 12:01:34 UTC
Created attachment 82208 [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 160 Alex Buckley 2018-11-10 12:02:07 UTC
Created attachment 82209 [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 161 Alex Buckley 2018-11-10 12:02:40 UTC
Created attachment 82210 [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 162 Alex Buckley 2018-11-10 12:03:48 UTC
Created attachment 82211 [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 163 Alex Buckley 2018-11-10 12:04:43 UTC
Created attachment 82212 [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 164 Alex Buckley 2018-11-10 12:05:10 UTC
Created attachment 82213 [details] [review]
Bug 15565: (QA follow-up) Fix some last filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 165 Alex Buckley 2018-11-10 12:05:45 UTC
Created attachment 82214 [details] [review]
Bug 15565: (follow-up) Corrected OPAC reserve interface display logic

Sponsored-By: Brimbank Library, Australia

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 166 Alex Buckley 2018-11-10 12:06:44 UTC
Created attachment 82215 [details] [review]
Bug 15565: (follow-up) Added handling for holds_per_day and respecting AllowHoldPolicyOverride

On OPAC and staff side this commit adds handling to respect
the holds per day circ rules setting.

In the OPAC once either maxreserves, holds per day or holds per record
(depending on which of the 3 values is lowest) is reached then remaining
unchecked item checkboxes are disabled.

On the staff side the same happens however if the
AllowHoldPolicyOverride is enabled then the checkboxes do not disable
but the staff member still sees warning modal boxes appear.

Also added unit test for new function GetAllowedHoldsForPatronToday in
C4::Reserves.

Sponsored-By: Brimbank Library, Australia
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
Created attachment 82216 [details] [review]
Bug 15565 (follow-up): Removed leftover JS debug

Sponsored-By: Brimbank Library, Australia
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
Created attachment 82220 [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 a modal box loads to inform you
that you have reached the maximum holds allowed today.
6. Click 'OK' on the modal and notice the first checkbox is selected and
other unchecked checkboxes are disabled.
7. Repeat step 4 this time setting holds per day to 5
8. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays and select another item checkbox (so
you have 2 selected checkboxes). Observe a modal saying you have reached
the maximum number of holds for the record is loaded
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.
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. 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 172 Alex Buckley 2018-11-12 10:16:34 UTC
Created attachment 82221 [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 a modal box loads to inform you
that you have reached the maximum holds allowed today.
6. Click 'OK' on the modal and notice the first checkbox is selected and
other unchecked checkboxes are disabled.
7. Repeat step 4 this time setting holds per day to 5
8. Now reloading the OPAC 'Place hold' page notice after clicking 'A
specific item' no modal displays and select another item checkbox (so
you have 2 selected checkboxes). Observe a modal saying you have reached
the maximum number of holds for the record is loaded
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.
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. 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 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