Bug 34234

Summary: Item groups dropdown in detail page modal does not respect display order
Product: Koha Reporter: Emily Lamancusa (emlam) <emily.lamancusa>
Component: CatalogingAssignee: Lucas Gass (lukeg) <lucas>
Status: Pushed to oldstable --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, lucas, m.de.rooy, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34232
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.04,23.05.10
Circulation function:
Bug Depends on: 24857    
Bug Blocks:    
Attachments: Bug 34234: Add template plugin for listing and sorting ItemGroups
Bug 34234: Add sub for count and logic to additem page
Bug 34234: Add template plugin for listing and sorting ItemGroups
Bug 34234: Add sub for count and logic to additem page
Bug 34234: add a unit test
Bug 34234: add a unit test
Bug 34234: add a unit test
Bug 34234: Respect display_order in additem.tt and detail.tt
Bug 34234: Respect display_order in additem.tt and detail.tt
Bug 34234: Respect display_order in additem.tt and detail.tt

Description Emily Lamancusa (emlam) 2023-07-07 20:47:22 UTC
To recreate:

1. EnableItemGroups
2. Find a record and add some new item groups with display orders that are different from the order in which the groups were added
3. Check the checkbox next to one or more items and click the link to "Add/move to item group"
4. Notice that the values in the dropdown do not respect the display order.
Comment 1 Owen Leonard 2023-09-13 10:37:04 UTC

*** This bug has been marked as a duplicate of bug 34232 ***
Comment 2 Emily Lamancusa (emlam) 2023-10-27 19:30:18 UTC
This is not exactly a duplicate of 34232 - it's the same problem, but in a different place.
Comment 3 Lucas Gass (lukeg) 2023-10-31 17:26:50 UTC Comment hidden (obsolete)
Comment 4 Lucas Gass (lukeg) 2023-10-31 17:29:15 UTC
I'm leaving this here to see what people think. This adds a template plugin for ItemGroups. If liked we can apply this elsewhere in Koha where we display item groups. 

Not sure if a template plugin is better than trying to retrieve the item groups from /api/v1/biblios/{biblionumber}/item_groups
Comment 5 Lucas Gass (lukeg) 2023-11-02 22:25:44 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2023-11-03 11:56:56 UTC
Created attachment 158328 [details] [review]
Bug 34234: Add template plugin for listing and sorting ItemGroups

This patch adds a template plugin to handle the retreiving and sorting of ItemGroups.

1. EnableItemGroups
2. Find a record and add some new item groups with display orders that are different from the order in which the groups were added
3. Check the checkbox next to one or more items and click the link to "Add/move to item group"
4. Notice that the values in the dropdown do not respect the display order.
5. Apply patch, restart all
6. Check again, sorting should be correct.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Owen Leonard 2023-11-03 11:56:58 UTC
Created attachment 158329 [details] [review]
Bug 34234: Add sub for count and logic to additem page

To Test:
1. EnableItemGroups
2. Find a record and add some new item groups with different display orders
3. Now edit an item on the record and scroll down to the dropdown underneath "+ Add to item group"
4. Notice that the values in the dropdown do not respect the display order.
5. Apply patch and restart_all
6. The display order should now be respected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Katrin Fischer 2023-11-05 11:39:57 UTC
Can we please get some unit tests for the new module?
Koha/Template/Plugin/ItemGroups.pm
Comment 9 Lucas Gass (lukeg) 2024-01-03 00:27:18 UTC
Created attachment 160442 [details] [review]
Bug 34234: add a unit test
Comment 10 Lucas Gass (lukeg) 2024-01-04 15:37:50 UTC
Created attachment 160542 [details] [review]
Bug 34234: add a unit test
Comment 11 Lucas Gass (lukeg) 2024-01-04 16:05:36 UTC
Created attachment 160543 [details] [review]
Bug 34234: add a unit test
Comment 12 Jonathan Druart 2024-01-12 08:48:07 UTC
Why using TT template?
Comment 13 Jonathan Druart 2024-01-12 08:55:06 UTC
Replace
  [% FOREACH ig IN biblio.item_groups %]
With
[% FOREACH ig IN biblio.item_groups.search({}, {order_by => 'display_order'}) %]

And you are done. I really don't understand the need of a TT plugin for that purpose.

You could suggest a Koha::Biblio::ItemGroups->search_ordered method however.
Comment 14 Lucas Gass (lukeg) 2024-01-12 17:38:00 UTC
Created attachment 160992 [details] [review]
Bug 34234: Respect display_order in additem.tt and detail.tt

1. APPLY PATCH
2. EnableItemGroups
3. Find a record and add some new item groups with display orders that are different from the order in which the groups were added
4. Check the checkbox next to one or more items and click the link to "Add/move to item group"
5. Ensure display order is correct
6. Now add a new item to the record and scroll down to the dropdown underneath "+ Add to item group"
7. Display order should be correct.
Comment 15 Lucas Gass (lukeg) 2024-01-12 17:38:29 UTC
New patchset, back to NSO.
Comment 16 David Nind 2024-01-13 18:02:35 UTC
Created attachment 161019 [details] [review]
Bug 34234: Respect display_order in additem.tt and detail.tt

1. APPLY PATCH
2. EnableItemGroups
3. Find a record and add some new item groups with display orders that are different from the order in which the groups were added
4. Check the checkbox next to one or more items and click the link to "Add/move to item group"
5. Ensure display order is correct
6. Now add a new item to the record and scroll down to the dropdown underneath "+ Add to item group"
7. Display order should be correct.

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 Victor Grousset/tuxayo 2024-02-05 13:00:53 UTC
Created attachment 161760 [details] [review]
Bug 34234: Respect display_order in additem.tt and detail.tt

1. APPLY PATCH
2. EnableItemGroups
3. Find a record and add some new item groups with display orders that are different from the order in which the groups were added
4. Check the checkbox next to one or more items and click the link to "Add/move to item group"
5. Ensure display order is correct
6. Now add a new item to the record and scroll down to the dropdown underneath "+ Add to item group"
7. Display order should be correct.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 18 Victor Grousset/tuxayo 2024-02-05 13:02:53 UTC
Works, makes sense, QA script happy, code looks good, passing QA :)

For some reason the add item form was already respecting the order (was not luck, changing the order was reflected). Still good to have this set in stone.
Comment 19 Katrin Fischer 2024-03-07 15:37:46 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 20 Fridolin Somers 2024-03-11 10:11:17 UTC
Maybe we should move this order by inside Koha::Biblio::item_groups
Comment 21 Fridolin Somers 2024-03-11 10:11:38 UTC
Pushed to 23.11.x for 23.11.04
Comment 22 Lucas Gass (lukeg) 2024-03-19 19:47:19 UTC
Backported to 23.05.x for upcoming 23.05.10