@@ -, +, @@
Koha::Result::Boolean
---
C4/ImportBatch.pm | 3 +-
Koha/Acquisition/Order.pm | 4 +--
Koha/BackgroundJob/BatchDeleteBiblio.pm | 6 ++--
Koha/BackgroundJob/BatchDeleteItem.pm | 2 +-
Koha/Item.pm | 21 +++++++++-----
cataloguing/additem.pl | 13 ++++-----
.../prog/en/includes/html_helpers.inc | 17 ++++++-----
misc/cronjobs/delete_items.pl | 4 +--
misc/cronjobs/delete_records_via_leader.pl | 14 +++++----
t/db_dependent/Circulation/IsItemIssued.t | 7 ++---
t/db_dependent/Koha/Item.t | 29 ++++++++++++-------
11 files changed, 68 insertions(+), 52 deletions(-)
--- a/C4/ImportBatch.pm
+++ a/C4/ImportBatch.pm
@@ -937,8 +937,7 @@ sub BatchRevertItems {
$sth->execute();
while (my $row = $sth->fetchrow_hashref()) {
my $item = Koha::Items->find($row->{itemnumber});
- my $error = $item->safe_delete;
- if ($error eq '1'){
+ if ($item->safe_delete){
my $updsth = $dbh->prepare("UPDATE import_items SET status = ? WHERE import_items_id = ?");
$updsth->bind_param(1, 'reverted');
$updsth->bind_param(2, $row->{'import_items_id'});
--- a/Koha/Acquisition/Order.pm
+++ a/Koha/Acquisition/Order.pm
@@ -127,11 +127,11 @@ sub cancel {
my $items = $self->items;
while ( my $item = $items->next ) {
my $deleted = $item->safe_delete;
- unless ( ref($deleted) eq 'Koha::Item' ) {
+ unless ( $deleted ) {
$self->add_message(
{
message => 'error_delitem',
- payload => { item => $item, reason => $deleted }
+ payload => { item => $item, reason => @{$deleted->messages}[0]->messages }
}
);
}
--- a/Koha/BackgroundJob/BatchDeleteBiblio.pm
+++ a/Koha/BackgroundJob/BatchDeleteBiblio.pm
@@ -112,14 +112,14 @@ sub process {
# Delete items
my $items = Koha::Items->search({ biblionumber => $biblionumber });
while ( my $item = $items->next ) {
- my $error = $item->safe_delete;
- if(ref($error) ne 'Koha::Item'){
+ my $deleted = $item->safe_delete;
+ if( $deleted ) {
push @messages, {
type => 'error',
code => 'item_not_deleted',
biblionumber => $biblionumber,
itemnumber => $item->itemnumber,
- error => $error,
+ error => @{$deleted->messages}[0]->messages,
};
$schema->storage->txn_rollback;
$job->progress( ++$job_progress )->store;
--- a/Koha/BackgroundJob/BatchDeleteItem.pm
+++ a/Koha/BackgroundJob/BatchDeleteItem.pm
@@ -114,7 +114,7 @@ sub process {
my $item = Koha::Items->find($record_id) || next;
my $return = $item->safe_delete;
- unless ( ref($return) ) {
+ unless ( $return ) {
# FIXME Do we need to rollback the whole transaction if a deletion failed?
push @not_deleted_itemnumbers, $item->itemnumber;
--- a/Koha/Item.pm
+++ a/Koha/Item.pm
@@ -45,6 +45,7 @@ use Koha::Libraries;
use Koha::StockRotationItem;
use Koha::StockRotationRotas;
use Koha::TrackedLinks;
+use Koha::Result::Boolean;
use base qw(Koha::Object);
@@ -248,7 +249,7 @@ sub safe_delete {
my $params = @_ ? shift : {};
my $safe_to_delete = $self->safe_to_delete;
- return $safe_to_delete unless $safe_to_delete eq '1';
+ return $safe_to_delete unless $safe_to_delete;
$self->move_to_deleted;
@@ -274,22 +275,24 @@ returns 1 if the item is safe to delete,
sub safe_to_delete {
my ($self) = @_;
- return "book_on_loan" if $self->checkout;
+ my $error;
- return "not_same_branch"
+ $error = "book_on_loan" if $self->checkout;
+
+ $error = "not_same_branch"
if defined C4::Context->userenv
and !C4::Context->IsSuperLibrarian()
and C4::Context->preference("IndependentBranches")
and ( C4::Context->userenv->{branch} ne $self->homebranch );
# check it doesn't have a waiting reserve
- return "book_reserved"
+ $error = "book_reserved"
if $self->holds->search( { found => [ 'W', 'T' ] } )->count;
- return "linked_analytics"
+ $error = "linked_analytics"
if C4::Items::GetAnalyticsCount( $self->itemnumber ) > 0;
- return "last_item_for_hold"
+ $error = "last_item_for_hold"
if $self->biblio->items->count == 1
&& $self->biblio->holds->search(
{
@@ -297,7 +300,11 @@ sub safe_to_delete {
}
)->count;
- return 1;
+ if ( $error ) {
+ return Koha::Result::Boolean->new(0)->add_message({ message => $error });
+ }
+
+ return Koha::Result::Boolean->new(1);
}
=head3 move_to_deleted
--- a/cataloguing/additem.pl
+++ a/cataloguing/additem.pl
@@ -376,11 +376,11 @@ if ($op eq "additem") {
#-------------------------------------------------------------------------------
# check that there is no issue on this item before deletion.
my $item = Koha::Items->find($itemnumber);
- my $error = $item->safe_delete;
- if(ref($error) eq 'Koha::Item'){
+ my $deleted = $item->safe_delete;
+ if($deleted) {
print $input->redirect("additem.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid");
- }else{
- push @errors,$error;
+ } else {
+ push @errors, @{$deleted->messages}[0]->message;
$nextop="additem";
}
#-------------------------------------------------------------------------------
@@ -388,9 +388,8 @@ if ($op eq "additem") {
#-------------------------------------------------------------------------------
my $items = Koha::Items->search({ biblionumber => $biblionumber });
while ( my $item = $items->next ) {
- my $error = $item->safe_delete({ skip_record_index => 1 });
- next if ref $error eq 'Koha::Item'; # Deleted item is returned if deletion successful
- push @errors,$error;
+ my $deleted = $item->safe_delete({ skip_record_index => 1 });
+ push @errors, @{$deleted->messages}[0]->message;
}
my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX });
$indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" );
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc
+++ a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc
@@ -264,13 +264,16 @@
[% IF item.safe_to_delete == 1 %]
[% ELSE %]
- [% SWITCH item.safe_to_delete%]
- [% CASE "book_on_loan" %][% SET cannot_delete_reason = t("Item is checked out") %]
- [% CASE "not_same_branch" %][% 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 "linked_analytics" %][% SET cannot_delete_reason = t("Item has linked analytics") %]
- [% CASE "last_item_for_hold" %][% SET cannot_delete_reason = t("Last item for bibliographic record with biblio-level hold on it") %]
- [% CASE %][% SET cannot_delete_reason = t("Unknown reason") _ '(' _ item.safe_to_delete _ ')' %]
+ [% SET messages = item.safe_to_delete.messages %]
+ [% FOR m IN messages %]
+ [% SWITCH m %]
+ [% CASE "book_on_loan" %][% SET cannot_delete_reason = t("Item is checked out") %]
+ [% CASE "not_same_branch" %][% 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 "linked_analytics" %][% SET cannot_delete_reason = t("Item has linked analytics") %]
+ [% CASE "last_item_for_hold" %][% SET cannot_delete_reason = t("Last item for bibliographic record with biblio-level hold on it") %]
+ [% CASE %][% SET cannot_delete_reason = t("Unknown reason") _ '(' _ item.safe_to_delete _ ')' %]
+ [% END %]
[% END %]
--- a/misc/cronjobs/delete_items.pl
+++ a/misc/cronjobs/delete_items.pl
@@ -78,12 +78,12 @@ DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() )
my $item_object = Koha::Items->find($item->{itemnumber});
my $safe_to_delete = $item_object->safe_to_delete;
- if( $safe_to_delete eq '1' ) {
+ if( $safe_to_delete ) {
$item_object->safe_delete
if $OPTIONS->{flags}->{commit};
verbose "Deleting '$item->{itemnumber}'";
} else {
- verbose "Item '$item->{itemnumber}' not deleted: $safe_to_delete";
+ verbose sprintf "Item '%s' not deleted: %s", $item->{itemnumber}, @{$safe_to_delete->messages}[0]->message
}
}
--- a/misc/cronjobs/delete_records_via_leader.pl
+++ a/misc/cronjobs/delete_records_via_leader.pl
@@ -91,19 +91,21 @@ foreach my $m (@metadatas) {
my $itemnumber = $item->itemnumber;
if( $test ){
- my $result = $item->safe_to_delete;
- if ( $result eq "1") {
+ my $deleted = $item->safe_to_delete;
+ if ( $deleted ) {
say "TEST MODE: Item $itemnumber would have been deleted";
} else {
- say "TEST MODE: ERROR DELETING ITEM $itemnumber: $result";
+ my $error = @{$deleted->messages}[0]->message
+ say "TEST MODE: ERROR DELETING ITEM $itemnumber: $error";
}
} else {
- my $result = $item->safe_delete;
- if ( ref $result eq "Koha::Item" ){
+ my $deleted = $item->safe_to_delete;
+ if ( $deleted ) {
say "DELETED ITEM $itemnumber" if $verbose;
$deleted_items_count++;
} else {
- say "ERROR DELETING ITEM $itemnumber: $result";
+ my $error = @{$deleted->messages}[0]->message
+ say "ERROR DELETING ITEM $itemnumber: $error";
}
}
$total_items_count++;
--- a/t/db_dependent/Circulation/IsItemIssued.t
+++ a/t/db_dependent/Circulation/IsItemIssued.t
@@ -67,7 +67,7 @@ AddIssue($borrower, $item->barcode);
is ( IsItemIssued( $item->itemnumber ), 1, "item is now on loan" );
is(
- $item->safe_delete,
+ @{$item->safe_delete->messages}[0]->message,
'book_on_loan',
'item that is on loan cannot be deleted',
);
@@ -77,9 +77,8 @@ is ( IsItemIssued( $item->itemnumber ), 0, "item has been returned" );
$item->discard_changes; # FIXME We should not need that
# If we do not, $self->checkout in safe_to_delete will not know the item has been checked out
-is(
- ref($item->safe_delete),
- 'Koha::Item',
+ok(
+ $item->safe_delete,
'item that is not on loan can be deleted',
);
--- a/t/db_dependent/Koha/Item.t
+++ a/t/db_dependent/Koha/Item.t
@@ -571,7 +571,7 @@ subtest 'request_transfer' => sub {
};
subtest 'deletion' => sub {
- plan tests => 12;
+ plan tests => 13;
$schema->storage->txn_begin;
@@ -606,17 +606,22 @@ subtest 'deletion' => sub {
C4::Circulation::AddIssue( $patron->unblessed, $item->barcode );
is(
- $item->safe_to_delete,
+ @{$item->safe_to_delete->messages}[0]->message,
'book_on_loan',
'Koha::Item->safe_to_delete reports item on loan',
);
is(
- $item->safe_delete,
+ @{$item->safe_to_delete->messages}[0]->message,
'book_on_loan',
'item that is on loan cannot be deleted',
);
+ ok(
+ ! $item->safe_to_delete,
+ 'Koha::Item->safe_to_delete shows item NOT safe to delete'
+ );
+
AddReturn( $item->barcode, $library->branchcode );
# book_reserved is tested in t/db_dependent/Reserves.t
@@ -626,13 +631,13 @@ subtest 'deletion' => sub {
my $item_2 = $builder->build_sample_item({ library => $library_2->branchcode });
is(
- $item_2->safe_to_delete,
+ @{$item_2->safe_to_delete->messages}[0]->message,
'not_same_branch',
'Koha::Item->safe_to_delete reports IndependentBranches restriction',
);
is(
- $item_2->safe_delete,
+ @{$item_2->safe_to_delete->messages}[0]->message,
'not_same_branch',
'IndependentBranches prevents deletion at another branch',
);
@@ -646,13 +651,13 @@ subtest 'deletion' => sub {
$item->discard_changes;
is(
- $item->safe_to_delete,
+ @{$item->safe_to_delete->messages}[0]->message,
'linked_analytics',
'Koha::Item->safe_to_delete reports linked analytics',
);
is(
- $item->safe_delete,
+ @{$item->safe_to_delete->messages}[0]->message,
'linked_analytics',
'Linked analytics prevents deletion of item',
);
@@ -661,15 +666,17 @@ subtest 'deletion' => sub {
{ # last_item_for_hold
C4::Reserves::AddReserve({ branchcode => $patron->branchcode, borrowernumber => $patron->borrowernumber, biblionumber => $item->biblionumber });
- is( $item->safe_to_delete, 'last_item_for_hold', 'Item cannot be deleted if a biblio-level is placed on the biblio and there is only 1 item attached to the biblio' );
-
+ is(
+ @{$item->safe_to_delete->messages}[0]->message,
+ 'last_item_for_hold',
+ 'Item cannot be deleted if a biblio-level is placed on the biblio and there is only 1 item attached to the biblio'
+ );
# With another item attached to the biblio, the item can be deleted
$builder->build_sample_item({ biblionumber => $item->biblionumber });
}
- is(
+ ok(
$item->safe_to_delete,
- 1,
'Koha::Item->safe_to_delete shows item safe to delete'
);
--