Bug 34234 - Item groups dropdown in detail page modal does not respect display order
Summary: Item groups dropdown in detail page modal does not respect display order
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on: 24857
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-07 20:47 UTC by Emily Lamancusa
Modified: 2024-02-05 13:02 UTC (History)
4 users (show)

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


Attachments
Bug 34234: Add template plugin for listing and sorting ItemGroups (3.61 KB, patch)
2023-10-31 17:26 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: Add sub for count and logic to additem page (4.14 KB, patch)
2023-11-02 22:25 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: Add template plugin for listing and sorting ItemGroups (3.65 KB, patch)
2023-11-03 11:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34234: Add sub for count and logic to additem page (4.19 KB, patch)
2023-11-03 11:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34234: add a unit test (4.47 KB, patch)
2024-01-03 00:27 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: add a unit test (4.93 KB, patch)
2024-01-04 15:37 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: add a unit test (4.87 KB, patch)
2024-01-04 16:05 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: Respect display_order in additem.tt and detail.tt (2.57 KB, patch)
2024-01-12 17:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 34234: Respect display_order in additem.tt and detail.tt (2.62 KB, patch)
2024-01-13 18:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 34234: Respect display_order in additem.tt and detail.tt (2.68 KB, patch)
2024-02-05 13:00 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emily Lamancusa 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 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 2023-10-31 17:26:50 UTC Comment hidden (obsolete)
Comment 4 Lucas Gass 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 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 2024-01-03 00:27:18 UTC
Created attachment 160442 [details] [review]
Bug 34234: add a unit test
Comment 10 Lucas Gass 2024-01-04 15:37:50 UTC
Created attachment 160542 [details] [review]
Bug 34234: add a unit test
Comment 11 Lucas Gass 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 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 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.