From c7643bbea495d524e6ccdfd7aaf88c01decb3ba8 Mon Sep 17 00:00:00 2001 From: Olli Kautonen Date: Thu, 24 Jul 2025 11:34:55 +0300 Subject: [PATCH] Bug 40154: prevent deletion of items with holds This patch prevents deletion of items that have item level holds on them. Test plan: 1. Create an item level hold. 2. Delete the item using alternative A or B: A. Open the biblios Detail-view -> From the toolbar, open "Edit"-dropdown menu -> Manage items -> Delete the item that has an item level hold attached to it from "Actions"-dropdown menu B. Open the biblios Detail-view -> Select the item that has an item level hold attached to it -> Click "Delete selected items" -> Confirm deletion 3. Notice that the item is deleted without any warning and the item level hold has disappeared. 4. Apply this patch. 5. Repeat steps 1-3. 6. Notice that the item cannot be deleted and a warning is shown. 7. prove t/db_dependent/Koha/Item.t 8. Run the REST API tests: prove t/db_dependent/api/v1/items.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Owen Leonard --- Koha/Item.pm | 5 ++++ Koha/REST/V1/Items.pm | 1 + api/v1/swagger/paths/items.yaml | 1 + .../batch_item_record_deletion.inc | 2 ++ .../prog/en/includes/html_helpers.inc | 2 ++ .../prog/en/modules/cataloguing/additem.tt | 1 + t/db_dependent/Koha/Item.t | 27 +++++++++++++++++-- t/db_dependent/api/v1/items.t | 3 ++- 8 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 75877478032..f3ea975f0db 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -345,6 +345,8 @@ returns 1 if the item is safe to delete, "book_reserved" if the there are holds aganst the item, or +"item_has_holds" if the item has holds, + "linked_analytics" if the item has linked analytic records. "last_item_for_hold" if the item is the last one on a record on which a biblio-level hold is placed @@ -367,6 +369,9 @@ sub safe_to_delete { $error //= "book_reserved" if $self->holds->filter_by_found->count; + $error //= "item_has_holds" + if $self->holds->count; + $error //= "linked_analytics" if C4::Items::GetAnalyticsCount( $self->itemnumber ) > 0; diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm index de505299dec..87fe3e58094 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -140,6 +140,7 @@ sub delete { { code => 'linked_analytics', description => 'The item has linked analytic records' }, not_same_branch => { code => 'not_same_branch', description => 'The item is blocked by independent branches' }, + item_has_holds => { code => 'item_has_holds', description => 'The item has item level holds' }, }; if ( any { $error->message eq $_ } keys %{$errors} ) { diff --git a/api/v1/swagger/paths/items.yaml b/api/v1/swagger/paths/items.yaml index 5acc18ad283..e99a6e0e55f 100644 --- a/api/v1/swagger/paths/items.yaml +++ b/api/v1/swagger/paths/items.yaml @@ -158,6 +158,7 @@ * last_item_for_hold: The item is the last one on a record on which a biblio-level hold is placed * linked_analytics: The item has linked analytic records * not_same_branch: The item is blocked by independent branches + * item_has_holds: The item has holds schema: $ref: "../swagger.yaml#/definitions/error" "500": diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/batch_item_record_deletion.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/batch_item_record_deletion.inc index d549de90b23..5dbd3460b01 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/batch_item_record_deletion.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/background_jobs/batch_item_record_deletion.inc @@ -49,6 +49,8 @@ Item does not belong to your library [% CASE "book_reserved" %] Item has a waiting hold + [% CASE "item_has_holds" %] + Item has item level holds [% CASE "linked_analytics" %] Item has linked analytics [% CASE "last_item_for_hold" %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc index 214b29cf401..68e65eec42a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -367,6 +367,8 @@ [% SET cannot_delete_reason = t("Item does not belong to your library") %] [% CASE "book_reserved" %] [% SET cannot_delete_reason = t("Item has a waiting hold") %] + [% CASE "item_has_holds" %] + [% SET cannot_delete_reason = t("Item has item level holds") %] [% CASE "linked_analytics" %] [% SET cannot_delete_reason = t("Item has linked analytics") %] [% CASE "last_item_for_hold" %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt index 8ee5d0e9994..f59fdc81915 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt @@ -95,6 +95,7 @@ [% IF ( no_next_barcode ) %]
Error saving items: Unable to automatically determine values for barcodes. No item has been inserted.
[% END %] [% IF ( book_on_loan ) %]
Cannot delete: item is checked out.
[% END %] [% IF ( book_reserved ) %]
Cannot delete: item has a waiting hold.
[% END %] + [% IF ( item_has_holds ) %]
Cannot delete: item has item level holds.
[% END %] [% IF ( not_same_branch ) %]
Cannot delete: The items do not belong to your library.
[% END %] [% IF ( linked_analytics ) %]
Cannot delete: item has linked analytics..
diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 907aa7b60f8..a5ff47f215a 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -1264,7 +1264,7 @@ subtest 'store check barcodes' => sub { }; subtest 'deletion' => sub { - plan tests => 16; + plan tests => 18; $schema->storage->txn_begin; @@ -1345,6 +1345,29 @@ subtest 'deletion' => sub { 'IndependentBranches prevents deletion at another branch', ); + # item_has_holds + my $item_level_hold = $builder->build_object( + { + class => 'Koha::Holds', + value => { + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + found => undef, + } + } + ); + + $item->discard_changes; + my $safe_to_delete = $item->safe_to_delete; + ok( !$safe_to_delete, 'Cannot delete item with item level holds' ); + is( + @{ $safe_to_delete->messages }[0]->message, + 'item_has_holds', + 'Koha::Item->safe_to_delete reports item has item level holds', + ); + + $item_level_hold->delete; + # linked_analytics { # codeblock to limit scope of $module->mock @@ -2852,7 +2875,7 @@ subtest 'store() tests' => sub { { borrowernumber => $patron->id, date => '1970-01-01 14:00:01', - amountoutstanding => 0, + amountoutstanding => 0, amount => -5, interface => 'commandline', credit_type_code => 'PAYMENT' diff --git a/t/db_dependent/api/v1/items.t b/t/db_dependent/api/v1/items.t index 5656c1a00be..9e7558ea773 100755 --- a/t/db_dependent/api/v1/items.t +++ b/t/db_dependent/api/v1/items.t @@ -357,7 +357,7 @@ subtest 'get() tests' => sub { subtest 'delete() tests' => sub { - plan tests => 23; + plan tests => 26; $schema->storage->txn_begin; @@ -398,6 +398,7 @@ subtest 'delete() tests' => sub { }, linked_analytics => { code => 'linked_analytics', description => 'The item has linked analytic records' }, not_same_branch => { code => 'not_same_branch', description => 'The item is blocked by independent branches' }, + item_has_holds => { code => 'item_has_holds', description => 'The item has item level holds' }, }; $fail = 1; -- 2.39.5