Bug 15516

Summary: Allow to place a hold on first available item from a group of titles
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Hold requestsAssignee: Aleisha Amohia <aleisha>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: new feature    
Priority: P5 - low CC: aholt, aleisha, andrewfh, arm, benjamin.rokseth, black23, cbrannon, claire.hernandez, cslone, dcook, fiona.borthwick, fridolin.somers, gmcharlt, gwilliams, hattara, jonathan.druart, josef.moravec, katrin.fischer, kebliss, kyle, lisette.scheer, lisettepalouse+koha, m.de.rooy, martin.renvoize, pasi.kallinen, rbit, sally.healey, victor
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17217
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25260
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24857
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565
Change sponsored?: Sponsored Patch complexity: Large patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This features allows a patron to place a hold on the first available item from a group of titles on the staff interface or OPAC.
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15566, 21528, 17217, 27300    
Attachments: Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516 - Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Schema updates
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Schema updates
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Schema updates
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Bug 15516: (follow-up) Fixing atomic update to prevent errors
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Schema updates
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Bug 15516: (follow-up) Fixing atomic update to prevent errors
Bug 15516: Fixing missing curly bracket
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Update old_reserves (reserve_group_id)
Bug 15516: Set reserve-group.pl to be executable
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: Fix translation issues
Bug 15516: (follow-up) Fix kohastructure.sql
Bug 15516: (follow-up) Fix reserve -> hold change
Bug 15516: (follow-up) Fix modal for hold group
Bug 15516: (follow-up) Fix pod in C4::Reserves
Bug 15516: (follow-up) Fix template filters
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Schema updates
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Bug 15516: (follow-up) Fixing atomic update to prevent errors
Bug 15516: Fixing missing curly bracket
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Fix QA and rebase issues
Bug 15516: (follow-up) Fix db structure and atomic update
Bug 15516: (follow-up) Fix db structure and atomic update
Bug 15516: DO NOT PUSH - Schema changes
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Fix QA and rebase issues
Bug 15516: (follow-up) Fix db structure and atomic update
Bug 15516: DO NOT PUSH - Schema changes
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: (follow-up) Remove C4::Reserves::GetHoldGroupHolds
Bug 15516: (follow-up) Fix creating hold group in placerequest
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Fix QA and rebase issues
Bug 15516: (follow-up) Fix db structure and atomic update
Bug 15516: DO NOT PUSH - Schema changes
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: (follow-up) Remove C4::Reserves::GetHoldGroupHolds
Bug 15516: (follow-up) Fix creating hold group in placerequest
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: (follow-up) Fix showing list of titles in hold group
Bug 15516: (follow-up) Fix showing list of titles in hold group
Bug 15516: (follow-up) Remove loading-small.gif reference
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Holding first available item from x biblios only count 1 reserve
Bug 15516: Reword 'reserve group' to 'hold group'
Bug 15516: (follow-up) Fixing modal view and more
Bug 15516: (follow-up) Fix QA and rebase issues
Bug 15516: (follow-up) Fix db structure and atomic update
Bug 15516: DO NOT PUSH - Schema changes
Bug 15516: (follow-up) Move to Koha::Object[s]
Bug 15516: (follow-up) Fix showing list of titles in hold group
Bug 15516: (follow-up) Remove loading-small.gif reference
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: Fix shebang in hold-group.pl
Bug 15516: (follow-up) Count group holds always
Bug 15516: Fix hold groups for multi items for one bib
Bug 15516: Add ability to create hold groups from OPAC
Bug 15516: Show user's hold groups to user via OPAC
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: Fix shebang in hold-group.pl
Bug 15516: (follow-up) Count group holds always
Bug 15516: Fix hold groups for multi items for one bib
Bug 15516: Add ability to create hold groups from OPAC
Bug 15516: Show user's hold groups to user via OPAC
Bug 15516: (follow-up) Prevent cancelling unintended holds
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: Fix shebang in hold-group.pl
Bug 15516: (follow-up) Count group holds always
Bug 15516: Fix hold groups for multi items for one bib
Bug 15516: Add ability to create hold groups from OPAC
Bug 15516: Show user's hold groups to user via OPAC
Bug 15516: (follow-up) Prevent cancelling unintended holds
Bug 15516: (follow-up) Fix hold group details not showing
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Schema updates
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: Fix shebang in hold-group.pl
Bug 15516: (follow-up) Count group holds always
Bug 15516: Fix hold groups for multi items for one bib
Bug 15516: Add ability to create hold groups from OPAC
Bug 15516: Show user's hold groups to user via OPAC
Bug 15516: (follow-up) Prevent cancelling unintended holds
Bug 15516: (follow-up) Fix hold group details not showing
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: (follow-up) Add hold group message to holds queue
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
Bug 15516: (follow-up) Fixes for qa tools
Bug 15516: (follow-up) Fixing file permissions and title elements order
Bug 15516: Fix shebang in hold-group.pl
Bug 15516: (follow-up) Count group holds always
Bug 15516: Fix hold groups for multi items for one bib
Bug 15516: Add ability to create hold groups from OPAC
Bug 15516: Show user's hold groups to user via OPAC
Bug 15516: (follow-up) Prevent cancelling unintended holds
Bug 15516: (follow-up) Fix hold group details not showing
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Add ability to create and view hold groups from OPAC
Bug 15516: Schema updates
Bug 15516: Database and installer updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Add ability to create and view hold groups from OPAC
Bug 15516: Schema updates
Bug 15516: [QA followup] Only show hold groups for multi hold in OPAC
Bug 15516: [QA followup] Fix item-specific holds in staff interface
Bug 15516: [QA followup] Only show hold groups for multi hold in staff interface
Bug 15516: Database and installer updates
Bug 15516: Schema updates
Bug 15516: Relevant controller changes and tests
Bug 15516: Allow to reserve first available item from a group of titles
Bug 15516: Tidy reserve/placerequest.pl
Bug 15516: Add ability to create and view hold groups from OPAC
Bug 15516: (follow-up) Only show hold groups for multi hold in OPAC
Bug 15516: (follow-up) Only show hold groups for multi hold in staff interface
Bug 15516: Add 'DisplayAddHoldGroups' system preference to toggle ability to add titles to hold groups

Description Julian Maurice 2016-01-07 13:56:59 UTC
Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from different publishers (so 1 title but several biblio records) but the user only wants a copy of the book, regardless of the publisher.
Comment 1 Julian Maurice 2016-01-07 13:57:27 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-01-28 10:15:23 UTC
(In reply to Julian Maurice from comment #1)
> Comments:
> - A column is added to the reserves table (reserve_group_id) to group
>   the reserves that belong together, and an additional table
>   (reserve_group) with a single column is used to generate an auto
>   increment ID. I feel that something better can be done...

It's because you don't have any other attributes, but I am sure you could find others: for instance a name, a timestamp, etc.
It's a 1n relation and it's correctly represented.
Maybe you could add a foreign key to borrowers.borrowernumber to list the reserve group of a given patron.

> - I don't know how to indicates in the interface that two or more
>   reserves are only one request in reality.

What do you plan to implement next?
If the reserve groups were named, and linked to a patron, you could display these infos.

> - I'm bad at naming things, and "reserve_group" should probably be
>   something else.

Some for me... but reserve_group sounds not too bad.
Comment 3 Julian Maurice 2016-01-29 08:46:38 UTC
(In reply to Jonathan Druart from comment #2)
> Maybe you could add a foreign key to borrowers.borrowernumber to list the
> reserve group of a given patron.

The link to the borrowers table is already there in reserves table, and this makes me think the schema is inconsistent because it allows grouping reserves for different borrowers. Maybe reserves.borrowernumber should be moved to reserve_group (and all reserves should belong to a group, even if the group only contains one reserve). What do you think ?


> > - I don't know how to indicates in the interface that two or more
> >   reserves are only one request in reality.
> 
> What do you plan to implement next?
> If the reserve groups were named, and linked to a patron, you could display
> these infos.

Next steps would be to change the interface to reflect that change everywhere we display holds.
And implement that in OPAC too.

In the tables that show all holds of a patron I can group visually the holds of the same "reserve_group", but I can't do that on tables that show only a title's holds for instance.
Comment 4 Julian Maurice 2016-02-01 10:14:11 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2016-02-03 06:54:09 UTC Comment hidden (obsolete)
Comment 6 Alex Arnaud 2016-02-12 11:20:01 UTC Comment hidden (obsolete)
Comment 7 Alex Arnaud 2016-02-12 11:21:05 UTC Comment hidden (obsolete)
Comment 8 Alex Arnaud 2016-02-12 11:21:11 UTC Comment hidden (obsolete)
Comment 9 Michal Denar 2016-02-15 22:24:00 UTC
Hello,
if I followed test plan, i got Internal server error as result of step 4. 
Koha 3.22.3 with active Plack.

Mike
Comment 10 Josef Moravec 2016-02-16 06:05:21 UTC
(In reply to Mike from comment #9)
> Hello,
> if I followed test plan, i got Internal server error as result of step 4. 
> Koha 3.22.3 with active Plack.
> 
> Mike

Are you sure you tested with 3.22.3 version? It should be tested on master... maybe the problem is that you didn't run update_dbix_class_files.pl script? Just guess...
Comment 11 Josef Moravec 2016-02-16 06:52:23 UTC Comment hidden (obsolete)
Comment 12 Josef Moravec 2016-02-16 06:52:30 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2016-02-29 12:45:23 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2016-02-29 12:51:57 UTC Comment hidden (obsolete)
Comment 15 Christopher Brannon 2016-03-21 22:56:19 UTC
*** Bug 15914 has been marked as a duplicate of this bug. ***
Comment 16 Christopher Brannon 2016-03-21 22:58:22 UTC
*** Bug 16119 has been marked as a duplicate of this bug. ***
Comment 17 Benjamin Rokseth 2016-04-08 21:24:37 UTC Comment hidden (obsolete)
Comment 18 Benjamin Rokseth 2016-04-08 21:24:46 UTC Comment hidden (obsolete)
Comment 19 Benjamin Rokseth 2016-04-08 21:24:53 UTC Comment hidden (obsolete)
Comment 20 Benjamin Rokseth 2016-07-27 09:37:33 UTC Comment hidden (obsolete)
Comment 21 Benjamin Rokseth 2016-07-27 09:37:42 UTC Comment hidden (obsolete)
Comment 22 Benjamin Rokseth 2016-07-27 09:37:50 UTC Comment hidden (obsolete)
Comment 23 Benjamin Rokseth 2016-07-27 09:38:52 UTC
Rebased against master
Comment 24 Benjamin Rokseth 2016-07-28 11:00:23 UTC Comment hidden (obsolete)
Comment 25 Benjamin Rokseth 2016-07-28 11:00:32 UTC Comment hidden (obsolete)
Comment 26 Benjamin Rokseth 2016-07-28 11:00:39 UTC Comment hidden (obsolete)
Comment 27 Benjamin Rokseth 2016-07-28 11:01:26 UTC
Rebased to update foreign keys
Comment 28 Benjamin Rokseth 2016-07-28 12:20:51 UTC Comment hidden (obsolete)
Comment 29 Benjamin Rokseth 2016-07-28 12:21:00 UTC Comment hidden (obsolete)
Comment 30 Benjamin Rokseth 2016-07-28 12:21:08 UTC Comment hidden (obsolete)
Comment 31 Benjamin Rokseth 2016-07-28 12:25:31 UTC
Now rebased against pushed Bug 15533 - Allow patrons and librarians to select itemtype when placing hold.

Tested to be working as before this patch was pushed
Comment 32 Julian Maurice 2016-08-29 13:22:55 UTC Comment hidden (obsolete)
Comment 33 Owen Leonard 2016-08-29 15:14:51 UTC
The term "reserve group" is inconsistent with our terminology guidelines. I think it should be "hold group." The word "reserve" is associated only with course reserves.

https://wiki.koha-community.org/wiki/Terminology
Comment 34 Julian Maurice 2016-09-07 13:46:33 UTC Comment hidden (obsolete)
Comment 35 Julian Maurice 2016-09-07 15:13:46 UTC
Created attachment 55304 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 36 Julian Maurice 2016-09-07 15:13:50 UTC
Created attachment 55305 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 37 Julian Maurice 2016-09-07 15:13:54 UTC
Created attachment 55306 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 38 Julian Maurice 2016-09-07 15:13:58 UTC
Created attachment 55307 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 39 Julian Maurice 2016-09-07 15:14:03 UTC
Created attachment 55308 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 40 Julian Maurice 2016-09-07 15:14:38 UTC
All patches rebased on master
Comment 41 Julian Maurice 2016-09-23 13:14:15 UTC
Created attachment 55768 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 42 Julian Maurice 2016-09-23 13:14:21 UTC
Created attachment 55769 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 43 Julian Maurice 2016-09-23 13:14:25 UTC
Created attachment 55770 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 44 Julian Maurice 2016-09-23 13:14:30 UTC
Created attachment 55771 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 45 Julian Maurice 2016-09-23 13:14:34 UTC
Created attachment 55772 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 46 Julian Maurice 2017-01-11 09:31:21 UTC
Created attachment 58799 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 47 Julian Maurice 2017-01-11 09:31:25 UTC
Created attachment 58800 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 48 Julian Maurice 2017-01-11 09:31:29 UTC
Created attachment 58801 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 49 Julian Maurice 2017-01-11 09:31:34 UTC
Created attachment 58802 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 50 Julian Maurice 2017-01-11 09:31:38 UTC
Created attachment 58803 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 51 Julian Maurice 2017-01-20 09:47:24 UTC
Created attachment 59314 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 52 Julian Maurice 2017-01-20 09:47:29 UTC
Created attachment 59315 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 53 Julian Maurice 2017-01-20 09:47:33 UTC
Created attachment 59316 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 54 Julian Maurice 2017-01-20 09:47:38 UTC
Created attachment 59317 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 55 Julian Maurice 2017-01-20 09:47:44 UTC
Created attachment 59318 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 56 Josef Moravec 2017-01-24 20:29:00 UTC
Sorry, but I found some problems...

First, the ReserveCount test is failing:

t/db_dependent/Reserves/ReserveCount.t .. 1/3 Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.

#   Failed test 'Test borrower has 2 reserve.'
#   at t/db_dependent/Reserves/ReserveCount.t line 82.
#          got: '1'
#     expected: '2'
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.
Value not allowed for auto_incr hold_group_id in HoldGroup at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 267.

#   Failed test 'Test borrower has 2 reserve.'
#   at t/db_dependent/Reserves/ReserveCount.t line 114.
#          got: '1'
#     expected: '3'
# Looks like you failed 2 tests of 3.


Second, Koha qa tools has complains:
 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc
   FAIL	  forbidden patterns
		forbidden pattern: simple-quote string (line 24)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 1)

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/reserve/reserve-group.tt
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

 FAIL	reserve/reserve-group.pl
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

And you should also change the modal syntax...

For bootstrap related issues see page https://wiki.koha-community.org/wiki/Interface_patterns
especially sections "Buttons" and "Modals"
Comment 57 Michal Denar 2017-11-22 22:31:13 UTC
Can anobody move this bug ... its on very good way....


Thanks


Mike
Comment 58 Katrin Fischer 2017-12-05 06:48:52 UTC
It's waiting for the patch author to fix the problems pointed out in testing.
Comment 59 Josef Moravec 2017-12-19 11:04:27 UTC
Created attachment 69902 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 60 Josef Moravec 2017-12-19 11:04:33 UTC
Created attachment 69903 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 61 Josef Moravec 2017-12-19 11:04:37 UTC
Created attachment 69904 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 62 Josef Moravec 2017-12-19 11:04:42 UTC
Created attachment 69905 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 63 Josef Moravec 2017-12-19 11:04:47 UTC
Created attachment 69906 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 64 Josef Moravec 2017-12-19 11:09:41 UTC
I did a rebase, the feature is not working now, but applies cleanly and I plan to continue on it during Christmas I hope. Changes to C4/Reserves.pm should be moved to objects and many more. The codebase changed too much since the patches were uploaded, but they are still good start to work on it.
Comment 65 Benjamin Rokseth 2018-03-06 17:21:57 UTC
Created attachment 72485 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 66 Benjamin Rokseth 2018-03-06 17:22:05 UTC
Created attachment 72486 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 67 Benjamin Rokseth 2018-03-06 17:22:13 UTC
Created attachment 72487 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 68 Benjamin Rokseth 2018-03-06 17:22:20 UTC
Created attachment 72488 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 69 Benjamin Rokseth 2018-03-06 17:22:26 UTC
Created attachment 72489 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 70 Benjamin Rokseth 2018-03-06 17:23:21 UTC
rebased against master
Comment 71 Benjamin Rokseth 2018-03-06 17:51:14 UTC
Created attachment 72491 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 72 Benjamin Rokseth 2018-03-06 17:52:37 UTC
added missing file hold-group-modal.inc
Comment 73 paxed 2018-03-07 06:19:51 UTC
Comment on attachment 72485 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

Couple tiny nitpicks:

1) the JS translation function should have the string in double quotes, so instead of _('Loading'), use _("Loading")

2) Having separate translatable strings for PART_OF_A and HOLD_GROUP isn't optimal, because those two always go together. It should perhaps be written as PART_OF_HOLD_GROUP = _("(part of a %shold group%s)") and then replace the placeholders with the html code. Even better would be to avoid the html in the middle of the sentence altogether, by making the whole sentence a link.
Comment 74 Julian Maurice 2018-09-19 08:37:30 UTC
Created attachment 79077 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 75 Julian Maurice 2018-09-19 08:37:37 UTC
Created attachment 79078 [details] [review]
Bug 15516 - Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 76 Julian Maurice 2018-09-19 08:37:43 UTC
Created attachment 79080 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 77 Julian Maurice 2018-09-19 08:37:49 UTC
Created attachment 79081 [details] [review]
Bug 15516: Set reserve-group.pl to be executable
Comment 78 Julian Maurice 2018-09-19 08:37:55 UTC
Created attachment 79082 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'
Comment 79 Julian Maurice 2018-09-19 08:38:02 UTC
Created attachment 79083 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)
Comment 80 Julian Maurice 2018-09-19 08:39:42 UTC
Patches rebased on master + followup for comment 73
Comment 81 Josef Moravec 2018-09-21 11:28:39 UTC
Created attachment 79213 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 82 Josef Moravec 2018-09-21 11:28:45 UTC
Created attachment 79214 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 83 Josef Moravec 2018-09-21 11:28:50 UTC
Created attachment 79215 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 84 Josef Moravec 2018-09-21 11:28:56 UTC
Created attachment 79216 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 85 Josef Moravec 2018-09-21 11:29:01 UTC
Created attachment 79217 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 86 Josef Moravec 2018-09-21 11:29:07 UTC
Created attachment 79218 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 87 Josef Moravec 2018-09-21 11:29:12 UTC
Created attachment 79219 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 88 Josef Moravec 2018-09-21 11:29:18 UTC
Created attachment 79220 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 89 Josef Moravec 2018-09-21 11:29:23 UTC
Created attachment 79221 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 90 Josef Moravec 2018-09-21 11:29:29 UTC
Created attachment 79222 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 91 Katrin Fischer 2018-10-28 13:08:44 UTC
Sorry, patch doesn't apply, please rebase and rerun the QA script!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15516: Allow to reserve first available item from a group of titles
Applying: Bug 15516: Holding first available item from x biblios only count 1 reserve
Applying: Bug 15516: Update old_reserves (reserve_group_id)
Applying: Bug 15516: Set reserve-group.pl to be executable
Applying: Bug 15516: Reword 'reserve group' to 'hold group'
Applying: Bug 15516: Fix translation issues
Applying: Bug 15516: (follow-up) Fix kohastructure.sql
Applying: Bug 15516: (follow-up) Fix reserve -> hold change
Applying: Bug 15516: (follow-up) Fix modal for hold group
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 15516: (follow-up) Fix modal for hold group
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".
Patch left in /tmp/Bug-15516-follow-up-Fix-modal-for-hold-group-khk1ja.patch
Comment 92 Josef Moravec 2018-10-29 07:41:36 UTC
Created attachment 81453 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 93 Josef Moravec 2018-10-29 07:41:44 UTC
Created attachment 81454 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 94 Josef Moravec 2018-10-29 07:41:51 UTC
Created attachment 81455 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 95 Josef Moravec 2018-10-29 07:41:57 UTC
Created attachment 81456 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 96 Josef Moravec 2018-10-29 07:42:05 UTC
Created attachment 81457 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 97 Josef Moravec 2018-10-29 07:42:12 UTC
Created attachment 81458 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 98 Josef Moravec 2018-10-29 07:42:20 UTC
Created attachment 81459 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 99 Josef Moravec 2018-10-29 07:42:27 UTC
Created attachment 81460 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 100 Josef Moravec 2018-10-29 07:42:33 UTC
Created attachment 81461 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 101 Josef Moravec 2018-10-29 07:42:39 UTC
Created attachment 81462 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 102 Josef Moravec 2018-10-29 07:42:45 UTC
Created attachment 81463 [details] [review]
Bug 15516: (follow-up) Fix template filters
Comment 103 Josef Moravec 2018-10-29 07:44:07 UTC
(In reply to Katrin Fischer from comment #91)
> Sorry, patch doesn't apply, please rebase and rerun the QA script!
> 

I've just rebased and added new follow-up (tt filters)
Comment 104 Aleisha Amohia 2019-01-31 02:51:34 UTC
Created attachment 84554 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 105 Aleisha Amohia 2019-01-31 02:51:42 UTC
Created attachment 84555 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 106 Aleisha Amohia 2019-01-31 02:51:49 UTC
Created attachment 84556 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 107 Aleisha Amohia 2019-01-31 02:51:57 UTC
Created attachment 84557 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 108 Aleisha Amohia 2019-01-31 02:52:04 UTC
Created attachment 84558 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 109 Aleisha Amohia 2019-01-31 02:52:13 UTC
Created attachment 84559 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 110 Aleisha Amohia 2019-01-31 02:52:22 UTC
Created attachment 84560 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 111 Aleisha Amohia 2019-01-31 02:52:32 UTC
Created attachment 84561 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 112 Aleisha Amohia 2019-01-31 02:52:43 UTC
Created attachment 84562 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 113 Aleisha Amohia 2019-01-31 02:53:09 UTC
Created attachment 84563 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 114 Aleisha Amohia 2019-01-31 02:53:20 UTC
Created attachment 84564 [details] [review]
Bug 15516: (follow-up) Fix template filters
Comment 115 Katrin Fischer 2019-02-04 07:12:44 UTC
Aleisha, did you mean to sign-off?
Comment 116 Aleisha Amohia 2019-02-04 20:08:05 UTC
(In reply to Katrin Fischer from comment #115)
> Aleisha, did you mean to sign-off?

Oops no was just rebasing. Still working on this
Comment 117 Josef Moravec 2019-03-27 10:58:17 UTC
(In reply to Aleisha Amohia from comment #116)
> (In reply to Katrin Fischer from comment #115)
> > Aleisha, did you mean to sign-off?
> 
> Oops no was just rebasing. Still working on this

Aleisha, what did you want to do with this? Can we make some progress on these long standing patches?
Comment 118 Aleisha Amohia 2019-04-05 01:14:04 UTC
(In reply to Josef Moravec from comment #117)
> (In reply to Aleisha Amohia from comment #116)
> > (In reply to Katrin Fischer from comment #115)
> > > Aleisha, did you mean to sign-off?
> > 
> > Oops no was just rebasing. Still working on this
> 
> Aleisha, what did you want to do with this? Can we make some progress on
> these long standing patches?

Looking at these again now!
Comment 119 Aleisha Amohia 2019-04-12 04:04:13 UTC
Created attachment 87828 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 120 Aleisha Amohia 2019-04-12 04:04:20 UTC
Created attachment 87829 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 121 Aleisha Amohia 2019-04-12 04:04:27 UTC
Created attachment 87830 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 122 Aleisha Amohia 2019-04-12 04:04:35 UTC
Created attachment 87831 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 123 Aleisha Amohia 2019-04-12 04:04:42 UTC
Created attachment 87832 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 124 Aleisha Amohia 2019-04-12 04:04:49 UTC
Created attachment 87833 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 125 Aleisha Amohia 2019-04-12 04:04:55 UTC
Created attachment 87834 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 126 Aleisha Amohia 2019-04-12 04:05:02 UTC
Created attachment 87835 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 127 Aleisha Amohia 2019-04-12 04:05:09 UTC
Created attachment 87836 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 128 Aleisha Amohia 2019-04-12 04:05:17 UTC
Created attachment 87837 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 129 Aleisha Amohia 2019-04-12 04:05:24 UTC
Created attachment 87838 [details] [review]
Bug 15516: (follow-up) Fix template filters
Comment 130 Michal Denar 2019-04-12 06:40:13 UTC
Hi Aleisha,
ready for test? This can be nice new feature for 19.05 release.
Comment 131 Aleisha Amohia 2019-04-14 21:25:31 UTC
(In reply to Michal Denar from comment #130)
> Hi Aleisha,
> ready for test? This can be nice new feature for 19.05 release.

Not yet, almost there!
Comment 132 Aleisha Amohia 2019-04-16 02:27:48 UTC
Created attachment 88038 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl
Comment 133 Aleisha Amohia 2019-04-16 02:27:59 UTC
Created attachment 88039 [details] [review]
Bug 15516: (follow-up) Schema updates
Comment 134 Aleisha Amohia 2019-04-16 02:28:37 UTC
Ready for testing!
Comment 135 Josef Moravec 2019-04-16 05:46:36 UTC
Comment on attachment 88038 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Review of attachment 88038 [details] [review]:
-----------------------------------------------------------------

Just quick look into the follow-ups, found this:

::: installer/data/mysql/atomicupdate/bug_15516_-_hold_group_table.perl
@@ +3,5 @@
> +    $dbh->do(q{DROP TABLE IF EXISTS hold_group});
> +    $dbh->do(q{CREATE TABLE hold_group (
> +        hold_group_id int unsigned NOT NULL AUTO_INCREMENT,
> +        PRIMARY KEY (hold_group_id)
> +        ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci});

should be utf8mb4
Comment 136 Aleisha Amohia 2019-04-16 21:44:42 UTC
(In reply to Josef Moravec from comment #135)
> Comment on attachment 88038 [details] [review] [review]
> Bug 15516: (follow-up) Fixing modal view and more
> 
> Review of attachment 88038 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Just quick look into the follow-ups, found this:
> 
> ::: installer/data/mysql/atomicupdate/bug_15516_-_hold_group_table.perl
> @@ +3,5 @@
> > +    $dbh->do(q{DROP TABLE IF EXISTS hold_group});
> > +    $dbh->do(q{CREATE TABLE hold_group (
> > +        hold_group_id int unsigned NOT NULL AUTO_INCREMENT,
> > +        PRIMARY KEY (hold_group_id)
> > +        ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci});
> 
> should be utf8mb4

I'll fix this up
Comment 137 Michal Denar 2019-04-16 22:19:08 UTC
Patch look working and useful. I'll wait till follow-up for final test.

Thank you.
Comment 138 Aleisha Amohia 2019-04-17 01:26:33 UTC
Created attachment 88125 [details] [review]
Bug 15516: (follow-up) Changing utf8 to utf8mb4
Comment 139 Aleisha Amohia 2019-04-17 01:26:59 UTC
(In reply to Michal Denar from comment #137)
> Patch look working and useful. I'll wait till follow-up for final test.
> 
> Thank you.

Ready for testing!
Comment 140 Michal Denar 2019-04-17 11:21:32 UTC
Created attachment 88173 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 141 Michal Denar 2019-04-17 11:21:40 UTC
Created attachment 88174 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 142 Michal Denar 2019-04-17 11:21:47 UTC
Created attachment 88175 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 143 Michal Denar 2019-04-17 11:21:54 UTC
Created attachment 88176 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 144 Michal Denar 2019-04-17 11:22:01 UTC
Created attachment 88177 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 145 Michal Denar 2019-04-17 11:22:08 UTC
Created attachment 88178 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 146 Michal Denar 2019-04-17 11:22:15 UTC
Created attachment 88179 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 147 Michal Denar 2019-04-17 11:22:22 UTC
Created attachment 88180 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 148 Michal Denar 2019-04-17 11:22:31 UTC
Created attachment 88181 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 149 Michal Denar 2019-04-17 11:22:39 UTC
Created attachment 88182 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 150 Michal Denar 2019-04-17 11:22:46 UTC
Created attachment 88183 [details] [review]
Bug 15516: (follow-up) Fix template filters

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 151 Michal Denar 2019-04-17 11:22:53 UTC
Created attachment 88184 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 152 Michal Denar 2019-04-17 11:23:01 UTC
Created attachment 88185 [details] [review]
Bug 15516: (follow-up) Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 153 Michal Denar 2019-04-17 11:23:09 UTC
Created attachment 88186 [details] [review]
Bug 15516: (follow-up) Changing utf8 to utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 154 Michal Denar 2019-04-17 11:47:21 UTC
Hi there,
patch works. Great work.

I've just one notice. If somebody use this feature to hold to pull (reserve on title with available item). It's wrong process, I know it. But some libraian can make this fault. In this case tool don't delete reserves on next titles. I'm not sure that is an error. It's same behaviour from earlier version of Place hold on more than one title.

Ideas:
- if title with available item is add into group, Koha show kind of info text (easy solution)
- if somedoby try to add title with available item, Koha don't allow it with message

What do you think about it?

Thank you.
Comment 155 Katrin Fischer 2019-05-11 19:55:31 UTC
Currently this patch set doesn't apply. Can you please rebase? It might make sense to wait a bit until after the 19.05 release as this will be for 19.11.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15516: Allow to reserve first available item from a group of titles
Using index info to reconstruct a base tree...
M	C4/Reserves.pm
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
M	reserve/placerequest.pl
M	reserve/request.pl
Falling back to patching base and 3-way merge...
Auto-merging reserve/request.pl
Auto-merging reserve/placerequest.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Reserves.pm
Applying: Bug 15516: Holding first available item from x biblios only count 1 reserve
Applying: Bug 15516: Update old_reserves (reserve_group_id)
Applying: Bug 15516: Set reserve-group.pl to be executable
Applying: Bug 15516: Reword 'reserve group' to 'hold group'
Applying: Bug 15516: Fix translation issues
Applying: Bug 15516: (follow-up) Fix kohastructure.sql
Applying: Bug 15516: (follow-up) Fix reserve -> hold change
Applying: Bug 15516: (follow-up) Fix modal for hold group
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt).
error: could not build fake ancestor
Patch failed at 0001 Bug 15516: (follow-up) Fix modal for hold group
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".
Patch left in /tmp/Bug-15516-follow-up-Fix-modal-for-hold-group-vopUHO.patch
Comment 156 Michal Denar 2019-05-11 20:03:57 UTC
I really love this at 19.05. Aleisa, can you rebase it on master. I'm ready for test.
Comment 157 Katrin Fischer 2019-05-11 20:07:57 UTC
(In reply to Michal Denar from comment #156)
> I really love this at 19.05. Aleisa, can you rebase it on master. I'm ready
> for test.

Hi Michal, we have reached 'feature freeze' - this means no new features will be pushed for 19.05 apart from a select few that the RM has marked with rel_19_05_candidate. See here:
http://lists.koha-community.org/pipermail/koha-devel/2019-April/045179.html
Comment 158 Michal Denar 2019-05-11 20:20:33 UTC
Hi Katrin,
OK, one day after. It's sad for me. But I hope that we finish this bug soon. 19.11. will be fine.
Comment 159 Aleisha Amohia 2019-05-12 21:29:31 UTC
Thanks all, I'll rebase this in June after the 19.05 release.
Comment 160 Michal Denar 2019-05-13 07:40:35 UTC
I hope that we together finalize this feature for 19.11 release.
Comment 161 Aleisha Amohia 2019-06-04 02:59:02 UTC
Created attachment 90289 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 162 Aleisha Amohia 2019-06-04 02:59:11 UTC
Created attachment 90290 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 163 Aleisha Amohia 2019-06-04 02:59:18 UTC
Created attachment 90291 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 164 Aleisha Amohia 2019-06-04 02:59:25 UTC
Created attachment 90292 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 165 Aleisha Amohia 2019-06-04 02:59:31 UTC
Created attachment 90293 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 166 Aleisha Amohia 2019-06-04 02:59:38 UTC
Created attachment 90294 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 167 Aleisha Amohia 2019-06-04 02:59:44 UTC
Created attachment 90295 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 168 Aleisha Amohia 2019-06-04 02:59:52 UTC
Created attachment 90296 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 169 Aleisha Amohia 2019-06-04 02:59:58 UTC
Created attachment 90297 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 170 Aleisha Amohia 2019-06-04 03:00:05 UTC
Created attachment 90298 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 171 Aleisha Amohia 2019-06-04 03:00:13 UTC
Created attachment 90299 [details] [review]
Bug 15516: (follow-up) Fix template filters

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 172 Aleisha Amohia 2019-06-04 03:00:19 UTC
Created attachment 90300 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 173 Aleisha Amohia 2019-06-04 03:00:26 UTC
Created attachment 90301 [details] [review]
Bug 15516: (follow-up) Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 174 Aleisha Amohia 2019-06-04 03:00:32 UTC
Created attachment 90302 [details] [review]
Bug 15516: (follow-up) Changing utf8 to utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 175 Aleisha Amohia 2019-06-04 03:00:39 UTC
Created attachment 90303 [details] [review]
Bug 15516: (follow-up) Fixing atomic update to prevent errors

This patch prevents errors when applying atomic update, particularly for
those that have already tested this feature before.
Comment 176 Aleisha Amohia 2019-06-04 03:02:22 UTC
Rebased and ready to test
Comment 177 Aleisha Amohia 2019-06-20 02:55:48 UTC
Created attachment 90829 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 178 Aleisha Amohia 2019-06-20 02:55:56 UTC
Created attachment 90830 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 179 Aleisha Amohia 2019-06-20 02:56:04 UTC
Created attachment 90831 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 180 Aleisha Amohia 2019-06-20 02:56:12 UTC
Created attachment 90832 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 181 Aleisha Amohia 2019-06-20 02:56:22 UTC
Created attachment 90833 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 182 Aleisha Amohia 2019-06-20 02:56:29 UTC
Created attachment 90834 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 183 Aleisha Amohia 2019-06-20 02:56:37 UTC
Created attachment 90835 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 184 Aleisha Amohia 2019-06-20 02:56:45 UTC
Created attachment 90836 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 185 Aleisha Amohia 2019-06-20 02:56:52 UTC
Created attachment 90837 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 186 Aleisha Amohia 2019-06-20 02:57:00 UTC
Created attachment 90838 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 187 Aleisha Amohia 2019-06-20 02:57:07 UTC
Created attachment 90839 [details] [review]
Bug 15516: (follow-up) Fix template filters

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 188 Aleisha Amohia 2019-06-20 02:57:14 UTC
Created attachment 90840 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 189 Aleisha Amohia 2019-06-20 02:57:21 UTC
Created attachment 90841 [details] [review]
Bug 15516: (follow-up) Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 190 Aleisha Amohia 2019-06-20 02:57:28 UTC
Created attachment 90842 [details] [review]
Bug 15516: (follow-up) Changing utf8 to utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 191 Aleisha Amohia 2019-06-20 02:57:35 UTC
Created attachment 90843 [details] [review]
Bug 15516: (follow-up) Fixing atomic update to prevent errors

This patch prevents errors when applying atomic update, particularly for
those that have already tested this feature before.
Comment 192 Aleisha Amohia 2019-06-20 03:22:08 UTC
Created attachment 90844 [details] [review]
Bug 15516: Fixing missing curly bracket
Comment 193 Julian Maurice 2019-09-13 13:11:43 UTC
Created attachment 92783 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 194 Julian Maurice 2019-09-13 13:11:51 UTC
Created attachment 92784 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 195 Julian Maurice 2019-09-13 13:11:58 UTC
Created attachment 92785 [details] [review]
Bug 15516: Update old_reserves (reserve_group_id)

Without it, deletion of reserves will fail

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 196 Julian Maurice 2019-09-13 13:12:06 UTC
Created attachment 92786 [details] [review]
Bug 15516: Set reserve-group.pl to be executable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 197 Julian Maurice 2019-09-13 13:12:12 UTC
Created attachment 92787 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 198 Julian Maurice 2019-09-13 13:12:24 UTC
Created attachment 92788 [details] [review]
Bug 15516: Fix translation issues

- Use " inside _()
- Merge two strings (PART_OF_A, HOLD_GROUP)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 199 Julian Maurice 2019-09-13 13:12:32 UTC
Created attachment 92789 [details] [review]
Bug 15516: (follow-up) Fix kohastructure.sql

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 200 Julian Maurice 2019-09-13 13:12:42 UTC
Created attachment 92790 [details] [review]
Bug 15516: (follow-up) Fix reserve -> hold change

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 201 Julian Maurice 2019-09-13 13:12:50 UTC
Created attachment 92791 [details] [review]
Bug 15516: (follow-up) Fix modal for hold group

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 202 Julian Maurice 2019-09-13 13:12:58 UTC
Created attachment 92792 [details] [review]
Bug 15516: (follow-up) Fix pod in C4::Reserves

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 203 Julian Maurice 2019-09-13 13:13:05 UTC
Created attachment 92793 [details] [review]
Bug 15516: (follow-up) Fix template filters

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 204 Julian Maurice 2019-09-13 13:13:11 UTC
Created attachment 92794 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 205 Julian Maurice 2019-09-13 13:13:19 UTC
Created attachment 92795 [details] [review]
Bug 15516: (follow-up) Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 206 Julian Maurice 2019-09-13 13:13:29 UTC
Created attachment 92796 [details] [review]
Bug 15516: (follow-up) Changing utf8 to utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 207 Julian Maurice 2019-09-13 13:13:37 UTC
Created attachment 92797 [details] [review]
Bug 15516: (follow-up) Fixing atomic update to prevent errors

This patch prevents errors when applying atomic update, particularly for
those that have already tested this feature before.
Comment 208 Julian Maurice 2019-09-13 13:13:44 UTC
Created attachment 92798 [details] [review]
Bug 15516: Fixing missing curly bracket
Comment 209 Julian Maurice 2019-09-13 13:14:58 UTC
Patches rebased on master.

Status changed to Signed Off, as most patches are already signed off.
Comment 210 Katrin Fischer 2019-10-12 12:07:49 UTC
I am having issues with the Schema updates. 

I tried to skip 
Bug 15516: (follow-up) Schema updates
in favor of runing the dbic command myself later, but 
Bug 15516: Fixing missing curly bracket
looks like there are also manual changes?

Can you please keep the automated updates in a separate patch and rebase?
Comment 211 Katrin Fischer 2019-10-12 12:33:42 UTC
Some doubts here:

1) Koha vs. C4

This adds quite some new code to the C4 namespace:
AddHoldGroup
DeleteHoldGroup
GetHoldGroupHolds
GetReserveCount

We already have started to move to Hold.pm and Holds.pm in the Koha namespace.

I feel like the new things (*HoldGroup*) at least should be started in the new namespace... someone else might weigh in here.

Also uses lots of SQL.

2) Database changes

The database update looks quite complicated, because you try to catch that the holds_groups could already exist. I feel it could be simplified.
 
The holds groups table only consists of a PK column?

I think the charset is not correct, should be:

3) Had the same question looking at the new method added:
+        # TODO: Should be: $reserve_cnt = $patron->holds->count;
+        $reserve_cnt = GetReserveCount( $borrowernumber );

4) Rename test file (must have missed the global renaming :) )

5) How does this feature affect limits on holds? (total and daily limits). Will every hold be counted as before?
Comment 212 Martin Renvoize 2020-01-29 07:25:44 UTC
Are you hoping to pick this back up at some point Julian or are you hoping someone will adopt it?
Comment 213 Owen Leonard 2020-01-29 16:39:03 UTC
(In reply to Martin Renvoize from comment #212)
> Are you hoping to pick this back up at some point Julian or are you hoping
> someone will adopt it?

I second this question! My library is looking for something like this.
Comment 214 Julian Maurice 2020-01-29 17:37:12 UTC
(In reply to Martin Renvoize from comment #212)
> Are you hoping to pick this back up at some point Julian or are you hoping
> someone will adopt it?

I'm always hoping to have the time to work on patches I submitted, but that shouldn't stop people to rebase them, or submit follow-up patches.
I certainly won't be mad if someone other than me make it pass QA :D
Comment 215 Michal Denar 2020-11-27 23:00:13 UTC
Hi Julian,
is possible to make rebase and follow-up? It's nice feature for 21.05. Isn't it?

I'm ready for testing.

Thank you.
Comment 216 Michal Denar 2020-11-28 08:35:23 UTC
Maybe Aleisha can finish it?

Thank you.
Comment 217 Aleisha Amohia 2020-11-29 20:06:01 UTC
I would love to pick this back up but unfortunately won't have time, at least not until the New Year.
Comment 218 David Cook 2020-12-18 06:06:55 UTC
Is it possible to squash these patches down a bit? With 16 patches, I think it's difficult to rebase and difficult to have a cohesive idea of how this change is supposed to work.
Comment 219 David Cook 2020-12-18 06:35:59 UTC
Julian, what's the commit that these are based off? I'd love to apply these to a stable base so that I could get a sense of the changes...
Comment 220 Jonathan Druart 2020-12-18 10:06:53 UTC
(In reply to David Cook from comment #219)
> Julian, what's the commit that these are based off? I'd love to apply these
> to a stable base so that I could get a sense of the changes...

See https://gist.github.com/jajm/f0b77da1a41b6b1c5ab1
Comment 221 Josef Moravec 2020-12-18 10:59:58 UTC
Created attachment 114513 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 222 Josef Moravec 2020-12-18 11:00:07 UTC
Created attachment 114514 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 223 Josef Moravec 2020-12-18 11:00:16 UTC
Created attachment 114515 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 224 Josef Moravec 2020-12-18 11:00:24 UTC
Created attachment 114516 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl
- utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 225 Josef Moravec 2020-12-18 11:00:33 UTC
Created attachment 114517 [details] [review]
Bug 15516: (follow-up) Fix QA and rebase issues
Comment 226 Josef Moravec 2020-12-18 11:00:43 UTC
Created attachment 114518 [details] [review]
Bug 15516: (follow-up) Fix db structure and atomic update
Comment 227 Josef Moravec 2020-12-18 11:02:57 UTC
I rebased and squashed a bit, but this still must be considered as WIP
Comment 228 David Cook 2020-12-22 06:06:51 UTC
(In reply to Josef Moravec from comment #227)
> I rebased and squashed a bit, but this still must be considered as WIP

I'll hopefully take a look at this tomorrow. 

Why is it considered a WIP at this point?
Comment 229 Josef Moravec 2020-12-22 06:40:33 UTC
(In reply to David Cook from comment #228)
> (In reply to Josef Moravec from comment #227)
> > I rebased and squashed a bit, but this still must be considered as WIP
> 
> I'll hopefully take a look at this tomorrow. 
> 
> Why is it considered a WIP at this point?

I do consider i as WIP mainly because, thinks needs to be moved from C4 to Koha namespace (using Koha::Object[s] of course).

Second step would to modify hold to by always a hold group (even on only one title). But this could be eventually done on another report.
Comment 230 Josef Moravec 2020-12-22 07:27:50 UTC
Created attachment 114585 [details] [review]
Bug 15516: (follow-up) Fix db structure and atomic update
Comment 231 Josef Moravec 2020-12-22 07:27:59 UTC
Created attachment 114586 [details] [review]
Bug 15516: DO NOT PUSH - Schema changes
Comment 232 Josef Moravec 2020-12-22 13:41:30 UTC
Created attachment 114603 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]
Comment 233 Josef Moravec 2020-12-22 22:01:05 UTC
Created attachment 114624 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 234 Josef Moravec 2020-12-22 22:01:14 UTC
Created attachment 114625 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 235 Josef Moravec 2020-12-22 22:01:22 UTC
Created attachment 114626 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 236 Josef Moravec 2020-12-22 22:01:31 UTC
Created attachment 114627 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl
- utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 237 Josef Moravec 2020-12-22 22:01:40 UTC
Created attachment 114628 [details] [review]
Bug 15516: (follow-up) Fix QA and rebase issues
Comment 238 Josef Moravec 2020-12-22 22:01:49 UTC
Created attachment 114629 [details] [review]
Bug 15516: (follow-up) Fix db structure and atomic update
Comment 239 Josef Moravec 2020-12-22 22:01:56 UTC
Created attachment 114630 [details] [review]
Bug 15516: DO NOT PUSH - Schema changes
Comment 240 Josef Moravec 2020-12-22 22:02:05 UTC
Created attachment 114631 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]
Comment 241 Josef Moravec 2020-12-22 22:02:48 UTC
Just rebased on current master
Comment 242 Josef Moravec 2020-12-22 22:05:45 UTC
Created attachment 114632 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]

Test plan:
prove t/db_dependent/Koha/HoldGroup.t
prove t/db_dependent/Koha/Holds.t
Comment 243 Josef Moravec 2020-12-22 22:19:03 UTC
Created attachment 114633 [details] [review]
Bug 15516: (follow-up) Remove C4::Reserves::GetHoldGroupHolds
Comment 244 Josef Moravec 2020-12-22 22:22:57 UTC
Created attachment 114634 [details] [review]
Bug 15516: (follow-up) Fix creating hold group in placerequest
Comment 245 Michal Denar 2020-12-22 22:39:33 UTC
Created attachment 114635 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 246 Michal Denar 2020-12-22 22:39:41 UTC
Created attachment 114636 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 247 Michal Denar 2020-12-22 22:39:48 UTC
Created attachment 114637 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 248 Michal Denar 2020-12-22 22:39:55 UTC
Created attachment 114638 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl
- utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 249 Michal Denar 2020-12-22 22:40:02 UTC
Created attachment 114639 [details] [review]
Bug 15516: (follow-up) Fix QA and rebase issues

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 250 Michal Denar 2020-12-22 22:40:08 UTC
Created attachment 114640 [details] [review]
Bug 15516: (follow-up) Fix db structure and atomic update

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 251 Michal Denar 2020-12-22 22:40:14 UTC
Created attachment 114641 [details] [review]
Bug 15516: DO NOT PUSH - Schema changes

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 252 Michal Denar 2020-12-22 22:40:20 UTC
Created attachment 114642 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]

Test plan:
prove t/db_dependent/Koha/HoldGroup.t
prove t/db_dependent/Koha/Holds.t

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 253 Michal Denar 2020-12-22 22:40:27 UTC
Created attachment 114643 [details] [review]
Bug 15516: (follow-up) Remove C4::Reserves::GetHoldGroupHolds

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 254 Michal Denar 2020-12-22 22:40:34 UTC
Created attachment 114644 [details] [review]
Bug 15516: (follow-up) Fix creating hold group in placerequest

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 255 Josef Moravec 2020-12-22 22:41:50 UTC
I have still some concerns:
- ModReserveAffect, when hold from group is set to waiting (or transfer), this  just delete other reserves from group and even group itself. I don't think it is what we want... We probably want to keep history
- The changes in ModReserveAffect are not covered by tests
- Is there other way of set hold waiting? If so, we need to make changes an all places where the hold status is affected
- hold-group.js contains TT tags ;) (I will fix it)
Comment 256 Josef Moravec 2020-12-22 22:44:44 UTC
Created attachment 114645 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]

Test plan:
prove t/db_dependent/Koha/HoldGroup.t
prove t/db_dependent/Koha/Holds.t

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 257 Josef Moravec 2020-12-22 22:46:07 UTC
Just squashed last three patches into one for better readability
Comment 258 Josef Moravec 2020-12-22 23:00:46 UTC
Created attachment 114647 [details] [review]
Bug 15516: (follow-up) Fix showing list of titles in hold group
Comment 259 Josef Moravec 2020-12-22 23:08:21 UTC
Created attachment 114648 [details] [review]
Bug 15516: (follow-up) Fix showing list of titles in hold group
Comment 260 Josef Moravec 2020-12-22 23:14:07 UTC
Created attachment 114649 [details] [review]
Bug 15516: (follow-up) Remove loading-small.gif reference

Test plan:
Ensure the hold group modal is still working
Comment 261 Josef Moravec 2020-12-22 23:17:16 UTC
(In reply to Josef Moravec from comment #255)

> - hold-group.js contains TT tags ;) (I will fix it)

fixed
Comment 262 Michal Denar 2020-12-22 23:20:16 UTC
Created attachment 114650 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library have the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This patch is only for staff interface.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
   and click on 'Place hold' button at the top of the results.
2. Enter a patron cardnumber and click Search
3. Check the box for 'Place a hold on the next available item' (it's
   usually hidden when placing a hold on multiple biblios)
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
   additional text '(part of a reserve group)' in Details column.
6. Click on the "reserve group" link. A modal window should appear with
   the content of the reserve group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are
   deleted.

Note: the "reserve group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 263 Michal Denar 2020-12-22 23:20:25 UTC
Created attachment 114651 [details] [review]
Bug 15516: Holding first available item from x biblios only count 1 reserve

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 264 Michal Denar 2020-12-22 23:20:31 UTC
Created attachment 114652 [details] [review]
Bug 15516: Reword 'reserve group' to 'hold group'

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 265 Michal Denar 2020-12-22 23:20:38 UTC
Created attachment 114653 [details] [review]
Bug 15516: (follow-up) Fixing modal view and more

Fixing
- modal view
- missed changes from 'reserve group' to 'hold group'
- updating atomic update to perl
- utf8mb4

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 266 Michal Denar 2020-12-22 23:20:44 UTC
Created attachment 114654 [details] [review]
Bug 15516: (follow-up) Fix QA and rebase issues

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 267 Michal Denar 2020-12-22 23:20:50 UTC
Created attachment 114655 [details] [review]
Bug 15516: (follow-up) Fix db structure and atomic update

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 268 Michal Denar 2020-12-22 23:20:57 UTC
Created attachment 114656 [details] [review]
Bug 15516: DO NOT PUSH - Schema changes

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 269 Michal Denar 2020-12-22 23:21:04 UTC
Created attachment 114657 [details] [review]
Bug 15516: (follow-up) Move to Koha::Object[s]

Test plan:
prove t/db_dependent/Koha/HoldGroup.t
prove t/db_dependent/Koha/Holds.t

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 270 Michal Denar 2020-12-22 23:21:11 UTC
Created attachment 114658 [details] [review]
Bug 15516: (follow-up) Fix showing list of titles in hold group

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 271 Michal Denar 2020-12-22 23:21:18 UTC
Created attachment 114659 [details] [review]
Bug 15516: (follow-up) Remove loading-small.gif reference

Test plan:
Ensure the hold group modal is still working

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 272 David Cook 2020-12-23 00:58:17 UTC
My test plan:

Test plan:
0a. koha-shell kohadev -c /kohadevbox/koha/installer/data/mysql/updatedatabase.pl
#0b. koha-shell kohadev -c "/kohadevbox/koha/misc/devel/update_dbix_class_files.pl --koha-conf /etc/koha/sites/kohadev/koha-conf.xml --db_name koha_kohadev"
0c. koha-plack --restart kohadev

1. Go to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=e
2. Click checkboxes next to # 1 and # 2
3. Click "Place hold" button at the top of search results
4. Enter "koha" into patron search and press "Search", then choose "koha,"
5. Check the box for "Hold next available item" 
6. Click "Place hold"

7. Note the text "(part of a hold group)" in Details column.
8. Click on the "hold group" link text
9. Note a modal with a title of "Hold group" and containing the two titles that you've previously selected

10. Go to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51
11. Click on the "Holds" tab and note that there are 2 holds. One for each of the titles that you've selected. (There is no evidence here that they're part of a hold group.)
12. Checkin "39999000002195"
13. Note a modal saying "Hold found" and click "Confirm hold"
14. Go to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51
15. Note that the 2nd hold for "The Arden edition of the works of William Shakespeare." has been removed. 

--

I'm failing QA as I can only see the "hold group" link on the following page:
- reserve/request.pl

It does *not* appear on the following pages (although it is claimed it is in the commit messages):
- circ/circulation.pl
- members/moremember.pl

It is unknown whether or not it appears on the following pages because there were no tests for these pages:
- circ/pendingreserves.pl
Comment 273 David Cook 2020-12-23 01:03:35 UTC
(In reply to Josef Moravec from comment #255)
> I have still some concerns:
> - ModReserveAffect, when hold from group is set to waiting (or transfer),
> this  just delete other reserves from group and even group itself. I don't
> think it is what we want... We probably want to keep history

I am curious about that as well. 

> - The changes in ModReserveAffect are not covered by tests

That's not good. I think that might be an automatic QA fail?

> - Is there other way of set hold waiting? If so, we need to make changes an
> all places where the hold status is affected

I don't know.
Comment 274 David Cook 2020-12-23 01:04:34 UTC
Output from qa tool:

testing 10 commit(s) (applied to 6982680 'e9 Bug 14564: Set permissions on conf')

Processing files before patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 312.
|========================>| 31 / 31 (100.00%)
Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 312.
|========================>| 31 / 31 (100.00%)

 OK     C4/Reserves.pm
 OK     Koha/HoldGroup.pm
 OK     Koha/HoldGroups.pm
 OK     Koha/Holds.pm
 FAIL   Koha/Schema/Result/HoldGroup.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/Schema/Result/OldReserve.pm
 OK     Koha/Schema/Result/Reserve.pm
 OK     Koha/Template/Plugin/Biblio.pm
 OK     circ/pendingreserves.pl
 FAIL   installer/data/mysql/atomicupdate/bug_15516.sql
   FAIL   git manipulation
                The file has been added and deleted in the same patchset

 OK     installer/data/mysql/kohastructure.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/reserve-group-modal.inc
   FAIL   git manipulation
                The file has been added and deleted in the same patchset

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/reserve-group.tt
 OK     koha-tmpl/intranet-tmpl/prog/js/hold-group.js
 OK     koha-tmpl/intranet-tmpl/prog/js/holds.js
 OK     opac/opac-reserve.pl
 OK     reserve/hold-group.pl
 OK     reserve/placerequest.pl
 OK     reserve/request.pl
 FAIL   reserve/reserve-group.pl
   FAIL   file permissions
                File must have the exec flag

 OK     svc/holds
 OK     t/db_dependent/Koha/HoldGroup.t
 OK     t/db_dependent/Koha/Holds.t
 FAIL   t/db_dependent/Reserves/HoldCount.t
   FAIL   file permissions
                File must have the exec flag
   FAIL   git manipulation
                The file has been added and deleted in the same patchset


Processing additional checks OK!
Comment 275 David Cook 2020-12-23 23:23:41 UTC
Ugh, "holds group" is already being used by Bug 22284. 

That's already confusing me when I read Bugzilla reports...
Comment 276 David Cook 2021-02-01 03:29:35 UTC
Who is working on this one now?
Comment 277 David Cook 2021-02-10 22:50:52 UTC
I could potentially take over some work on this in the near future, although I might be more valuable as a tester/QAer.
Comment 278 Aleisha Amohia 2021-02-25 02:04:19 UTC
Working on this now
Comment 279 David Cook 2021-02-25 04:40:59 UTC
(In reply to Aleisha Amohia from comment #278)
> Working on this now

\0/
Comment 280 Aleisha Amohia 2021-03-01 02:46:10 UTC
Created attachment 117423 [details] [review]
Bug 15516: Database and installer updates
Comment 281 Aleisha Amohia 2021-03-01 02:46:17 UTC
Created attachment 117424 [details] [review]
Bug 15516: Relevant controller changes and tests
Comment 282 Aleisha Amohia 2021-03-01 02:46:24 UTC
Created attachment 117425 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT
Comment 283 Aleisha Amohia 2021-03-01 02:46:31 UTC
Created attachment 117426 [details] [review]
Bug 15516: Schema updates
Comment 284 Kyle M Hall 2021-03-01 17:13:26 UTC
What impact will this feature have on the holds queue. As it stands, there will be a request in the queue for each available item in the hold group. I see the holds to pull report gives a link to the holds group. I think the same functionality would be needed for the holds queue report as well.

In addition, we'll need to make sure this bug and bug 24359 work together.
Comment 285 David Cook 2021-03-03 05:03:23 UTC
(In reply to Kyle M Hall from comment #284)
> What impact will this feature have on the holds queue. As it stands, there
> will be a request in the queue for each available item in the hold group. I
> see the holds to pull report gives a link to the holds group. I think the
> same functionality would be needed for the holds queue report as well.
> 
> In addition, we'll need to make sure this bug and bug 24359 work together.

I'm willing to test this, but holding off while I wait for an answer to Kyle's comment.

Personally, I don't understand the holds queue feature well enough to really even understand the question.
Comment 286 David Cook 2021-03-03 05:03:23 UTC
(In reply to Kyle M Hall from comment #284)
> What impact will this feature have on the holds queue. As it stands, there
> will be a request in the queue for each available item in the hold group. I
> see the holds to pull report gives a link to the holds group. I think the
> same functionality would be needed for the holds queue report as well.
> 
> In addition, we'll need to make sure this bug and bug 24359 work together.

I'm willing to test this, but holding off while I wait for an answer to Kyle's comment.

Personally, I don't understand the holds queue feature well enough to really even understand the question.
Comment 287 Aleisha Amohia 2021-03-16 03:47:36 UTC
Created attachment 118281 [details] [review]
Bug 15516: Database and installer updates
Comment 288 Aleisha Amohia 2021-03-16 03:47:43 UTC
Created attachment 118282 [details] [review]
Bug 15516: Relevant controller changes and tests
Comment 289 Aleisha Amohia 2021-03-16 03:47:50 UTC
Created attachment 118283 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT
Comment 290 Aleisha Amohia 2021-03-16 03:47:57 UTC
Created attachment 118284 [details] [review]
Bug 15516: Schema updates
Comment 291 Aleisha Amohia 2021-03-16 03:48:04 UTC
Created attachment 118285 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.
Comment 292 Aleisha Amohia 2021-03-16 03:53:23 UTC
Created attachment 118286 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting
Comment 293 Aleisha Amohia 2021-03-16 03:54:03 UTC
(In reply to Kyle M Hall from comment #284)
> What impact will this feature have on the holds queue. As it stands, there
> will be a request in the queue for each available item in the hold group. I
> see the holds to pull report gives a link to the holds group. I think the
> same functionality would be needed for the holds queue report as well.
> 
> In addition, we'll need to make sure this bug and bug 24359 work together.

Have rebased and addressed these points!
Comment 294 Michal Denar 2021-03-16 20:50:32 UTC
Created attachment 118350 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 295 Michal Denar 2021-03-16 20:50:44 UTC
Created attachment 118351 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 296 Michal Denar 2021-03-16 20:50:55 UTC
Created attachment 118352 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 297 Michal Denar 2021-03-16 20:51:05 UTC
Created attachment 118353 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 298 Michal Denar 2021-03-16 20:51:15 UTC
Created attachment 118354 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 299 Michal Denar 2021-03-16 20:51:26 UTC
Created attachment 118355 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 300 David Cook 2021-05-03 01:53:39 UTC Comment hidden (obsolete)
Comment 301 David Cook 2021-05-03 01:54:51 UTC
Created attachment 120368 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 302 David Cook 2021-05-03 01:54:59 UTC
Created attachment 120369 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 303 David Cook 2021-05-03 01:55:08 UTC
Created attachment 120370 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 304 David Cook 2021-05-03 01:55:17 UTC
Created attachment 120371 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 305 David Cook 2021-05-03 01:55:24 UTC
Created attachment 120372 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 306 David Cook 2021-05-03 01:55:33 UTC
Created attachment 120373 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 307 David Cook 2021-05-03 01:56:20 UTC
Rebased and works as expected with following test plan:

My test plan:
0a. Apply the patch
0b. perl ./installer/data/mysql/updatedatabase.pl
0c. perl misc/devel/update_dbix_class_files.pl --koha-conf /etc/koha/sites/kohadev/koha-conf.xml
0d. restart_all
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
(e.g. http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test)
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
(e.g. Check in 39999000001310 )
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold
Comment 308 David Cook 2021-05-03 01:57:00 UTC
However, it's failing the QA test tools:

root@kohadevbox:koha(bug_15516)$ qa
testing 6 commit(s) (applied to 8fe090f 'e3 Bug 27864: (QA follow-up) Select2 ')

Processing files before patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)
Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)

 OK     C4/Reserves.pm
 FAIL   Koha/Hold.pm
   FAIL   critic
                # Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s).

 OK     Koha/HoldGroup.pm
 OK     Koha/HoldGroups.pm
 OK     Koha/Holds.pm
 FAIL   Koha/Schema/Result/HoldGroup.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/Schema/Result/OldReserve.pm
 OK     Koha/Schema/Result/Reserve.pm
 OK     circ/view_holdsqueue.pl
 OK     installer/data/mysql/kohastructure.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc
 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
   FAIL   filters
                wrong_html_filter at line 181 (                    <div>(part of a <a href="/cgi-bin/koha/reserve/hold-group.pl?hold_group_id=[% hold.hold_group_id %]" class="hold-group">hold group</a>)</div>)
                missing_filter at line 181 (                    <div>(part of a <a href="/cgi-bin/koha/reserve/hold-group.pl?hold_group_id=[% hold.hold_group_id %]" class="hold-group">hold group</a>)</div>)

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt
   FAIL   filters
                wrong_html_filter at line 185 (                    <div>(part of a <a href="/cgi-bin/koha/reserve/hold-group.pl?hold_group_id=[% itemsloo.hold_group_id %]" class="hold-group">hold group</a>)</div>)
                missing_filter at line 185 (                    <div>(part of a <a href="/cgi-bin/koha/reserve/hold-group.pl?hold_group_id=[% itemsloo.hold_group_id %]" class="hold-group">hold group</a>)</div>)

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt
   FAIL   filters
                missing_filter at line 23 (              <td>[% hold.priority %]</td>)
                missing_filter at line 24 (              <td>[% hold.reservenotes %]</td>)
   FAIL   forbidden patterns
                forbidden pattern: Title elements must start with the unique information (bug 26703) (line 2)

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
 OK     koha-tmpl/intranet-tmpl/prog/js/hold-group.js
 OK     koha-tmpl/intranet-tmpl/prog/js/holds.js
 OK     opac/opac-reserve.pl
 FAIL   reserve/hold-group.pl
   FAIL   critic
                Subroutine "new" called using indirect syntax at line 25, column 11. See page 349 of PBP.
   FAIL   file permissions
                File must have the exec flag
   FAIL   forbidden patterns
                forbidden pattern: Script permissions is authnotrequired => 0, it could be correct for an OPAC script if it is was you really want error (bug 24663) (line 30)

 OK     reserve/placerequest.pl
 OK     reserve/request.pl
 OK     svc/holds
 OK     t/db_dependent/Koha/Holds.t
 FAIL   t/db_dependent/Reserves/HoldGroup.t
   FAIL   file permissions
                File must have the exec flag


Processing additional checks OK!
Comment 309 David Cook 2021-05-03 02:08:10 UTC
Code looks all right just looking at the diff, so I think fixing the QA test tool failures should resolve things.
Comment 310 David Cook 2021-05-03 05:20:57 UTC
Comment on attachment 120368 [details] [review]
Bug 15516: Database and installer updates

Review of attachment 120368 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/kohastructure.sql
@@ +4350,3 @@
>    CONSTRAINT `reserves_ibfk_4` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE,
>    CONSTRAINT `reserves_ibfk_5` FOREIGN KEY (`itemtype`) REFERENCES `itemtypes` (`itemtype`) ON DELETE CASCADE ON UPDATE CASCADE,
>    CONSTRAINT `reserves_ibfk_6` FOREIGN KEY (`desk_id`) REFERENCES `desks` (`desk_id`) ON DELETE SET NULL ON UPDATE CASCADE

There is a missing comma here which causes this error:

Something went wrong loading file /kohadevbox/koha/installer/data/mysql/kohastructure.sql ([SQL ERROR]: CREATE TABLE `reserves` (
Comment 311 Aleisha Amohia 2021-05-05 05:35:31 UTC
Working on this now
Comment 312 Aleisha Amohia 2021-05-05 22:06:19 UTC
Created attachment 120574 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools
Comment 313 David Cook 2021-05-06 07:00:54 UTC
Still failing the QA tools.

I think the pod coverage one can be ignored, as I think that's a QA tool bug.

You do need to "chmod +x reserve/hold-group.pl t/db_dependent/Reserves/HoldGroup.t".

And it looks like the <title> elements needs to be ordered specific to general. I recall that being a fairly recent requirement: https://wiki.koha-community.org/wiki/Coding_Guidelines#ACC3:_Title_elements_should_contain_unique_information_information_first

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

Processing files before patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)
Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)

 OK     C4/Reserves.pm
 OK     Koha/Hold.pm
 OK     Koha/HoldGroup.pm
 OK     Koha/HoldGroups.pm
 OK     Koha/Holds.pm
 FAIL   Koha/Schema/Result/HoldGroup.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/Schema/Result/OldReserve.pm
 OK     Koha/Schema/Result/Reserve.pm
 OK     circ/view_holdsqueue.pl
 OK     installer/data/mysql/kohastructure.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt
   FAIL   forbidden patterns
                forbidden pattern: Title elements must start with the unique information (bug 26703) (line 2)

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
 OK     koha-tmpl/intranet-tmpl/prog/js/hold-group.js
 OK     koha-tmpl/intranet-tmpl/prog/js/holds.js
 OK     opac/opac-reserve.pl
 FAIL   reserve/hold-group.pl
   FAIL   file permissions
                File must have the exec flag

 OK     reserve/placerequest.pl
 OK     reserve/request.pl
 OK     svc/holds
 OK     t/db_dependent/Koha/Holds.t
 FAIL   t/db_dependent/Reserves/HoldGroup.t
   FAIL   file permissions
                File must have the exec flag


Processing additional checks OK!
Comment 314 Victor Grousset/tuxayo 2021-05-07 00:29:55 UTC
Created attachment 120643 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 315 Victor Grousset/tuxayo 2021-05-07 00:30:03 UTC
Created attachment 120644 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 316 Victor Grousset/tuxayo 2021-05-07 00:30:18 UTC
Created attachment 120645 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 317 Victor Grousset/tuxayo 2021-05-07 00:30:29 UTC
Created attachment 120646 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 318 Victor Grousset/tuxayo 2021-05-07 00:30:38 UTC
Created attachment 120647 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 319 Victor Grousset/tuxayo 2021-05-07 00:30:48 UTC
Created attachment 120648 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 320 Victor Grousset/tuxayo 2021-05-07 00:30:59 UTC
Created attachment 120649 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 321 Victor Grousset/tuxayo 2021-05-07 00:34:19 UTC
Works as expected :)

Note: maxreserves counts the number of holds in the group hold, i.e. it doesn't count it as one. It seems that was intentional. But highlighting that for QA to review.
Holds per day and holds total circ rules count the group as 1.

Signing off. Though above QA fixes still needed.
Comment 322 Aleisha Amohia 2021-05-07 01:05:37 UTC
Created attachment 120650 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order
Comment 323 Victor Grousset/tuxayo 2021-05-07 01:13:19 UTC
Created attachment 120651 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 324 David Cook 2021-05-09 23:53:29 UTC
(In reply to Victor Grousset/tuxayo from comment #321)
> Note: maxreserves counts the number of holds in the group hold, i.e. it
> doesn't count it as one. It seems that was intentional. But highlighting
> that for QA to review.
> Holds per day and holds total circ rules count the group as 1.


That's an interesting point. On one hand, you might want to limit the number of group members. On the other hand, it doesn't make sense to count the group as 1 hold but then also consider group members in terms of max reserves.

Aleisha: What are your thoughts?
Comment 325 David Cook 2021-05-09 23:55:12 UTC
(In reply to David Cook from comment #324)
> That's an interesting point. On one hand, you might want to limit the number
> of group members. On the other hand, it doesn't make sense to count the
> group as 1 hold but then also consider group members in terms of max
> reserves.
> 
> Aleisha: What are your thoughts?

Oh I think I might be conflating 15565 and 15516. I think Victor's comment is still relevant though.
Comment 326 Aleisha Amohia 2021-05-13 04:20:01 UTC
(In reply to Victor Grousset/tuxayo from comment #321)
> Works as expected :)
> 
> Note: maxreserves counts the number of holds in the group hold, i.e. it
> doesn't count it as one. It seems that was intentional. But highlighting
> that for QA to review.
> Holds per day and holds total circ rules count the group as 1.
> 
> Signing off. Though above QA fixes still needed.

Ah I remember now - yes, a hold group is supposed to be treated as one hold because when one item from that group is confirmed to fill the hold, the rest of the group is deleted.

I'll fix this up.
Comment 327 David Cook 2021-05-25 07:07:35 UTC
(In reply to Aleisha Amohia from comment #326)
> Ah I remember now - yes, a hold group is supposed to be treated as one hold
> because when one item from that group is confirmed to fill the hold, the
> rest of the group is deleted.
> 
> I'll fix this up.

How are you going with this one, Aleisha?
Comment 328 Michal Denar 2021-05-25 07:36:42 UTC
Hi Aleisha,
We have been waiting for this functionality for a really long time and impatiently. I will be happy to do further testing. It would be great to get it in version 21.11.
Comment 329 Aleisha Amohia 2021-05-25 20:50:20 UTC
Sorry I did try to fix it up but got too busy with other things. It is on my list and I'll revisit it when I can maybe next week, but if someone else wants to fix it up asap you are more than welcome!
Comment 330 David Cook 2021-05-27 03:56:32 UTC
I can look at fixing up some of those QA errors as they're very minor.

I can take a look at the maxreserves thing too although that might be more complex..
Comment 331 David Cook 2021-06-01 03:18:24 UTC
Comment on attachment 120645 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

Review of attachment 120645 [details] [review]:
-----------------------------------------------------------------

::: reserve/hold-group.pl
@@ +1,1 @@
> +#!/usr/bin/env perl

This has the wrong shebang.
Comment 332 David Cook 2021-06-01 05:19:10 UTC
I thought that I'd run the QA tools again, as I just noticed that there is a follow-up for the QA tools. 

My test plan:
0a. Apply the patch
0b. perl ./installer/data/mysql/updatedatabase.pl
0c. qa

testing 8 commit(s) (applied to 949f0aa '68 Koha 21.05.00 is here!')

Processing files before patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)
Processing files after patches
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 331.
|========================>| 27 / 27 (100.00%)

 OK     C4/Reserves.pm
 OK     Koha/Hold.pm
 OK     Koha/HoldGroup.pm
 OK     Koha/HoldGroups.pm
 OK     Koha/Holds.pm
 FAIL   Koha/Schema/Result/HoldGroup.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

 OK     Koha/Schema/Result/OldReserve.pm
 OK     Koha/Schema/Result/Reserve.pm
 OK     circ/view_holdsqueue.pl
 OK     installer/data/mysql/kohastructure.sql
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt
 OK     koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
 OK     koha-tmpl/intranet-tmpl/prog/js/hold-group.js
 OK     koha-tmpl/intranet-tmpl/prog/js/holds.js
 OK     opac/opac-reserve.pl
 OK     reserve/hold-group.pl
 OK     reserve/placerequest.pl
 OK     reserve/request.pl
 OK     svc/holds
 OK     t/db_dependent/Koha/Holds.t
 OK     t/db_dependent/Reserves/HoldGroup.t

Processing additional checks OK!
Comment 333 David Cook 2021-06-01 05:22:43 UTC
Created attachment 121523 [details] [review]
Bug 15516: Fix shebang in hold-group.pl
Comment 334 David Cook 2021-06-01 05:53:41 UTC
(In reply to Aleisha Amohia from comment #326)
> Ah I remember now - yes, a hold group is supposed to be treated as one hold
> because when one item from that group is confirmed to fill the hold, the
> rest of the group is deleted.
> 
> I'll fix this up.

After reading through the code and further testing and thinking, I don't think that's feasible. It would require much more refactoring to achieve that, and I'm not sure that it's worth it. 

So I think if we pull out the count_grouped() code, then this could actually pass QA, so long as the intention is to create a group of holds, and that fulfilling one hold will clear the other holds. That works well. 

The counting of the hold group as 1 hold does not work at all though, so perhaps that concept should be abandoned (for now at least).
Comment 335 David Cook 2021-06-01 05:57:14 UTC
Test plan setup: 
0a. Apply the patch
0b. perl ./installer/data/mysql/updatedatabase.pl
0c. perl misc/devel/update_dbix_class_files.pl --koha-conf /etc/koha/sites/kohadev/koha-conf.xml
0d. restart_all

Test: Create a hold group from a group of items
1. Go to intranet search, display some results, click on the first 2 checkboxes
and click on 'Place hold' button at the top of the results.
(e.g. http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test)
2. Search for the "koha" patron (cardnumber 42)
3a. Check the 'Treat as hold group' checkbox
3b. Set pickup location for each item
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
Result: PASS

Test: Hold groups count as 1 hold (maxreserves)
1. Change maxreserves to 2
2. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?borrowernumber=51&biblionumber=134
3. Note that it says "koha can only place a maximum of 2 total holds"
4. Change maxreserves to 50
Result: FAIL

Test: Hold groups count as 1 hold ("Holds allowed (total)")
1. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl
2. Change "Holds allowed (total)" to 2
3. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?borrowernumber=51&biblionumber=134
4. Note that it says "koha can only place a maximum of 2 total holds"
5. Change "Holds allowed (total)" to unlimited (by entering a blank value)
Result: FAIL

Test: Hold groups count as 1 hold ("Holds allowed (daily)")
1. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl
2. Change "Holds allowed (daily)" to 2
3. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?borrowernumber=51&biblionumber=134
4. Note it says "No items are available to be placed on hold" and near the bottom of the screen each item says "Daily hold limit reached for patron"
Result: FAIL

Test: Hold groups count as 1 hold (Patron view)
1. Go to http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51
2. Note that 2 holds are listed, although each hold is listed as "part of a hold group" (and the links to that hold group are for the same hold group)
Result: FAIL

Test: Hold groups count as 1 hold (Circulation view)
1. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
2. Note that 2 holds are listed, although each hold is listed as "part of a hold group" (and the links to that hold group are for the same hold group)
Result: FAIL

Test: Hold groups count as 1 hold (OPAC home page view)
1. Go to http://localhost:8080/
2. Note that the screen says "2 holds pending"
Result: FAIL

Test: Hold groups count as 1 hold (OPAC user page view)
1. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds 
2. Note that the screen says "2 holds" without any indication of them being part of the same hold group
Result: FAIL

Test: Hold groups count as 1 hold (OPAC place hold view)
1. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=134
2. Note that the screen says "There are no items that can be placed on hold"
Result: FAIL

Test: Hold groups are disbanded when 1 hold in the group is fulfilled
1. Check in an item of one of the reserved biblios and confirm the hold
(e.g. Check in 39999000001310 )
8. The hold status is changed to Waiting, and all other holds in the hold group are deleted.
Result: PASS

Note: 
The "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Comment 336 David Cook 2021-06-01 06:06:58 UTC
Also, I notice that the text for the release notes say here that this is only for the staff interface, so the change for opac/opac-reserve.pl shouldn't be here at all. 

Since it's only for the staff interface, the concept of a hold group counting as 1 hold also doesn't make sense, since it would require complicated hold calculations that would vary depending on the interface.
Comment 337 David Cook 2021-06-01 06:14:28 UTC
*** Bug 27300 has been marked as a duplicate of this bug. ***
Comment 338 Christopher Brannon 2021-06-01 15:10:43 UTC
(In reply to David Cook from comment #334)
> (In reply to Aleisha Amohia from comment #326)
> > Ah I remember now - yes, a hold group is supposed to be treated as one hold
> > because when one item from that group is confirmed to fill the hold, the
> > rest of the group is deleted.
> > 
> > I'll fix this up.
> 
> After reading through the code and further testing and thinking, I don't
> think that's feasible. It would require much more refactoring to achieve
> that, and I'm not sure that it's worth it. 
> 
> So I think if we pull out the count_grouped() code, then this could actually
> pass QA, so long as the intention is to create a group of holds, and that
> fulfilling one hold will clear the other holds. That works well. 
> 
> The counting of the hold group as 1 hold does not work at all though, so
> perhaps that concept should be abandoned (for now at least).

I'm not sure if I am following correctly or not.  Is the group hold going to count as one placed hold or count for every item in the group?? If it is going to show as multiple holds, then it becomes a problem for anyone that uses it.  The group hold HAS to count as a single hold for this to be useful and not exploited by patrons.

Without looking at the code, logically, it seems like if you have each group hold count as some kind of of proxy hold in the hold system, when Koha looks to see how many holds there are, it should also see how many group holds there are, and each group for that user should just reflect as one hold.  I may be over simplifying this.
Comment 339 Christopher Brannon 2021-06-01 15:14:38 UTC
(In reply to David Cook from comment #336)
> Also, I notice that the text for the release notes say here that this is
> only for the staff interface, so the change for opac/opac-reserve.pl
> shouldn't be here at all. 
> 
> Since it's only for the staff interface, the concept of a hold group
> counting as 1 hold also doesn't make sense, since it would require
> complicated hold calculations that would vary depending on the interface.

I don't know why this was limited to just the staff client.  Maybe it is with the intent that someone will duplicate it for the OPAC?  But if it is going to be just for staff, I think that it does only have the intended job from the original request.  The original request was not for this to be a staff only function.
Comment 340 David Cook 2021-06-02 02:44:18 UTC
(In reply to Christopher Brannon from comment #338)
> Is the group hold going to count as one placed hold or count for every item in the group?? 

At the moment, each member of a group hold counts as its own hold.

> If it is
> going to show as multiple holds, then it becomes a problem for anyone that
> uses it.  The group hold HAS to count as a single hold for this to be useful
> and not exploited by patrons.

I'm not sure what you mean here by "exploited". To me, each member of a group hold counting as a hold is problematic only in terms of hold limits. If you had a limit of 10 holds, and they create 1 hold group with 10 members, then they've exhausted their limit, and that's not necessarily fair to patrons. 

> Without looking at the code, logically, it seems like if you have each group
> hold count as some kind of of proxy hold in the hold system, when Koha looks
> to see how many holds there are, it should also see how many group holds
> there are, and each group for that user should just reflect as one hold.  I
> may be over simplifying this.

I wouldn't say over simplifying heh. It's quite complex.
Comment 341 David Cook 2021-06-02 02:46:47 UTC
(In reply to Christopher Brannon from comment #339)
> I don't know why this was limited to just the staff client.  Maybe it is
> with the intent that someone will duplicate it for the OPAC?  But if it is
> going to be just for staff, I think that it does only have the intended job
> from the original request.  The original request was not for this to be a
> staff only function.

Bug 17217 is for the OPAC side, but I think it's incomplete.

There are libraries out in the world running a local version of this, but it's different to what's in Bugzilla.
Comment 342 Aleisha Amohia 2021-06-02 02:48:36 UTC
Thanks for discussing this everyone. If you just decide what the best behaviour should be and let me know when I need to implement I should be able to get to this later this week.
Comment 343 David Cook 2021-06-02 03:08:59 UTC
(In reply to Aleisha Amohia from comment #342)
> Thanks for discussing this everyone. If you just decide what the best
> behaviour should be and let me know when I need to implement I should be
> able to get to this later this week.

I think that it would be good for them to be counted as 1 hold for the sake of hold limits. However, I think that's probably easier said than done. 

Based on my investigations for Bug 15516 and 15565, it seems like the master code for determining hold limits is a mess. 

One downside of hold groups counting as 1 hold for limits is that it might not be clear to users how many holds they can place as well, although maybe that should be a separate enhancement...
Comment 344 Aleisha Amohia 2021-06-03 02:36:51 UTC
I agree that hold groups need to be counted as one hold, which probably means modifying existing hold count code to always work like 'count_grouped', rather than adding the new count_grouped subroutine.

I think for now there can be no limit to how many holds can be made in one hold group because the borrower will still only get hold out of that group fulfilled, however that can of course be discussed and changed in the future.

I'll just work on getting the functionality of placing, fulfilling, and counting hold groups for now.

Will also remove the OPAC work acknowledging that that's being tackled in Bug 17217.

Aleisha
Comment 345 Aleisha Amohia 2021-06-04 04:09:55 UTC
Created attachment 121623 [details] [review]
Bug 15516: (follow-up) Count group holds always
Comment 346 Aleisha Amohia 2021-06-04 04:12:08 UTC
I decided to amend count_grouped() to count_holds() so it sounds more like the standard subroutine it will become. It needs to use count() which is why I couldn't just overwrite count().

Left the OPAC count_holds() in because borrowers might get group holds placed in the staff client then try to place holds via the OPAC so they need that count to calculate their limit.

I imagine the most recent patch doesn't catch all of the staff interface cases for placing a hold so feel free to tell me what I've missed.
Comment 347 David Cook 2021-06-18 02:29:19 UTC
I haven't tried the latest follow-up, but got a fatal error trying to create a hold group using multiple items from the same bib via the detail page.
Comment 348 David Cook 2021-06-18 04:05:27 UTC
Created attachment 122097 [details] [review]
Bug 15516: Fix hold groups for multi items for one bib
Comment 349 David Cook 2021-06-18 04:12:50 UTC
We should probably rename this one and change the notes, because it's not accurate.

This change lets you place a hold on the next available item from a group of titles from the search result page, but it also lets you place a hold on a group of items for one bib from the detail page.
Comment 350 David Cook 2021-06-18 07:34:01 UTC
Created attachment 122102 [details] [review]
Bug 15516: Add ability to create hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.
Comment 351 David Cook 2021-06-18 07:35:01 UTC
I can't guarantee that my last patches actually apply on master, but I'm firing them off as I'm writing them. I can come back to clean them up...
Comment 352 David Cook 2021-06-18 07:36:02 UTC
I think Bug 17217 is hopeless, so I figure we may as well just do the OPAC side here. It's not that much more work.
Comment 353 Victor Grousset/tuxayo 2021-06-23 04:24:45 UTC
(In reply to David Cook from comment #351)
> I can't guarantee that my last patches actually apply on master

Indeed lol :P

> Patch failed at 0001 Bug 15516: Add ability to create hold groups from OPAC
Comment 354 David Cook 2021-06-25 04:46:50 UTC
(In reply to Victor Grousset/tuxayo from comment #353)
> (In reply to David Cook from comment #351)
> > I can't guarantee that my last patches actually apply on master
> 
> Indeed lol :P
> 
> > Patch failed at 0001 Bug 15516: Add ability to create hold groups from OPAC

I can look at fixing that in early July I think.
Comment 355 David Cook 2021-07-07 04:25:17 UTC
Also noting that there is no mention of hold groups in the OPAC, but hopefully that won't be too hard to add...
Comment 356 David Cook 2021-07-07 06:12:50 UTC
Created attachment 122627 [details] [review]
Bug 15516: Show user's hold groups to user via OPAC

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.
Comment 357 David Cook 2021-07-08 04:34:21 UTC
(In reply to Victor Grousset/tuxayo from comment #353)
> (In reply to David Cook from comment #351)
> > I can't guarantee that my last patches actually apply on master
> 
> Indeed lol :P
> 
> > Patch failed at 0001 Bug 15516: Add ability to create hold groups from OPAC

I'm getting merge conflicts earlier now...

Bug 15516 - Allow to place a hold on first available item from a group of titles

120643 - Bug 15516: Database and installer updates
120644 - Bug 15516: Relevant controller changes and tests
120645 - Bug 15516: Allow to reserve first available item from a group of titles
120646 - Bug 15516: Schema updates
120647 - Bug 15516: (follow-up) Add hold group message to holds queue
120648 - Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue
120649 - Bug 15516: (follow-up) Fixes for qa tools
120651 - Bug 15516: (follow-up) Fixing file permissions and title elements order
121523 - Bug 15516: Fix shebang in hold-group.pl
121623 - Bug 15516: (follow-up) Count group holds always
122097 - Bug 15516: Fix hold groups for multi items for one bib
122102 - Bug 15516: Add ability to create hold groups from OPAC
122627 - Bug 15516: Show user's hold groups to user via OPAC

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15516: Database and installer updates
Applying: Bug 15516: Relevant controller changes and tests
Applying: Bug 15516: Allow to reserve first available item from a group of titles
Using index info to reconstruct a base tree...
M       koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
M       koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
M       opac/opac-reserve.pl
M       reserve/placerequest.pl
M       reserve/request.pl
Falling back to patching base and 3-way merge...
Auto-merging reserve/request.pl
Auto-merging reserve/placerequest.pl
Auto-merging opac/opac-reserve.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 15516: Allow to reserve first available item from a group of titles
Comment 358 David Cook 2021-07-08 04:36:15 UTC
Of course, this is why a lot of useful features never make it into Koha. The master branch moves too fast, and it's just too much work to update features to keep up.
Comment 359 David Cook 2021-07-08 04:56:16 UTC
*** Bug 17217 has been marked as a duplicate of this bug. ***
Comment 360 David Cook 2021-10-26 00:02:09 UTC
Comment on attachment 120644 [details] [review]
Bug 15516: Relevant controller changes and tests

Review of attachment 120644 [details] [review]:
-----------------------------------------------------------------

::: C4/Reserves.pm
@@ +1113,4 @@
>      logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) )
>          if C4::Context->preference('HoldsLog');
>  
> +    # if this hold was part of a group, cancel other holds in the group

This causes every hold with a null hold_group_id to be cancelled. This needs to be wrapped in a check for $hold_group_id first!

@@ +1222,5 @@
>          # Complete transfer if one exists
>          my $transfer = $hold->item->get_transfer;
>          $transfer->receive if $transfer;
> +
> +        # if this hold was part of a group, cancel other holds in the group

This causes every hold with a null hold_group_id to be cancelled. This needs to be wrapped in a check for $hold_group_id first!
Comment 361 David Cook 2021-10-26 00:03:03 UTC
My latest comment shows a very very bad regression which needs to be fixed. It's a simple fix but nasty problem.
Comment 362 Michal Denar 2021-12-01 12:04:27 UTC
Hi Aleisha,
can you fix errors mentioned ba David? I'm ready to test. We really need this feature.

Thank you.
Comment 363 David Cook 2021-12-01 22:57:18 UTC
(In reply to Michal Denar from comment #362)
> Hi Aleisha,
> can you fix errors mentioned ba David? I'm ready to test. We really need
> this feature.
> 
> Thank you.

My hope is to get some sponsorship to keep working on this in the coming months. I've got it running on a customized 21.05 Koha, but if we don't work on upstreaming it soon, I fear it will become very challenging to try again in the future...
Comment 364 Michal Denar 2021-12-02 09:20:47 UTC
I agree with David. We need finish this feature ASAP. Lets talk about sponsorship.
Comment 365 Aleisha Amohia 2021-12-02 21:30:10 UTC
Hi everyone, I'll try to fix the patches today!

Aleisha
Comment 366 Michal Denar 2021-12-02 21:32:04 UTC
Great news, Aleisha!
Comment 367 Aleisha Amohia 2021-12-03 02:32:32 UTC
Created attachment 128189 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 368 Aleisha Amohia 2021-12-03 02:32:41 UTC
Created attachment 128190 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 369 Aleisha Amohia 2021-12-03 02:32:49 UTC
Created attachment 128191 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 370 Aleisha Amohia 2021-12-03 02:32:57 UTC
Created attachment 128192 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 371 Aleisha Amohia 2021-12-03 02:33:05 UTC
Created attachment 128193 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 372 Aleisha Amohia 2021-12-03 02:33:13 UTC
Created attachment 128194 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 373 Aleisha Amohia 2021-12-03 02:33:21 UTC
Created attachment 128195 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 374 Aleisha Amohia 2021-12-03 02:33:29 UTC
Created attachment 128196 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 375 Aleisha Amohia 2021-12-03 02:33:37 UTC
Created attachment 128197 [details] [review]
Bug 15516: Fix shebang in hold-group.pl
Comment 376 Aleisha Amohia 2021-12-03 02:33:44 UTC
Created attachment 128198 [details] [review]
Bug 15516: (follow-up) Count group holds always
Comment 377 Aleisha Amohia 2021-12-03 02:33:52 UTC
Created attachment 128199 [details] [review]
Bug 15516: Fix hold groups for multi items for one bib
Comment 378 Aleisha Amohia 2021-12-03 02:34:01 UTC
Created attachment 128200 [details] [review]
Bug 15516: Add ability to create hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.
Comment 379 Aleisha Amohia 2021-12-03 02:34:09 UTC
Created attachment 128201 [details] [review]
Bug 15516: Show user's hold groups to user via OPAC

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.
Comment 380 Aleisha Amohia 2021-12-03 02:34:17 UTC
Created attachment 128202 [details] [review]
Bug 15516: (follow-up) Prevent cancelling unintended holds

And a fix for consistent TT code
Comment 381 Michal Denar 2021-12-03 08:35:13 UTC
Hi Aleisha,
I just tested the new patches. Everything seems to be working, but the model is empty in all places after clicking on "hold Group". https://prnt.sc/21iwwv5
Comment 382 Aleisha Amohia 2021-12-06 03:00:24 UTC
Thanks Michal I will investigate.
Comment 383 Aleisha Amohia 2021-12-06 03:30:27 UTC
Created attachment 128255 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 384 Aleisha Amohia 2021-12-06 03:30:37 UTC
Created attachment 128256 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 385 Aleisha Amohia 2021-12-06 03:30:45 UTC
Created attachment 128257 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 386 Aleisha Amohia 2021-12-06 03:30:54 UTC
Created attachment 128258 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 387 Aleisha Amohia 2021-12-06 03:31:02 UTC
Created attachment 128259 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 388 Aleisha Amohia 2021-12-06 03:31:10 UTC
Created attachment 128260 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 389 Aleisha Amohia 2021-12-06 03:31:18 UTC
Created attachment 128261 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 390 Aleisha Amohia 2021-12-06 03:31:27 UTC
Created attachment 128262 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 391 Aleisha Amohia 2021-12-06 03:31:34 UTC
Created attachment 128263 [details] [review]
Bug 15516: Fix shebang in hold-group.pl
Comment 392 Aleisha Amohia 2021-12-06 03:31:44 UTC
Created attachment 128264 [details] [review]
Bug 15516: (follow-up) Count group holds always
Comment 393 Aleisha Amohia 2021-12-06 03:31:52 UTC
Created attachment 128265 [details] [review]
Bug 15516: Fix hold groups for multi items for one bib
Comment 394 Aleisha Amohia 2021-12-06 03:32:00 UTC
Created attachment 128266 [details] [review]
Bug 15516: Add ability to create hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.
Comment 395 Aleisha Amohia 2021-12-06 03:32:09 UTC
Created attachment 128267 [details] [review]
Bug 15516: Show user's hold groups to user via OPAC

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.
Comment 396 Aleisha Amohia 2021-12-06 03:32:17 UTC
Created attachment 128268 [details] [review]
Bug 15516: (follow-up) Prevent cancelling unintended holds

And a fix for consistent TT code
Comment 397 Aleisha Amohia 2021-12-06 03:32:25 UTC
Created attachment 128269 [details] [review]
Bug 15516: (follow-up) Fix hold group details not showing
Comment 398 Michal Denar 2021-12-06 20:17:42 UTC
Created attachment 128285 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 399 Michal Denar 2021-12-06 20:17:52 UTC
Created attachment 128286 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 400 Michal Denar 2021-12-06 20:18:01 UTC
Created attachment 128287 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 401 Michal Denar 2021-12-06 20:18:11 UTC
Created attachment 128288 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 402 Michal Denar 2021-12-06 20:18:19 UTC
Created attachment 128289 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 403 Michal Denar 2021-12-06 20:18:28 UTC
Created attachment 128290 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 404 Michal Denar 2021-12-06 20:18:38 UTC
Created attachment 128291 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 405 Michal Denar 2021-12-06 20:18:47 UTC
Created attachment 128292 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 406 Michal Denar 2021-12-06 20:18:55 UTC
Created attachment 128293 [details] [review]
Bug 15516: Fix shebang in hold-group.pl

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 407 Michal Denar 2021-12-06 20:19:04 UTC
Created attachment 128294 [details] [review]
Bug 15516: (follow-up) Count group holds always

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 408 Michal Denar 2021-12-06 20:19:13 UTC
Created attachment 128295 [details] [review]
Bug 15516: Fix hold groups for multi items for one bib

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 409 Michal Denar 2021-12-06 20:19:21 UTC
Created attachment 128296 [details] [review]
Bug 15516: Add ability to create hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 410 Michal Denar 2021-12-06 20:19:29 UTC
Created attachment 128297 [details] [review]
Bug 15516: Show user's hold groups to user via OPAC

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 411 Michal Denar 2021-12-06 20:19:37 UTC
Created attachment 128298 [details] [review]
Bug 15516: (follow-up) Prevent cancelling unintended holds

And a fix for consistent TT code

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 412 Michal Denar 2021-12-06 20:19:46 UTC
Created attachment 128299 [details] [review]
Bug 15516: (follow-up) Fix hold group details not showing

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 413 Fridolin Somers 2022-01-15 05:41:17 UTC
Arf, this is somehow linked to other feature in development :

Bug 24857 - Add ability to group items for records
Bug 24023 - Add ability to create bundles of items
Bug 28854 - [ALTERNATIVE] Add ability to create bundles of items

We should think on how not put too much complexity in the system (already really complex).
Comment 414 Martin Renvoize 2022-01-18 11:54:15 UTC
It's not really related to the Bundles stuff.. whilst that development does indeed allow items to be grouped.. the grouping logic is entirely distinct and it's more about the circulation of those bundled sets.
Comment 415 David Cook 2022-04-11 06:38:16 UTC
Yeah, as Martin says, it's totally different. 

By the way, this has been used in production for years now. I can't recall if it's been 4 years or 6 years, but a while in any case.
Comment 416 Michal Denar 2022-04-11 08:50:19 UTC
Hi,
We have been waiting for this feature for a really long time. We will use it mainly when dealing with students who require documents for school work. The idea is that they receive reading lists and then request the titles from the local library. They don't actually care which particular title they get, but they need one from the list. This feature would be very helpful in processing this kind of requests. 
Yes, Koha is a complex system. Because it is global and reflects different approaches to library work. Bundles of titles are actually an extension of this, which is almost done now. A sort of generalization of the ability to make reservations over a list of titles, extended with the ability to define groups more comprehensively than from search results.
I'm in favor of pushing the feature into production. At the same time, let's debate how to deal with requests for complex functionality in general. Would it make sense to design functionality more generally and then provide extensions with plugins? We also need to keep sustainability in mind.
Comment 417 Victor Grousset/tuxayo 2022-07-30 19:34:34 UTC
Hi :) I tried to rebase because why not, but hit a wall on the changes on ModReserveFill which was moved (and changed a lot) to Hold::fill(). I don't know where is a right place to put the new code:

    # if this hold was part of a group, cancel other holds in the group
    my @holds = Koha::Holds->search({ hold_group_id => $hold->hold_group_id });
    foreach my $h ( @holds ) {
        $h->cancel unless $h->reserve_id == $hold->reserve_id;
    }


Also, maybe these lines are also needed in Hold::cancel()
Not sure at all.

I would gladly give another try if someone know what to do about the two above points.
Comment 418 David Cook 2022-08-01 02:07:24 UTC
(In reply to Victor Grousset/tuxayo from comment #417)
> I would gladly give another try if someone know what to do about the two
> above points.

Hopefully Aleisha can provide some insight.

In theory, I'll look at this again one day, but I don't know when.
Comment 419 Aleisha Amohia 2022-08-02 23:36:40 UTC
Thanks all, I'm taking a look now and will pass on anything I'm not sure about!
Comment 420 Aleisha Amohia 2022-08-04 01:56:50 UTC
Created attachment 138576 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 421 Aleisha Amohia 2022-08-04 01:57:00 UTC
Created attachment 138577 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 422 Aleisha Amohia 2022-08-04 01:57:09 UTC
Created attachment 138578 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

This feature only applies to the staff client.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 423 Aleisha Amohia 2022-08-04 01:57:17 UTC
Created attachment 138579 [details] [review]
Bug 15516: (follow-up) Add hold group message to holds queue

A hold that is part of a hold group will be indicated as such in the
holds queue.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 424 Aleisha Amohia 2022-08-04 01:57:26 UTC
Created attachment 138580 [details] [review]
Bug 15516: (follow-up) Remove other holds in hold_group from holds_queue

When one hold is fulfilled or set waiting

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 425 Aleisha Amohia 2022-08-04 01:57:35 UTC
Created attachment 138581 [details] [review]
Bug 15516: (follow-up) Fixes for qa tools

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 426 Aleisha Amohia 2022-08-04 01:57:44 UTC
Created attachment 138582 [details] [review]
Bug 15516: (follow-up) Fixing file permissions and title elements order

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 427 Aleisha Amohia 2022-08-04 01:57:55 UTC
Created attachment 138583 [details] [review]
Bug 15516: Fix shebang in hold-group.pl

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 428 Aleisha Amohia 2022-08-04 01:58:04 UTC
Created attachment 138584 [details] [review]
Bug 15516: (follow-up) Count group holds always

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 429 Aleisha Amohia 2022-08-04 01:58:13 UTC
Created attachment 138585 [details] [review]
Bug 15516: Fix hold groups for multi items for one bib

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 430 Aleisha Amohia 2022-08-04 01:58:24 UTC
Created attachment 138586 [details] [review]
Bug 15516: Add ability to create hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 431 Aleisha Amohia 2022-08-04 01:58:33 UTC
Created attachment 138587 [details] [review]
Bug 15516: Show user's hold groups to user via OPAC

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 432 Aleisha Amohia 2022-08-04 01:58:43 UTC
Created attachment 138588 [details] [review]
Bug 15516: (follow-up) Prevent cancelling unintended holds

And a fix for consistent TT code

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 433 Aleisha Amohia 2022-08-04 01:58:51 UTC
Created attachment 138589 [details] [review]
Bug 15516: (follow-up) Fix hold group details not showing

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 434 Aleisha Amohia 2022-08-04 02:05:57 UTC
(In reply to Victor Grousset/tuxayo from comment #417)
> Hi :) I tried to rebase because why not, but hit a wall on the changes on
> ModReserveFill which was moved (and changed a lot) to Hold::fill(). I don't
> know where is a right place to put the new code:
> 
>     # if this hold was part of a group, cancel other holds in the group
>     my @holds = Koha::Holds->search({ hold_group_id => $hold->hold_group_id
> });
>     foreach my $h ( @holds ) {
>         $h->cancel unless $h->reserve_id == $hold->reserve_id;
>     }
> 
> 
> Also, maybe these lines are also needed in Hold::cancel()
> Not sure at all.
> 
> I would gladly give another try if someone know what to do about the two
> above points.

Have rebased the patches in case someone can come along and sign them off - but will now work on putting this code in the right place and squashing some of these patches. Welcome to test in the meantime.
Comment 435 Aleisha Amohia 2022-08-04 04:47:33 UTC
Created attachment 138593 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 436 Aleisha Amohia 2022-08-04 04:47:44 UTC
Created attachment 138594 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 437 Aleisha Amohia 2022-08-04 04:47:53 UTC
Created attachment 138595 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
The other holds will also be removed from the holds queue.
9. Confirm tests pass
- t/db_dependent/Koha/Holds.t
- t/db_dependent/Reserves/HoldGroup.t
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
- circ/view_holdsqueue.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 438 Aleisha Amohia 2022-08-04 04:48:02 UTC
Created attachment 138596 [details] [review]
Bug 15516: Add ability to create and view hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 439 Aleisha Amohia 2022-08-04 04:48:12 UTC
Created attachment 138597 [details] [review]
Bug 15516: Schema updates
Comment 440 Michal Denar 2022-08-06 19:11:54 UTC
Created attachment 138738 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 441 Michal Denar 2022-08-06 19:12:04 UTC
Created attachment 138739 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 442 Michal Denar 2022-08-06 19:12:14 UTC
Created attachment 138740 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
The other holds will also be removed from the holds queue.
9. Confirm tests pass
- t/db_dependent/Koha/Holds.t
- t/db_dependent/Reserves/HoldGroup.t
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
- circ/view_holdsqueue.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 443 Michal Denar 2022-08-06 19:12:23 UTC
Created attachment 138741 [details] [review]
Bug 15516: Add ability to create and view hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 444 Michal Denar 2022-08-06 19:12:38 UTC
Created attachment 138742 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 445 David Cook 2022-08-08 03:22:13 UTC
My test plan:
0a) koha-upgrade-schema kohadev
0b) perl misc/devel/update_dbix_class_files.pl --koha-conf /etc/koha/sites/kohadev/koha-conf.xml #NOTE: this step seemed unnecessary
0c) koha-plack --restart kohadev

__OPAC results__
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Checkbox "Treat as hold group (next available item fulfills all holds)"
5. Click "Confirm hold"
6. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl
7. Click "Holds" 
8. Confirm the two holds appear with the text "part of a hold group",
and when you click on that text it shows you both hold group titles
9. Note the "Holds" tab shows 1 hold signifying the 2 holds in the hold group count as 1 hold total

10. In the staff interface, check in "39999000001310"
11. Click "Confirm hold (Y)"
12. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
13. Note only 1 hold appears, and when you click on "part of a hold group" it only shows 1 hold title

12. In the OPAC, go to http://localhost:8080/cgi-bin/koha/opac-user.pl
*13. Note the following error:
Template process failed: undef error - Can't call method "rule_value" on an undefined value at /kohadevbox/koha/Koha/Hold.pm line 464.
 at /kohadevbox/koha/C4/Templates.pm line 127
 
14. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
15. Cancel the hold
 
__OPAC detail__
1. Go to http://localhost:8080/cgi-bin/koha/opac-reserve.pl?biblionumber=134
2. Checkbox "Treat as hold group (next available item fulfills all holds)" 
3. Click "Show more options"
4. Click "A specific item"
*5. Note that without https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565 you can't select multiple items,
so it doesn't make much sense to show "Treat as hold group" on this page... 
 
__Staff interface results__
1. Go to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Search for patron "koha"
5. Checkibox "Treat as hold group"
6. Choose "Centerville" as "Pickup at"
7. Click "Place holds"
8. Click on "part of a hold group" on the each hold and note the modal with "Hold group" appears showing the 2 titles
9. Check in "39999000001310"
10. Click "Confirm hold (Y)"
11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
12. Cancel the hold

__Staff interface detail__
1. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=134
2. Search for patron "koha"
3. Checkbox both items
4. Checkbox "Treat as hold group"
5. Click "Place hold"
*6. Note the following error: 
Alert box appears saying "- Please select an item to place a hold"
*7. In theory, one should be able to place holds on multiple items, but I think
in practice https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565 is needed
to make that happen, so it might not make sense to show "Treat as hold group" on this page
at this time

--

On http://localhost:8080/cgi-bin/koha/opac-main.pl it will say "2 holds pending", but when you click on that link it will show "Holds (1)" and then "Holds (2 total)". I think that this is likely to confuse users... while I know libraries want hold groups to count as 1 hold, I'm not sure that Koha can really handle that in a coherent way...
Comment 446 David Cook 2022-08-08 03:23:36 UTC
I am keen to get this functionality into Koha, so I don't know that I'd consider many of my above asterisk comments above blockers per se, but I had to fail it for  the fatal error at http://localhost:8080/cgi-bin/koha/opac-user.pl#opac-user-holds: 

Template process failed: undef error - Can't call method "rule_value" on an undefined value at /kohadevbox/koha/Koha/Hold.pm line 464.
 at /kohadevbox/koha/C4/Templates.pm line 127
Comment 447 David Cook 2022-08-08 03:33:16 UTC
Hmm this might be due to bug 22456 and it might be my fault...

I'm going to try pulling the latest Docker image and see if that makes a difference... but  might be a problem in master caused by bug 22456...
Comment 448 David Cook 2022-08-08 04:44:40 UTC
Ok I'm pretty sure the fatal error is a bug in master. 

I'm going to leave this as Failed QA for the moment just because we can't really progress it until the bug in master is fixed...
Comment 449 David Cook 2022-08-08 07:04:14 UTC
Created attachment 138772 [details] [review]
Bug 15516: [QA followup] Only show hold groups for multi hold in OPAC
Comment 450 David Cook 2022-08-08 07:04:47 UTC
My test plan:
0a) koha-upgrade-schema kohadev
0b) koha-plack --restart kohadev

__OPAC results__
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Checkbox "Treat as hold group (next available item fulfills all holds)"
5. Click "Confirm hold"
6. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl
7. Click "Holds" 
8. Confirm the two holds appear with the text "part of a hold group",
and when you click on that text it shows you both hold group titles
9. Note the "Holds" tab shows 1 hold signifying the 2 holds in the hold group count as 1 hold total

10. In the staff interface, check in "39999000001310"
11. Click "Confirm hold (Y)"
12. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
13. Note only 1 hold appears, and when you click on "part of a hold group" it only shows 1 hold title

12. In the OPAC, go to http://localhost:8080/cgi-bin/koha/opac-user.pl
13. Note that only 1 hold appears now 
14. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
15. Cancel the hold
 
__Staff interface results__
1. Go to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Search for patron "koha"
5. Checkibox "Treat as hold group"
6. Choose "Centerville" as "Pickup at"
7. Click "Place holds"
8. Click on "part of a hold group" on the each hold and note the modal with "Hold group" appears showing the 2 titles
9. Check in "39999000001310"
10. Click "Confirm hold (Y)"
11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
12. Cancel the hold
 
__OPAC detail__
NOTE: This is irrelevant without https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565
so 15565 can focus on implementing it.
 
__Staff interface detail__
1. Go to http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=134
2. Search for patron "koha"
3. Checkbox both items
4. Checkbox "Treat as hold group"
5. Click "Place hold"
*6. Note the following error: 
Alert box appears saying "- Please select an item to place a hold"
Comment 451 David Cook 2022-08-08 07:07:29 UTC
(In reply to David Cook from comment #450)
> __Staff interface detail__
> 1. Go to
> http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=134
> 2. Search for patron "koha"
> 3. Checkbox both items
> 4. Checkbox "Treat as hold group"
> 5. Click "Place hold"
> *6. Note the following error: 
> Alert box appears saying "- Please select an item to place a hold"

Failing at the moment as it appears to have broken item-specific holds (even without hold groups and multihold) on the staff interface. 

I've run out of time to keep working on this today, so leaving it here for now. I'll see if I can look at it again tomorrow.
Comment 452 David Cook 2022-08-09 00:44:28 UTC
Created attachment 138868 [details] [review]
Bug 15516: [QA followup] Fix item-specific holds in staff interface
Comment 453 David Cook 2022-08-09 00:49:50 UTC
I fixed the item-specific holds on the staff interface, but I'm Failing QA again, as I've discovered that the checkboxes are irrelevant when placing a staff interface multi-hold:

http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=29&amp;biblionumber=114&amp;biblionumber=134&amp;multi_hold=1&borrowernumber=51

The biblionumbers are in hidden inputs, which get sent no matter what. (The OPAC is fine though.)

--

Aleisha: Happy for you to squash my QA followup patches into your patches as well going forward to make it easier to maintain the patches.
Comment 454 David Cook 2022-08-09 00:51:32 UTC
Created attachment 138869 [details] [review]
Bug 15516: [QA followup] Only show hold groups for multi hold in staff interface
Comment 455 David Cook 2022-08-09 01:16:34 UTC
Another thing I haven't checked here is how it works with hold limits... that's a bit of a rabbithole as well...
Comment 456 David Cook 2022-11-15 05:12:38 UTC
Aleisha, do you have clients using this functionality?
Comment 457 Aleisha Amohia 2022-11-15 19:59:01 UTC
(In reply to David Cook from comment #456)
> Aleisha, do you have clients using this functionality?

Catalyst had received sponsorship to work on this for a library years ago, they may still be using a version of this in production but I'm not sure as we don't work with that library anymore and sponsorship has run out.
Comment 458 David Cook 2024-02-13 05:41:44 UTC
Created attachment 162076 [details] [review]
Bug 15516: Database and installer updates

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 459 David Cook 2024-02-13 05:41:54 UTC
Created attachment 162077 [details] [review]
Bug 15516: Schema updates

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 460 David Cook 2024-02-13 05:42:01 UTC
Created attachment 162078 [details] [review]
Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 461 David Cook 2024-02-13 05:42:08 UTC
Created attachment 162079 [details] [review]
Bug 15516: Allow to reserve first available item from a group of titles

It can be useful, for instance, if a library has the same title from
different publishers (so 1 title but several biblio records) but the
user only wants a copy of the book, regardless of the publisher.

Test plan:
0. Run updatedatabase.pl and misc/devel/update_dbix_class_files.pl
1. Go to intranet search, display some results, click on some checkboxes
and click on 'Place hold' button at the top of the results.
2. Search for a patron
3. Check the 'Treat as hold group' checkbox
4. Click on 'Place hold'
5. In the next screen you should see all the holds you placed with the
additional text '(part of a hold group)' in Details column.
6. Click on the "hold group" link. A modal window should appear with
the content of the hold group (a list of holds)
7. Check in an item of one of the reserved biblios and confirm the hold
8. The hold status is changed to Waiting, and all other holds are deleted.
The other holds will also be removed from the holds queue.
9. Confirm tests pass
- t/db_dependent/Koha/Holds.t
- t/db_dependent/Reserves/HoldGroup.t
Note: the "hold group" link is displayed in the following pages:
- reserve/request.pl
- circ/circulation.pl
- members/moremember.pl
- circ/pendingreserves.pl
- circ/view_holdsqueue.pl
Note: A hold group is counted as only one hold

Sponsored-by: Catalyst IT

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 462 David Cook 2024-02-13 05:42:14 UTC
Created attachment 162080 [details] [review]
Bug 15516: Tidy reserve/placerequest.pl

This includes tidying existing lines, so it'll be a separate patch,
and not squashed into the hold group code changes
Comment 463 David Cook 2024-02-13 05:42:20 UTC
Created attachment 162081 [details] [review]
Bug 15516: Add ability to create and view hold groups from OPAC

This patch adds a "Treat as hold group" to opac-reserve.pl,
which lets OPAC users create a hold group which can include
1+ bibs and either bib-level holds, or item-level holds, or both.

This patch shows the item-level and bib-level holds in a hold group
for the user's holds in the OPAC. This includes both the opac-user.pl
page and the self-checkout hold display.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 464 David Cook 2024-02-13 05:42:27 UTC
Created attachment 162082 [details] [review]
Bug 15516: (follow-up) Only show hold groups for multi hold in OPAC
Comment 465 David Cook 2024-02-13 05:42:33 UTC
Created attachment 162083 [details] [review]
Bug 15516: (follow-up) Only show hold groups for multi hold in staff interface
Comment 466 David Cook 2024-02-13 05:42:39 UTC
Created attachment 162084 [details] [review]
Bug 15516: Add 'DisplayAddHoldGroups' system preference to toggle ability to add titles to hold groups

These patches add the ability to create hold groups across multiple bibs in the staff interface or OPAC.
These hold groups are fulfilled by the first item checked in across the hold group.
Comment 467 David Cook 2024-02-13 05:52:12 UTC
QA tool seems mostly happy. I think it just has issues with Koha schema files which are irrelevant and sysprefs.sql which isn't related to this change.

--

 OK     C4/Reserves.pm

 OK     Koha/Hold.pm

 OK     Koha/HoldGroup.pm

 OK     Koha/HoldGroups.pm

 OK     Koha/Holds.pm

 FAIL   Koha/Schema/Result/HoldGroup.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 15)

 WARN   Koha/Schema/Result/OldReserve.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 136, now: 152)

 WARN   Koha/Schema/Result/Reserve.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 161, now: 177)

 OK     circ/circulation.pl

 OK     circ/view_holdsqueue.pl

 OK     installer/data/mysql/atomicupdate/bug_15516_-_hold_group_table.pl

 OK     installer/data/mysql/kohastructure.sql

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

 OK     koha-tmpl/intranet-tmpl/prog/en/includes/hold-group-modal.inc

 OK     koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingreserves.tt

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/circ/view_holdsqueue.tt

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt

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

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

 OK     koha-tmpl/intranet-tmpl/prog/js/hold-group.js

 OK     koha-tmpl/intranet-tmpl/prog/js/holds.js

 OK     koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc

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

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

 OK     koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/sco-main.tt

 OK     koha-tmpl/opac-tmpl/bootstrap/js/holds.js

 OK     members/moremember.pl

 OK     opac/opac-reserve.pl

 OK     opac/opac-user.pl

 OK     reserve/hold-group.pl

 OK     reserve/placerequest.pl

 OK     reserve/request.pl

 OK     svc/holds

 OK     t/db_dependent/Koha/Holds.t

 OK     t/db_dependent/Reserves/HoldGroup.t


Processing additional checks OK!

Running tests (2)
        * Proving /kohadevbox/koha/t/db_dependent/Reserves/HoldGroup.t OK!
        * Proving /kohadevbox/koha/t/db_dependent/Koha/Holds.t OK!
Comment 468 David Cook 2024-02-13 05:52:45 UTC
My test plan:
0a) koha-upgrade-schema kohadev
0b) koha-plack --restart kohadev

__OPAC results__
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Checkbox "Treat as hold group (next available item fulfills all holds)"
5. Click "Confirm hold"
6. Go to http://localhost:8080/cgi-bin/koha/opac-user.pl
7. Click "Holds" 
8. Confirm the two holds appear with the text "part of a hold group",
and when you click on that text it shows you both hold group titles
9. Note the "Holds" tab shows 1 hold signifying the 2 holds in the hold group count as 1 hold total

10. In the staff interface, check in "39999000001310"
11. Click "Confirm hold (Y)"
12. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
13. Note only 1 hold appears, and when you click on "part of a hold group" it only shows 1 hold title

12. In the OPAC, go to http://localhost:8080/cgi-bin/koha/opac-user.pl
13. Note that only 1 hold appears now 
14. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
15. Cancel the hold
 
__Staff interface results__
1. Go to http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=test
2. Checkbox results 1 and 2
3. Click "Place hold" at the top of the search results
4. Search for patron "koha"
5. Checkibox "Treat as hold group"
6. Choose "Centerville" as "Pickup at"
7. Click "Place holds"
8. Click on "part of a hold group" on the each hold and note the modal with "Hold group" appears showing the 2 titles
9. Check in "39999000001310"
10. Click "Confirm hold (Y)"
11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
12. Cancel the hold
 
__OPAC detail__
NOTE: This is irrelevant without https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565
so 15565 can focus on implementing it.
 
__Staff interface detail__
NOTE: This is irrelevant without https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15565
so 15565 can focus on implementing it.
Comment 469 David Cook 2024-03-12 04:59:43 UTC
Found a nasty bug in Koha::Holds->count_holds(). I'm going to need to review this code in-depth again...
Comment 470 David Cook 2024-03-12 05:19:50 UTC
Comment on attachment 162078 [details] [review]
Bug 15516: Relevant controller changes and tests

Review of attachment 162078 [details] [review]:
-----------------------------------------------------------------

::: Koha/Holds.pm
@@ +213,5 @@
> +
> +sub count_holds {
> +    my ( $self, $search_params ) = @_;
> +
> +    $search_params = {

These $search_params lines redefine the variable instead of updating the variable. This means that you end up selecting all holds and not just the holds for particular criteria (like an individual patron).
Comment 471 David Cook 2024-03-12 06:04:38 UTC
As I've noted before, trying to count multiple hold group members as part of the same hold has some fundamental problems. 

We'd need to pass the hold group to CanBookBeReserved/CanItemBeReserved to handle this better...

It's tempting to spin out a new report just for refactoring those functions to take $args instead. 

I want to make an API endpoint at some point too for whether or not a reserve can be placed. I think National Library of Finland have done work on this, although it's more discovery interface specific so might not quite meet what I'd want it for...