Bug 33639 - Adding item to item group from 'Add item' screen doesn't work
Summary: Adding item to item group from 'Add item' screen doesn't work
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 10 votes (vote)
Assignee: Lucas Gass
QA Contact: Martin Renvoize
URL:
Keywords:
: 33640 (view as bug list)
Depends on: 24857
Blocks: 35695 35774 36498
  Show dependency treegraph
 
Reported: 2023-04-28 20:41 UTC by Sara Brown
Modified: 2024-04-02 22:20 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes adding a new item to an item group (when using the item groups feature - EnableItemGroups system preference). before this fix, even if you selected an item group, it was not added to it.
Version(s) released in:
24.05.00,23.11.02,23.05.09


Attachments
Bug 33639: Fix selectors for adding to item group (1.65 KB, patch)
2023-04-28 23:49 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33639: Fix selectors for adding to item group (1.54 KB, patch)
2023-05-17 22:27 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33639: Fix selectors for adding to item group (1.60 KB, patch)
2023-05-18 15:59 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33639: Make add item to item group work from additem.pl (3.47 KB, patch)
2024-01-02 17:53 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33639: Make add item to item group work from additem.pl (3.51 KB, patch)
2024-01-02 21:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 33639: Make add item to item group work from additem.pl (4.32 KB, patch)
2024-01-11 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sara Brown 2023-04-28 20:41:42 UTC
When creating a new item in 22.11.02, the 'Add to item group' feature at the bottom of the 'Add item' screen doesn't successfully add that new item to a group. To replicate:

-Create one or more item groups for a bib if needed
-Create a new item, and at the bottom of the 'Add item' screen, select the item group to which you want this item added. Save.
-On the Normal tab, see that the item has not been added to the item group you selected.
Comment 1 Lucas Gass 2023-04-28 23:49:44 UTC
Created attachment 150422 [details] [review]
Bug 33639: Fix selectors for adding to item group

To test:
Apply patch
1 - Browse to details page for a record
2 - Create or ensure the record has item group(s)
3 - Click New->New item
4 - Note the bottom of the page has a form to attach to existing group
5 - Note when a group is selected the enumchron field is populated
6 - Confirm item is saved to group when saved

Note: this does not fix the 'Create new item group' feature in the dropdown. I am having trouble seeing why that would work in the first in place!
Comment 2 Lucas Gass 2023-04-28 23:53:41 UTC
Filed Bug 33640
Comment 3 Katrin Fischer 2023-05-01 13:00:36 UTC
This fixes the problem, but it creates another:

* When enumchron was already filled out, selecting an item group now will overwrite the entered value and replace it.

I feel like the initial intention here was to avoid this and I think that would be correct:

 if ( ! $('input.items-enumchron').val() ) {

I think if we keep the check, but change it to checking for the name with . not -, it might actually work?


 if ( ! $('input[name="items.enumchron"]').val() ) {
Comment 4 Lucas Gass 2023-05-03 21:50:18 UTC
(In reply to Katrin Fischer from comment #3)
> This fixes the problem, but it creates another:
> 
> * When enumchron was already filled out, selecting an item group now will
> overwrite the entered value and replace it.
> 
> I feel like the initial intention here was to avoid this and I think that
> would be correct:
> 
>  if ( ! $('input.items-enumchron').val() ) {
> 
> I think if we keep the check, but change it to checking for the name with .
> not -, it might actually work?
> 
> 
>  if ( ! $('input[name="items.enumchron"]').val() ) {

That makes sense. Should we also warn the user that enumchron value is not changing? 

If I clicked the dropdown and suspected the value was going to change but it does not I might be confused. 

So maybe we need a pop-up? "You are about to to change this value from A to B. Would you like to continue?"
Comment 5 Katrin Fischer 2023-05-03 22:20:22 UTC
Actually I was quite surprised about the forced connection to enumchron. 

But then I realized that I always grouped the items from the catalog detail page. If you do that, enumchron is not altered/updated neither when empty nor when set.

So we have some inconsistency there.

We do display the item group in the staff interface items table, but we don't display it in the OPAC. Enumchron shows in a lot of places, soo I understand the idea. At the same time it feels not quite right yet.

The initial intention was to not overwrite existing data, I think this is a good default. I am not sure if a dialog could get annoying, ideally maybe a checkbox setting to turn it off, that is remembered? But we should try to limit the changes here to the necessary.

I think maybe we should stick to 'don't overwrite if already set' and work on a more sophisticated solution separately (keeping backporting in mind)
Comment 6 Lucas Gass 2023-05-17 22:27:57 UTC
Created attachment 151383 [details] [review]
Bug 33639: Fix selectors for adding to item group

To test:
Apply patch
1 - Browse to details page for a record
2 - Create or ensure the record has item group(s)
3 - Click New->New item
4 - Note the bottom of the page has a form to attach to existing group
5 - Note when a group is selected the enumchron field is populated if it is blank
6 - Note that the enumchron field is NOT update is there is a an existing value
7 - Confirm item is saved to group when saved
Comment 7 ByWater Sandboxes 2023-05-18 15:59:24 UTC
Created attachment 151428 [details] [review]
Bug 33639: Fix selectors for adding to item group

To test:
Apply patch
1 - Browse to details page for a record
2 - Create or ensure the record has item group(s)
3 - Click New->New item
4 - Note the bottom of the page has a form to attach to existing group
5 - Note when a group is selected the enumchron field is populated if it is blank
6 - Note that the enumchron field is NOT update is there is a an existing value
7 - Confirm item is saved to group when saved

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 8 Sam Lau 2023-05-18 18:59:36 UTC
When adding a new item, a group is already preselected in the dropdown box, which makes the enumchron box empty by default, even though a group is selected. Had to deselect the group and then reselect it in order for the enumchron to be filled.

Another issue was that when the new item was added, the selected group did not show up in the group column, instead, the group column was left empty.
Comment 9 Miranda Nero 2023-09-22 15:57:42 UTC
Adding a comment to revive this bug! I think this was opened during our inital Koha training at Ocean State Libraries. We are still very interested in being able to add an item to an existing item group when creating said item. We do not want to be able to create a new item group at this step. We do not use the enumchron field to store data, so affecting what is in that field is not an issue for us.
Comment 10 Lucas Gass 2024-01-02 17:53:06 UTC
Created attachment 160418 [details] [review]
Bug 33639: Make add item to item group work from additem.pl

To test:

1, Create one or more item groups for a bib if needed
2. Create a new item, and at the bottom of the 'Add item' screen, select the item group to which you want this item added. Save.
3. On the Normal tab, see that the item has not been added to the item group you selected.
4. APPLY PATCH, restart_all
5. Try 1-3 again but this time the item should succesffuly be added to the item group.
6. Add a new item and to the same record and this time try the 'Create a new item group' from the dropdown.
7. Name the new item group.
8. Make sure it is correctly added to the item group.
Comment 11 Lucas Gass 2024-01-02 18:24:16 UTC
*** Bug 33640 has been marked as a duplicate of this bug. ***
Comment 12 David Nind 2024-01-02 21:01:08 UTC
Created attachment 160441 [details] [review]
Bug 33639: Make add item to item group work from additem.pl

To test:

1, Create one or more item groups for a bib if needed
2. Create a new item, and at the bottom of the 'Add item' screen, select the item group to which you want this item added. Save.
3. On the Normal tab, see that the item has not been added to the item group you selected.
4. APPLY PATCH, restart_all
5. Try 1-3 again but this time the item should succesffuly be added to the item group.
6. Add a new item and to the same record and this time try the 'Create a new item group' from the dropdown.
7. Name the new item group.
8. Make sure it is correctly added to the item group.

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2024-01-02 21:11:26 UTC
Testing notes (using KTD):

1. Enable the EnableItemGroups system preference.

2. enumchron issue noted in previous comments: after selecting New item, I added a value for serial enumeration, a call number, and a barcode. Under the "Add item group" it defaults to "Do not add to item group" - selecting a group does not change the value already entered in h - Serial enumeration / chronology. So I'm assuming that this is no longer an issue.
Comment 14 Jonathan Druart 2024-01-11 11:19:35 UTC
446                     add_item_to_item_group( $item->biblionumber, $item->biblioitemnumber, $item_group, $item_group_description );

Should it be $item->itemnumber instead of biblioitemnumber here as well?
Comment 15 Martin Renvoize 2024-01-11 11:32:44 UTC
Created attachment 160839 [details] [review]
Bug 33639: Make add item to item group work from additem.pl

To test:

1, Create one or more item groups for a bib if needed
2. Create a new item, and at the bottom of the 'Add item' screen, select the item group to which you want this item added. Save.
3. On the Normal tab, see that the item has not been added to the item group you selected.
4. APPLY PATCH, restart_all
5. Try 1-3 again but this time the item should succesffuly be added to the item group.
6. Add a new item and to the same record and this time try the 'Create a new item group' from the dropdown.
7. Name the new item group.
8. Make sure it is correctly added to the item group.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2024-01-11 11:33:47 UTC
Trivial QA, this patch is fixing issues that came in at the last moment when the feature was renamed from 'volumes' to 'item groups'.

All working as expected now, no QA warnings.. Passing QA
Comment 17 Jonathan Druart 2024-01-11 12:07:56 UTC
(In reply to Martin Renvoize from comment #16)
> Trivial QA, this patch is fixing issues that came in at the last moment when
> the feature was renamed from 'volumes' to 'item groups'.
> 
> All working as expected now, no QA warnings.. Passing QA

What about comment 14?
Comment 18 Katrin Fischer 2024-01-11 14:49:50 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 19 Lucas Gass 2024-01-11 15:27:10 UTC
(In reply to Jonathan Druart from comment #14)
> 446                     add_item_to_item_group( $item->biblionumber,
> $item->biblioitemnumber, $item_group, $item_group_description );
> 
> Should it be $item->itemnumber instead of biblioitemnumber here as well?

Even though this works as is, I think you're right. I will open a follow-up.
Comment 20 Martin Renvoize 2024-01-11 16:48:08 UTC
For consistency you're right, thanks for spotting it.
Comment 21 Jonathan Druart 2024-01-12 07:47:41 UTC
(In reply to Martin Renvoize from comment #20)
> For consistency you're right, thanks for spotting it.

Consistency only? Then why do we need this parameter if we can pass a wrong value?
Comment 22 Katrin Fischer 2024-01-12 11:59:24 UTC
Let me know when the follow-up is ready.
Comment 23 Lucas Gass 2024-01-12 18:04:10 UTC
(In reply to Jonathan Druart from comment #21)
> (In reply to Martin Renvoize from comment #20)
> > For consistency you're right, thanks for spotting it.
> 
> Consistency only? Then why do we need this parameter if we can pass a wrong
> value?

We do need to pass the itemnumber for item_group_items.item_id. Its a FK for the items table. Without it you'd get "Broken FK constraint".
Comment 24 Lucas Gass 2024-01-12 18:10:46 UTC
(In reply to Katrin Fischer from comment #22)
> Let me know when the follow-up is ready.

35774 is ready for testing.
Comment 25 Fridolin Somers 2024-01-15 14:21:21 UTC
Pushed to 23.11.x for 23.11.02
Comment 26 Lucas Gass 2024-02-02 16:10:05 UTC
Backported to 23.05.x for upcoming 23.05.09