From 14d9cabbf41c2e2a6618430925419f0f0189c0d0 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>
---
 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 363c417fd1f..f2aaa8c3cc6 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 245b9e94f33..a9340e05e67 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.28.0