As I worked on Bug 33639 I noticed some code in cataloging_additem.js that doesn't work/isn't needed: 137 $('#item-group-add-or-create-form-select').on('change', function() { 138 if ( ! $('input.items-enumchron').val() ) { 139 let item_group_selector = '#item-group-' + $(this).val(); 140 let enumchron = $(item_group_selector).val(); 141 $('input.items-enumchron').val( enumchron ); 142 } 143 }); The selectors are bad and it would be easy to make them work. But I don't see any reason why item groups should be updating the items.enumchron field.
Created attachment 160508 [details] [review] Bug 35695: Remove useless JS from cataloging_additem.js To test: 1. APPLY PATCH 2. Turn on EnableItemGroups 3. Go to a record with item groups, or create some. 4. Add an item, making sure items can be added to a group. ( Bug 33639 must be applied ) If I am wrong and this code should stay, tell me. I'll fix the jQuery instead of removing it. :)
Created attachment 160594 [details] [review] Bug 35695: Remove useless JS from cataloging_additem.js To test: 1. APPLY PATCH 2. Turn on EnableItemGroups 3. Go to a record with item groups, or create some. 4. Add an item, making sure items can be added to a group. ( Bug 33639 must be applied ) If I am wrong and this code should stay, tell me. I'll fix the jQuery instead of removing it. :) Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. I added and edited groups, and added items to groups (both from the normal page view, and when adding new items). 2. No differences or errors noted before and after the patch was applied.
Coming from: commit f83c7f86553641063725ff0b10d1b3d29a3506a9 Author: Nick Clemens <nick@bywatersolutions.com> Date: Tue Mar 8 12:03:38 2022 +0000 Bug 24857: Add ability to set item group when adding a new item During cataloging a user may wish to add an item to a group when creating a new item This patch also copies the group description to the enumchron field To test: 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, or create new 5 - Note when a group is selected the enumchron field is populated 6 - Confirm item is saved to group when saved Did you ask Nick?
> Did you ask Nick? I did, we feel like this is leftover cruft from a previous iteration to item groups. Currently, the JS I removed doesn't work/do anything anyways. We think it should be removed.
Created attachment 161430 [details] [review] Bug 35695: Remove useless JS from cataloging_additem.js To test: 1. APPLY PATCH 2. Turn on EnableItemGroups 3. Go to a record with item groups, or create some. 4. Add an item, making sure items can be added to a group. ( Bug 33639 must be applied ) If I am wrong and this code should stay, tell me. I'll fix the jQuery instead of removing it. :) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I agree with the decision.. Passing QA
I tested one of the iterations and I remember seeing the update, but not agreeing with it. I don't think they should be connected in that way. So I am happy to have this removed.
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.03
Backported to 23.05.x for upcoming 23.05.09