From e9da70f2d20024a709ae79f45bb3c380a90e6044 Mon Sep 17 00:00:00 2001 From: Jason Stephenson Date: Fri, 23 Oct 2015 15:02:58 -0400 Subject: [PATCH] Simplify RequestItem handling in NCIP::ILS::Evergreen. 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 --- lib/NCIP/ILS/Evergreen.pm | 250 +++++++------------------------------- 1 file changed, 47 insertions(+), 203 deletions(-) diff --git a/lib/NCIP/ILS/Evergreen.pm b/lib/NCIP/ILS/Evergreen.pm index 11f2488..45ad829 100644 --- a/lib/NCIP/ILS/Evergreen.pm +++ b/lib/NCIP/ILS/Evergreen.pm @@ -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; -- 2.43.2