At the moment it's possible to add the bundle item barcode to the bundle. The most prominent effect is, that the bundle can no longer be checked out, as a bundled item is blocked from circulation. So it's not super bad for a bug, but we should catch it with a nice error message if someone makes a copy&paste or scan error there.
Created attachment 136975 [details] [review] Bug 31080: Prevent bundles from being nested This is a follow-up for bug 28854 to prevent bundle type items from being nested in to other bundles.
I think this doesn't quite work as supposed yet. 1) Find or create a record with bib-level:c 2) Add an item 3) Copy the barcode of the item and add it to the bundle for the same item --> It will be added without an error Adding any other bundle item works ok (not added) and gives the message added here. Are we missing a unit test?
Created attachment 137053 [details] [review] Bug 31080: Unit tests Unit tests for add_to_bundle functionality that prevents bundle nesting. Test plan 1) Run t/db_dependent/Koha/Item.t 2) The test should fail 3) Apply the second patch 4) Re-run the test and watch it pass.
Created attachment 137054 [details] [review] Bug 31080: Prevent bundles from being nested This is a follow-up for bug 28854 to prevent bundle type items from being nested in to other bundles.
Unit tests added and corrections made to adjust for feedback.
Looks like this got forgotten about...
Created attachment 140908 [details] [review] Bug 31080: Unit tests Unit tests for add_to_bundle functionality that prevents bundle nesting. Test plan 1) Run t/db_dependent/Koha/Item.t 2) The test should fail 3) Apply the second patch 4) Re-run the test and watch it pass.
Created attachment 140909 [details] [review] Bug 31080: Prevent bundles from being nested This is a follow-up for bug 28854 to prevent bundle type items from being nested in to other bundles.
Rebased for good measure.. be nice to see this follow-up get in within this cycle.
Created attachment 143991 [details] [review] Bug 31080: Unit tests Unit tests for add_to_bundle functionality that prevents bundle nesting. Test plan 1) Run t/db_dependent/Koha/Item.t 2) The test should fail 3) Apply the second patch 4) Re-run the test and watch it pass. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 143992 [details] [review] Bug 31080: Prevent bundles from being nested This is a follow-up for bug 28854 to prevent bundle type items from being nested in to other bundles. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Shame this didn't make it in time for 22.11
(In reply to Martin Renvoize from comment #12) > Shame this didn't make it in time for 22.11 It contains strings. Sorry ;)
We have had some incremental updates to the po files. Maybe we put this in Tomas queue and when something bad requires a string change, this could be pushed with it before we run another update.
if ( ref($_) eq 'Koha::Exceptions::Object::DuplicateID' ) { return $c->render( status => 409, openapi => { error => 'Item is already bundled', key => $_->duplicate_id } ); } elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::IsBundle' ) { return $c->render( status => 400, openapi => { error => 'Bundles cannot be nested' Somehow I wonder why we differentiate here between 400 and 409 ? 400 is the general failure, 409 is a conflict. The nested bundle could be a 409 too. But I would not mind having both on 400 as well. This seems inconsistent. No blocker, easily to be adjusted..
Dont forget the template btw: + } else if ( data.status === 400 ) {
Created attachment 144054 [details] [review] Bug 31080: Unit tests Unit tests for add_to_bundle functionality that prevents bundle nesting. Test plan 1) Run t/db_dependent/Koha/Item.t 2) The test should fail 3) Apply the second patch 4) Re-run the test and watch it pass. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 144055 [details] [review] Bug 31080: Prevent bundles from being nested This is a follow-up for bug 28854 to prevent bundle type items from being nested in to other bundles. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Katrin Fischer from comment #14) > We have had some incremental updates to the po files. Maybe we put this in > Tomas queue and when something bad requires a string change, this could be > pushed with it before we run another update. Tomás: Do as you please :)
Pushed to master for 22.11. Nice work everyone, thanks!