Bug 33639

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!

Filed Bug 33640

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() ) {

(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?"

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)

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

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>

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.

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.

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.

*** Bug 33640 has been marked as a duplicate of this bug. ***

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>

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.

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?

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>

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

(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?

Pushed for 24.05!

Well done everyone, thank you!

(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.

For consistency you're right, thanks for spotting it.

(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?

Let me know when the follow-up is ready.

(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".

(In reply to Katrin Fischer from comment #22)
> Let me know when the follow-up is ready.

35774 is ready for testing.

Pushed to 23.11.x for 23.11.02

Backported to 23.05.x for upcoming 23.05.09