Bugzilla – Attachment 185216 Details for
Bug 40154
Deleting an item does not warn about an item level hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40154: prevent deletion of items with holds
0001-Bug-40154-prevent-deletion-of-items-with-holds.patch (text/plain), 9.05 KB, created by
Johanna Räisä
on 2025-08-07 10:56:03 UTC
(
hide
)
Description:
Bug 40154: prevent deletion of items with holds
Filename:
MIME Type:
Creator:
Johanna Räisä
Created:
2025-08-07 10:56:03 UTC
Size:
9.05 KB
patch
obsolete
>From 0435c9d040c88c3d6122dd36df58bbc48930bd81 Mon Sep 17 00:00:00 2001 >From: Olli Kautonen <bhm503@myy.haaga-helia.fi> >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 >--- > 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 7587747803..f3ea975f0d 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 de505299de..87fe3e5809 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 5acc18ad28..e99a6e0e55 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 d549de90b2..5dbd3460b0 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 @@ > <span>Item does not belong to your library</span> > [% CASE "book_reserved" %] > <span>Item has a waiting hold</span> >+ [% CASE "item_has_holds" %] >+ <span>Item has item level holds</span> > [% CASE "linked_analytics" %] > <span>Item has linked analytics</span> > [% 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 214b29cf40..68e65eec42 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 8ee5d0e999..f59fdc8191 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 ) %]<div class="alert alert-warning"><strong>Error saving items</strong>: Unable to automatically determine values for barcodes. No item has been inserted.</div>[% END %] > [% IF ( book_on_loan ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: item is checked out.</div>[% END %] > [% IF ( book_reserved ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: item has a waiting hold.</div>[% END %] >+ [% IF ( item_has_holds ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: item has item level holds.</div>[% END %] > [% IF ( not_same_branch ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: The items do not belong to your library.</div>[% END %] > [% IF ( linked_analytics ) %] > <div class="alert alert-warning"><strong>Cannot delete</strong>: item has linked <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber | uri %]&analyze=1">analytics.</a>.</div> >diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t >index 907aa7b60f..a5ff47f215 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 5656c1a00b..9e7558ea77 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.34.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40154
:
185216
|
185406
|
185407