Bug 31080 - Block adding the bundle item to its own bundle
Summary: Block adding the bundle item to its own bundle
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 28854
Blocks: 32802
  Show dependency treegraph
 
Reported: 2022-07-01 13:44 UTC by Katrin Fischer
Modified: 2023-10-14 12:32 UTC (History)
5 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:
22.11.00


Attachments
Bug 31080: Prevent bundles from being nested (4.91 KB, patch)
2022-07-01 14:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31080: Unit tests (1.94 KB, patch)
2022-07-04 10:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31080: Prevent bundles from being nested (5.00 KB, patch)
2022-07-04 10:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31080: Unit tests (1.94 KB, patch)
2022-09-23 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31080: Prevent bundles from being nested (4.99 KB, patch)
2022-09-23 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31080: Unit tests (2.00 KB, patch)
2022-11-17 10:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31080: Prevent bundles from being nested (5.04 KB, patch)
2022-11-17 10:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31080: Unit tests (2.09 KB, patch)
2022-11-18 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31080: Prevent bundles from being nested (5.14 KB, patch)
2022-11-18 10:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-07-01 13:44:24 UTC
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.
Comment 1 Martin Renvoize 2022-07-01 14:27:03 UTC
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.
Comment 2 Katrin Fischer 2022-07-02 21:33:43 UTC
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?
Comment 3 Martin Renvoize 2022-07-04 10:23:20 UTC
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.
Comment 4 Martin Renvoize 2022-07-04 10:23:24 UTC
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.
Comment 5 Martin Renvoize 2022-07-04 10:23:50 UTC
Unit tests added and corrections made to adjust for feedback.
Comment 6 Martin Renvoize 2022-09-23 13:46:05 UTC
Looks like this got forgotten about...
Comment 7 Martin Renvoize 2022-09-23 13:46:44 UTC
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.
Comment 8 Martin Renvoize 2022-09-23 13:46:47 UTC
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.
Comment 9 Martin Renvoize 2022-09-23 13:47:18 UTC
Rebased for good measure.. be nice to see this follow-up get in within this cycle.
Comment 10 Katrin Fischer 2022-11-17 10:55:29 UTC
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>
Comment 11 Katrin Fischer 2022-11-17 10:55:34 UTC
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>
Comment 12 Martin Renvoize 2022-11-17 15:39:26 UTC
Shame this didn't make it in time for 22.11
Comment 13 Marcel de Rooy 2022-11-18 09:49:19 UTC
(In reply to Martin Renvoize from comment #12)
> Shame this didn't make it in time for 22.11

It contains strings. Sorry ;)
Comment 14 Katrin Fischer 2022-11-18 09:53:36 UTC
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.
Comment 15 Marcel de Rooy 2022-11-18 10:25:47 UTC
        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..
Comment 16 Marcel de Rooy 2022-11-18 10:26:59 UTC
Dont forget the template btw:

+                      } else if ( data.status === 400 ) {
Comment 17 Marcel de Rooy 2022-11-18 10:29:49 UTC
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>
Comment 18 Marcel de Rooy 2022-11-18 10:29:52 UTC
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>
Comment 19 Marcel de Rooy 2022-11-18 10:30:38 UTC
(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 :)
Comment 20 Tomás Cohen Arazi 2022-11-22 19:40:10 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!