From 6f940b5fe90518112db612d7e19aa1419a6ef84e Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Fri, 9 Oct 2020 17:01:29 -0300
Subject: [PATCH] Bug 26515: Better feedback on errors

This patch makes the possible causes of biblio removal failure to be
specified on the passed error message. This way the UI could render
better reports on the situation.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Acquisition/Order.pm               | 22 ++++++++++++++++++----
 t/db_dependent/Koha/Acquisition/Order.t | 21 ++++++++++-----------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm
index 363c417fd1..f2aaa8c3cc 100644
--- a/Koha/Acquisition/Order.pm
+++ b/Koha/Acquisition/Order.pm
@@ -131,17 +131,31 @@ sub cancel {
         my $biblio = $self->biblio;
         if ( $biblio and $delete_biblio ) {
 
-            if (    $biblio->active_orders->count - 1 == 0 # minus ourself
+            if (
+                $biblio->active_orders->search(
+                    { ordernumber => { '!=' => $self->ordernumber } }
+                )->count == 0
                 and $biblio->subscriptions->count == 0
-                and $biblio->items->count == 0 )
+                and $biblio->items->count == 0
+              )
             {
 
                 my $error = DelBiblio( $biblio->id );
-                $self->add_message({ message => 'error_delbiblio', error => $error })
+                $self->add_message( { message => 'error_delbiblio'} )
                   if $error;
             }
             else {
-                $self->add_message({ message => 'error_delbiblio' });
+                if ( $biblio->active_orders->search(
+                    { ordernumber => { '!=' => $self->ordernumber } }
+                )->count > 0 ) {
+                    $self->add_message( { message => 'error_delbiblio_active_orders' } );
+                }
+                elsif ( $biblio->subscriptions->count > 0 ) {
+                    $self->add_message( { message => 'error_delbiblio_subscriptions' } );
+                }
+                else { # $biblio->items->count > 0
+                    $self->add_message( { message => 'error_delbiblio_items' } );
+                }
             }
         }
     }
diff --git a/t/db_dependent/Koha/Acquisition/Order.t b/t/db_dependent/Koha/Acquisition/Order.t
index 245b9e94f3..a9340e05e6 100755
--- a/t/db_dependent/Koha/Acquisition/Order.t
+++ b/t/db_dependent/Koha/Acquisition/Order.t
@@ -619,7 +619,7 @@ subtest 'cancel() tests' => sub {
                 cancellationreason      => undef,
             }
         }
-    )->reset_messages; # reset them as TestBuilder doesn't call new
+    );
     $order->add_item( $item->id );
 
     my $patron = $builder->build_object({ class => 'Koha::Patrons' });
@@ -651,7 +651,7 @@ subtest 'cancel() tests' => sub {
 
     C4::Circulation::AddReturn( $item->barcode );
 
-    $order->reset_messages;
+    $order = Koha::Acquisition::Orders->find($order->ordernumber);
     $order->cancel({ reason => $reason })
           ->discard_changes;
 
@@ -684,7 +684,7 @@ subtest 'cancel() tests' => sub {
                 cancellationreason      => undef,
             }
         }
-    )->reset_messages;
+    );
     $order->add_item( $item_1->id );
 
     $order->cancel({ reason => $reason, delete_biblio => 1 })
@@ -697,7 +697,7 @@ subtest 'cancel() tests' => sub {
     is( ref(Koha::Items->find($item_2->id)), 'Koha::Item', 'The item is still present' );
     is( ref(Koha::Biblios->find($biblio_id)), 'Koha::Biblio', 'The biblio is still present' );
     @messages = @{ $order->messages };
-    is( $messages[0]->message, 'error_delbiblio', 'Cannot delete biblio and it gets notified' );
+    is( $messages[0]->message, 'error_delbiblio_items', 'Cannot delete biblio and it gets notified' );
 
     # Scenario:
     # * order with one item attached
@@ -719,7 +719,7 @@ subtest 'cancel() tests' => sub {
                 cancellationreason      => undef,
             }
         }
-    )->reset_messages;
+    );
     $order->add_item( $item->id );
 
     # Add another order
@@ -744,7 +744,7 @@ subtest 'cancel() tests' => sub {
     is( Koha::Items->find($item->id), undef, 'The item is no longer present' );
     is( ref(Koha::Biblios->find($biblio_id)), 'Koha::Biblio', 'The biblio is still present' );
     @messages = @{ $order->messages };
-    is( $messages[0]->message, 'error_delbiblio', 'Cannot delete biblio and it gets notified' );
+    is( $messages[0]->message, 'error_delbiblio_active_orders', 'Cannot delete biblio and it gets notified' );
 
     # Scenario:
     # * order with one item attached
@@ -766,7 +766,7 @@ subtest 'cancel() tests' => sub {
                 cancellationreason      => undef,
             }
         }
-    )->reset_messages;
+    );
     $order->add_item( $item->id );
 
     # Add a subscription
@@ -788,7 +788,7 @@ subtest 'cancel() tests' => sub {
     is( Koha::Items->find($item->id), undef, 'The item is no longer present' );
     is( ref(Koha::Biblios->find($biblio_id)), 'Koha::Biblio', 'The biblio is still present' );
     @messages = @{ $order->messages };
-    is( $messages[0]->message, 'error_delbiblio', 'Cannot delete biblio and it gets notified' );
+    is( $messages[0]->message, 'error_delbiblio_subscriptions', 'Cannot delete biblio and it gets notified' );
 
     # Scenario:
     # * order with one item attached
@@ -812,9 +812,8 @@ subtest 'cancel() tests' => sub {
     );
     $order->add_item( $item->id );
 
-    $order->cancel({ reason => $reason, delete_biblio => 1 });
-    # refresh the order object
-    $order->discard_changes;
+    $order->cancel({ reason => $reason, delete_biblio => 1 })
+          ->discard_changes;
 
     is( $order->orderstatus, 'cancelled', 'Order is not marked as cancelled' );
     isnt( $order->datecancellationprinted, undef, 'datecancellationprinted is not undef' );
-- 
2.39.5