Simplify RequestItem handling in NCIP::ILS::Evergreen.
authorJason Stephenson <jstephenson@mvlc.org>
Fri, 23 Oct 2015 19:02:58 +0000 (15:02 -0400)
committerJason Stephenson <jstephenson@mvlc.org>
Mon, 26 Oct 2015 13:17:13 +0000 (09:17 -0400)
Replace several subroutines and OpenILS calls to check for existing
copies, holds, etc., and to check if the hold is possible before
creating the hold with a single call to open-ils.circ.holds.
test_and_create.batch.  This call does everything in one step and
leads to cleaner code for us.

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

lib/NCIP/ILS/Evergreen.pm

index 11f2488..45ad829 100644 (file)
@@ -908,15 +908,6 @@ sub requestitem {
     my $response = NCIP::Response->new({type => $message . 'Response'});
     $response->header($self->make_header($request));
 
-    # We originally had a mass of complicated code here to handle most
-    # of the possibilities provided by the standard.  However, that
-    # proved too difficult to get working with what we've agreed to do
-    # with Auto-Graphics.  The response was supposed to include the
-    # ItemId, and for some reason that I couldn't figure out it was
-    # not there.  In order to get this working, I've decided to ignor
-    # the extra stuff in the standard and just go with what our
-    # current vendor sends.
-
     # Because we need to have a user to place a hold, because the user
     # is likely to have problems, and because getting the item
     # information for the hold is trickier than getting the user
@@ -997,26 +988,6 @@ sub requestitem {
         return $response;
     }
 
-    # We need to see if the bib exists and has a holdable, not deleted
-    # copy at the selection_ou.  If successful, we retun a
-    # copy_details hashref for the holdable copy.
-    my $copy_details = $self->find_target_details_by_bre($bre, $selection_ou);
-    unless ($copy_details) {
-        # We don't know if the items do not circulate or are not
-        # holdable, but the closest "standard" problem message is Item
-        # Does Not Circulate.
-        $problem = NCIP::Problem->new(
-            {
-                ProblemType => 'Item Does Not Circulate',
-                ProblemDetail => 'Request of Item cannot proceed because the Item is non-circulating',
-                ProblemElement => 'BibliographicRecordIdentifier',
-                ProblemValue => $bre->id()
-            }
-        );
-        $response->problem($problem);
-        return $response;
-    }
-
     # See if we were given a pickup location.
     my $pickup_ou;
     if ($request->{$message}->{PickupLocation}) {
@@ -1062,7 +1033,8 @@ sub requestitem {
             $data->{UserOptionalFields} = $optionalfields;
         }
         $elements = $request->{$message}->{ItemElementType};
-        if ($elements) {
+        if ($elements && $hold->current_copy()) {
+            my $copy_details = $self->retrieve_copy_details_by_id($hold->current_copy());
             $elements = [$elements] unless (ref($elements) eq 'ARRAY');
             my $optionalfields = $self->handle_item_elements($copy_details->{copy}, $elements);
             $data->{ItemOptionalFields} = $optionalfields;
@@ -2293,112 +2265,72 @@ sub place_hold {
     # have been fleshed when the user was retrieved.
     $location = $user->home_ou() unless ($location);
 
-    # $hold is the hold. $params is for the is_possible check.
-    my ($hold, $params);
-
-    # Prep the hold with fields common to all hold types:
-    $hold = Fieldmapper::action::hold_request->new();
-    $hold->isnew(1); # Just to make sure.
-    $hold->target($item->id());
-    $hold->usr($user->id());
-    $hold->pickup_lib($location->id());
-    $hold->expire_time(cleanse_ISO8601($expiration)) if ($expiration);
-    if (!$user->email()) {
-        $hold->email_notify('f');
-        $hold->phone_notify($user->day_phone()) if ($user->day_phone());
-    } else {
-        $hold->email_notify('t');
-    }
-
-    # Ditto the params:
-    $params = { pickup_lib => $location->id(), patronid => $user->id() };
+    # $params for the hold.
+    my $params = { pickup_lib => $location->id(), patronid => $user->id() };
 
     if (ref($item) eq 'Fieldmapper::asset::copy') {
         my $type = ($self->{config}->{items}->{use_force_holds}) ? 'F' : 'C';
-        $hold->hold_type($type);
-        $hold->current_copy($item->id());
         $params->{hold_type} = $type;
-        $params->{copy_id} = $item->id();
     } elsif (ref($item) eq 'Fieldmapper::asset::call_number') {
-        $hold->hold_type('V');
         $params->{hold_type} = 'V';
-        $params->{volume_id} = $item->id();
     } elsif (ref($item) eq 'Fieldmapper::biblio::record_entry') {
-        $hold->hold_type('T');
         $params->{hold_type} = 'T';
-        $params->{titleid} = $item->id();
         if ($org_unit && ref($org_unit) eq 'Fieldmapper::actor::org_unit') {
-            $hold->selection_ou($org_unit->id());
             $params->{selection_ou} = $org_unit->id();
-            if (ref($org_unit->ou_type())) {
-                $hold->selection_depth($org_unit->ou_type->depth());
-                $params->{depth} = $org_unit->ou_type->depth();
-            }
+            $params->{depth} = $org_unit->ou_type->depth() if (ref($org_unit->ou_type()));
         }
     }
 
-    # Check for a duplicate hold:
-    my $duplicate = $U->simplereq(
-        'open-ils.pcrud',
-        'open-ils.pcrud.search.ahr',
-        $self->{session}->{authtoken},
-        {
-            hold_type => $hold->hold_type(),
-            target => $hold->target(),
-            usr => $hold->usr(),
-            expire_time => {'>' => 'now'},
-            cancel_time => undef,
-            fulfillment_time => undef
-        }
-    );
-    if ($duplicate) {
-        return NCIP::Problem->new(
-            {
-                ProblemType => 'Duplicate Request',
-                ProblemDetail => 'A request for this item already exists for this patron.',
-                ProblemElement => 'NULL',
-                ProblemValue => 'NULL'
-            }
-        );
-    }
-
-    # Check if the hold is possible:
-    my $r = $U->simplereq(
+    my $response = $U->simplereq(
         'open-ils.circ',
-        'open-ils.circ.title_hold.is_possible',
+        'open-ils.circ.holds.test_and_create.batch',
         $self->{session}->{authtoken},
-        $params
+        $params,
+        [$item->id()]
     );
 
-    if ($r->{success}) {
-        $hold = $U->simplereq(
-            'open-ils.circ',
-            'open-ils.circ.holds.create.override',
-            $self->{session}->{authtoken},
-            $hold
-        );
-        if (ref($hold)) {
-            $hold = $hold->[0] if (ref($hold) eq 'ARRAY');
-            $hold = _problem_from_event('User Ineligible To Request This Item', $hold);
-        } else {
-            # open-ils.circ.holds.create.override returns the id on
-            # success, so we retrieve the full hold object from the
-            # database to return it.
-            $hold = $U->simplereq(
-                'open-ils.pcrud',
-                'open-ils.pcrud.retrieve.ahr',
-                $self->{session}->{authtoken},
-                $hold
+    if (ref($response->{result})) {
+        my $event = (ref($response->{result}) eq 'ARRAY') ? $response->{result}->[0] : $response->{result}->{last_event};
+        if ($event->{textcode} eq 'HOLD_EXISTS') {
+            return NCIP::Problem->new(
+                {
+                    ProblemType => 'Duplicate Request',
+                    ProblemDetail => 'A request for this item already exists for this patron.',
+                    ProblemElement => 'NULL',
+                    ProblemValue => 'NULL'
+                }
             );
         }
-    } elsif ($r->{last_event}) {
-        $hold = _problem_from_event('User Ineligible To Request This Item', $r->{last_event});
-    } elsif ($r->{textcode}) {
-        $hold = _problem_from_event('User Ineligible To Request This Item', $r);
-    } else {
-        $hold = _problem_from_event('User Ineligible To Request This Item');
+        if ($event->{textcode} eq 'ITEM_NOT_HOLDABLE') {
+            return NCIP::Problem->new(
+                {
+                    ProblemType => 'User Ineligible To Request This Item',
+                    ProblemDetail => 'Agency rules prevent the Item from being requested by the User.',
+                    ProblemElement => 'NULL',
+                    ProblemValue => 'NULL'
+                }
+            );
+        }
+        if ($event->{textcode} eq 'HIGH_LEVEL_HOLD_HAS_NO_COPIES') {
+            return NCIP::Problem->new(
+                {
+                    ProblemType => 'Unknown Item',
+                    ProblemDetail => 'Agency does not have an Item to fill this request.',
+                    ProblemElement => 'NULL',
+                    ProblemValue => 'NULL'
+                }
+            );
+        }
+        return _problem_from_event('User Ineligible To Request This Item', $event);
     }
 
+    # If we make it here, $response->{result} is a hold id.
+    my $hold = $U->simplereq(
+        'open-ils.pcrud',
+        'open-ils.pcrud.retrieve.ahr',
+        $self->{session}->{authtoken},
+        $response->{result}
+    );
     return $hold;
 }
 
@@ -2558,47 +2490,6 @@ sub find_item_barcode {
     return (wantarray) ? ($value, $idfield) : $value;
 }
 
-=head2 find_target_details_by_bre
-
-    $copy_details = $ils->find_target_details_by_bre($bre, $selection_ou);
-
-Returns copy details hashref for the "first" holdable copy found on a
-biblio.record_entry at an optionally given selection organization.  If
-no suitable copy is found, this method returns undef.
-
-=cut
-
-sub find_target_details_by_bre {
-    my $self = shift;
-    my $bre = shift;
-    my $selection_ou = shift;
-
-    # The copy details that we find:
-    my $details;
-
-    # We're going to search for non-deleted call numbers and flesh
-    # copies with copy location and copy status.
-    my $acns = $self->_call_number_search($bre->id(), $selection_ou, 1);
-    if ($acns && @$acns) {
-        # Now, we get available copies, sorted by status id.  We
-        # only need one, so we take the first that comes out.
-        my @copies;
-        foreach (@$acns) {
-            push(@copies, @{$_->copies()});
-        }
-        my ($copy) = sort {$a->status->id() <=> $b->status->id()}
-            grep { $_->deleted() eq 'f' && $_->holdable() eq 't' && $_->circulate() eq 't' &&
-                       $_->location->holdable() eq 't' && $_->location->circulate() eq 't' &&
-                           $_->status->holdable() eq 't' && $_->status->copy_active() eq 't' }
-                @copies;
-        if ($copy) {
-            $details = $self->retrieve_copy_details_by_id($copy->id());
-        }
-    }
-
-    return $details;
-}
-
 =head2 find_location_failover
 
     $location = $ils->find_location_failover($location, $request, $message);
@@ -2749,53 +2640,6 @@ sub _init {
     }
 }
 
-# Search asset.call_number by a bre.id and location object.
-sub _call_number_search {
-    my $self = shift;
-    my $bibid = shift;
-    my $location = shift;
-    my $flesh = shift;
-
-    my $search = {record => $bibid, deleted => 'f'};
-    if ($location) {
-        $search->{owning_lib} = {
-            in => {
-                select => {
-                    aou => [{
-                        column => 'id',
-                        transform => 'actor.org_unit_descendants',
-                        result_field => 'id'
-                    }]
-                },
-                from => 'aou',
-                where => {id => $location->id()}
-            }
-        };
-    }
-
-    # If flesh is passed a true value, we flesh copies, copy status,
-    # and copy location for the call numbers.
-    if ($flesh) {
-        $flesh = {
-            flesh => 2,
-            flesh_fields => {
-                acn => ['copies'],
-                acp => ['status', 'location']
-            }
-        }
-    }
-
-    my $acns = $U->simplereq(
-        'open-ils.pcrud',
-        'open-ils.pcrud.search.acn.atomic',
-        $self->{session}->{authtoken},
-        $search,
-        $flesh
-    );
-
-    return $acns;
-}
-
 # Search for holds using the user, idvalue and selection_ou.
 sub _hold_search {
     my $self = shift;