Attachment 111430 Details for Bug 26515 – Bug 26515: Better feedback on errors

[patch] Bug 26515: Better feedback on errors

Bug-26515-Better-feedback-on-errors.patch (text/plain), 5.85 KB, created by Tomás Cohen Arazi (tcohen) on 2020-10-09 20:04:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-10-09 20:04:13 UTC

Size: 5.85 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 26515: 110603 | 110604 | 110641 | 110642 | 110751 | 110752 | 110939 | 110940 | 110970 | 110971 | 110992 | 111031 | 111170 | 111172 | 111173 | 111372 | 111430 | 111724