From: Jason Stephenson <jstephenson@mvlc.org>
Date: Tue, 18 Aug 2015 13:27:54 +0000 (-0400)
Subject: Add error handling to NCIP::ILS::Evergreen->cancelrequestitem.
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=bb1d2c5c3dc374ebf49adbc1579418b1ac8c6909;p=working%2FNCIPServer.git

Add error handling to NCIP::ILS::Evergreen->cancelrequestitem.

We did not check the return value of open-ils.circ.hold.cancel, because
why would it fail?  Turns out there are a couple of reasons it could
fail, including not setting up permissions for the staff user correctly.

The cancel_hold helper function now returns the return value of the
open-ils.circ.hold.cancel method.  This method returns 1 on succes or
a reference ot an event hash on failure.

The cancelrequestitem method now returns a Problem response message
of Temporary Processing Failure if cancel_hold returns a ref.

Signed-off-by: Jason Stephenson <jstephenson@mvlc.org>
---

diff --git a/lib/NCIP/ILS/Evergreen.pm b/lib/NCIP/ILS/Evergreen.pm
index 80f0008..fe2f83b 100644
--- a/lib/NCIP/ILS/Evergreen.pm
+++ b/lib/NCIP/ILS/Evergreen.pm
@@ -1134,46 +1134,50 @@ sub cancelrequestitem {
             )
        );
     } elsif ($hold) {
-        $self->cancel_hold($hold);
-        my $data = {
-            RequestId => NCIP::RequestId->new(
-                {
-                    AgencyId => $request->{$message}->{RequestId}->{AgencyId},
-                    RequestIdentifierType => $request->{$message}->{RequestId}->{RequestIdentifierType},
-                    RequestIdentifierValue => $request->{$message}->{RequestId}->{RequestIdentifierValue}
-                }
-            ),
-            UserId => NCIP::User::Id->new(
-                {
-                    UserIdentifierType => 'Barcode Id',
-                    UserIdentifierValue => $user->card->barcode()
-                }
-            ),
-            ItemId => NCIP::Item::Id->new(
-                {
-                    AgencyId => $request->{$message}->{ItemId}->{AgencyId},
-                    ItemIdentifierType => $request->{$message}->{ItemId}->{ItemIdentifierType},
-                    ItemIdentifierValue => $request->{$message}->{ItemId}->{ItemIdentifierValue}
+        my $result = $self->cancel_hold($hold);
+        if (ref($result)) {
+            $response->problem(_problem_from_event("Temporary Processing Failure", $result));
+        } else {
+            my $data = {
+                RequestId => NCIP::RequestId->new(
+                    {
+                        AgencyId => $request->{$message}->{RequestId}->{AgencyId},
+                        RequestIdentifierType => $request->{$message}->{RequestId}->{RequestIdentifierType},
+                        RequestIdentifierValue => $request->{$message}->{RequestId}->{RequestIdentifierValue}
+                    }
+                ),
+                UserId => NCIP::User::Id->new(
+                    {
+                        UserIdentifierType => 'Barcode Id',
+                        UserIdentifierValue => $user->card->barcode()
+                    }
+                ),
+                ItemId => NCIP::Item::Id->new(
+                    {
+                        AgencyId => $request->{$message}->{ItemId}->{AgencyId},
+                        ItemIdentifierType => $request->{$message}->{ItemId}->{ItemIdentifierType},
+                        ItemIdentifierValue => $request->{$message}->{ItemId}->{ItemIdentifierValue}
+                    }
+                )
+            };
+            # Look for UserElements requested and add it to the response:
+            my $elements = $request->{$message}->{UserElementType};
+            if ($elements) {
+                $elements = [$elements] unless (ref $elements eq 'ARRAY');
+                my $optionalfields = $self->handle_user_elements($user, $elements);
+                $data->{UserOptionalFields} = $optionalfields;
+            }
+            $elements = $request->{$message}->{ItemElementType};
+            if ($elements && $hold->current_copy()) {
+                $elements = [$elements] unless (ref $elements eq 'ARRAY');
+                my $copy_details = $self->retrieve_copy_details_by_id($hold->current_copy());
+                if ($copy_details) {
+                    my $optionalfields = $self->handle_item_elements($copy_details->{copy}, $elements);
+                    $data->{ItemOptionalFields} = $optionalfields;
                 }
-            )
-        };
-        # Look for UserElements requested and add it to the response:
-        my $elements = $request->{$message}->{UserElementType};
-        if ($elements) {
-            $elements = [$elements] unless (ref $elements eq 'ARRAY');
-            my $optionalfields = $self->handle_user_elements($user, $elements);
-            $data->{UserOptionalFields} = $optionalfields;
-        }
-        $elements = $request->{$message}->{ItemElementType};
-        if ($elements && $hold->current_copy()) {
-            $elements = [$elements] unless (ref $elements eq 'ARRAY');
-            my $copy_details = $self->retrieve_copy_details_by_id($hold->current_copy());
-            if ($copy_details) {
-                my $optionalfields = $self->handle_item_elements($copy_details->{copy}, $elements);
-                $data->{ItemOptionalFields} = $optionalfields;
             }
+            $response->data($data);
         }
-        $response->data($data);
     } else {
         $response->problem(
             NCIP::Problem->new(
@@ -2356,15 +2360,15 @@ sub place_hold {
 
 =head2 cancel_hold
 
-    $ils->cancel_hold($hold);
+    $result = $ils->cancel_hold($hold);
 
 This method cancels the hold argument. It makes no checks on the hold,
 so if there are certain conditions that need to be fulfilled before
 the hold is canceled, then you must check them before calling this
 method.
 
-It returns undef on success or failure. If it fails, you've usually
-got bigger problems.
+It returns the result of the backend call to cancel the hold: 1 on
+succes or an ILS event on failure.
 
 =cut
 
@@ -2381,7 +2385,7 @@ sub cancel_hold {
         'Canceled via NCIPServer'
     );
 
-    return undef;
+    return $r;
 }
 
 =head2 delete_copy