From: Bill Erickson Date: Thu, 9 Jun 2016 16:17:06 +0000 (-0400) Subject: hold targeter reify experiment X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=643d35ff7478e1c9b2fe07ae87060926510b3c10;p=working%2FEvergreen.git hold targeter reify experiment Signed-off-by: Bill Erickson --- diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm index 7384ef6921..018d510e94 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/HoldTargeter.pm @@ -71,7 +71,6 @@ use OpenSRF::AppSession; use OpenSRF::Utils::Logger qw(:logger); use OpenILS::Application::AppUtils; use OpenILS::Utils::CStoreEditor qw/:funcs/; -use OpenILS::Utils::PermitHold; sub new { my ($class, %args) = @_; @@ -137,7 +136,8 @@ sub result { error => $self->error, success => $self->success, message => $self->message, - targeted_copy => $self->{targeted_copy} + targeted_copy => $self->{targeted_copy}, + found_copy => $self->{found_copy} }; } @@ -212,26 +212,6 @@ sub handle_expired_hold { return $self->exit_targeter("Hold is expired"); } -# Hold copy maps are only automatically deleted when the hold -# is fulfilled or canceled. Here, they have to be manually removed. -# Returns true on success, false on error. -sub remove_copy_maps { - my $self = shift; - my $e = $self->editor; - my $prev_maps = - $e->search_action_hold_copy_map({hold => $self->hold_id}); - - for my $map (@$prev_maps) { - if (!$e->delete_action_hold_copy_map($map)) { - my $evt = $e->die_event; - return $self->exit_targeter( - "Error deleting copy maps: ".$evt->{textcode}, 1); - } - } - - return 1; -} - sub get_hold_copies { my $self = shift; my $e = $self->editor; @@ -306,11 +286,10 @@ sub get_hold_copies { } unless ($hold_type eq 'C' || $hold_type eq 'I' || $hold_type eq 'P') { - # TODO: Should this include 'R' holds? The original hold targeter - # does not include them either. # For volume and higher level holds, avoid targeting copies that # act as instances of monograph parts. - + # TODO: Should this include 'R' holds? The original hold + # targeter does not include them either. $query->{from}->{acp}->{acpm} = { type => 'left', field => 'target_copy', @@ -416,34 +395,16 @@ sub inspect_potential_copies { } -sub build_copy_maps { +# Delete and rebuild copy maps +sub update_copy_maps { my $self = shift; my $e = $self->editor; - for my $copy_hash (@{$self->copy_hashes}) { - my $map = Fieldmapper::action::hold_copy_map->new; - - $map->hold($self->hold_id); - $map->target_copy($copy_hash->{id}); - - if (!$e->create_action_hold_copy_map($map)) { - my $evt = $e->die_event; - return $self->exit_targeter( - "Error creating hold copy map: ".$evt->{textcode}, 1); - } - } - - # Collect copy proximity info (generated via DB trigger) from the - # newly create copy maps. - my $proximities = $e->json_query({ - select => {ahcm => ['target_copy', 'proximity']}, - from => 'ahcm', - where => {hold => $self->hold_id} - }); - - $self->copy_prox_map({ - map {$_->{target_copy} => $_->{proximity}} @$proximities - }); + $e->json_query({from => [ + 'action.hold_request_regen_copy_maps', + $self->hold_id, + map {$_->{id}} @{$self->copy_hashes} # @array / variadic + ]}); return 1; } @@ -455,9 +416,20 @@ sub build_copy_maps { sub compile_weighted_proximity_map { my $self = shift; + # Collect copy proximity info (generated via DB trigger) + # from our newly create copy maps. + my $hold_copy_maps = $self->editor->json_query({ + select => {ahcm => ['target_copy', 'proximity']}, + from => 'ahcm', + where => {hold => $self->hold_id} + }); + + my %copy_prox_map = + map {$_->{target_copy} => $_->{proximity}} @$hold_copy_maps; + my %prox_map; for my $copy_hash (@{$self->copy_hashes}) { - my $prox = $self->copy_proximity_map->{$copy_hash->{id}}; + my $prox = $copy_prox_map{$copy_hash->{id}}; $prox_map{$prox} ||= []; my $weight = $self->parent->get_ou_setting( @@ -538,12 +510,9 @@ sub inspect_previous_target { $self->{valid_previous_copy} = $prev; - # If there are other copies we can focus on first, remove the - # previous copy from the working set of potential copies. (The - # previous copy may be revisited later as needed). Otherwise, - # treat the previous copy as the only potential copy. - $self->copy_hashes([grep {$_->{id} ne $prev_id} @copies]) - if scalar(@copies) > 1; + # Remove the previous copy from the working set of potential copies. + # It will be revisited later if needed. + $self->copy_hashes([grep {$_->{id} ne $prev_id} @copies]); return 1; } @@ -766,16 +735,18 @@ sub copy_is_permitted { my ($self, $copy) = @_; return 0 unless $copy; - my $permitted = OpenILS::Utils::PermitHold::permit_copy_hold({ - patron_id => $self->hold->usr, - copy_id => $copy->{id}, - requestor => $self->hold->requestor, - request_lib => $self->hold->request_lib, - pickup_lib => $self->hold->pickup_lib, - retarget => 1 + my $resp = $self->editor->json_query({ + from => [ + 'action.hold_retarget_permit_test', + $self->hold->request_lib, + $self->hold->pickup_lib, + $copy->{id}, + $self->hold->usr, + $self->hold->requestor + ] }); - return 1 if $permitted; # found a targetable copy. + return 1 if $resp->[0]->{success}; # Copy is confirmed non-viable. # Remove it from our potentials list. @@ -858,10 +829,9 @@ sub target { return $self->exit_targeter("Hold is not eligible for targeting") if $hold->capture_time || $hold->cancel_time; - return unless $self->remove_copy_maps; return unless $self->handle_expired_hold; return unless $self->get_hold_copies; - return unless $self->build_copy_maps; + return unless $self->update_copy_maps; return unless $self->inspect_previous_target; return unless $self->filter_copies_by_status; return unless $self->filter_closed_date_copies; diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql new file mode 100644 index 0000000000..43983daa77 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.hold_targeter.sql @@ -0,0 +1,13 @@ + + +BEGIN; + +CREATE OR REPLACE FUNCTION + action.hold_request_regen_copy_maps( + hold_id INTEGER, copy_ids VARIADIC INTEGER[]) RETURNS VOID AS $$ + DELETE FROM action.hold_copy_map WHERE hold = $1; + INSERT INTO action.hold_copy_map (hold, target_copy) SELECT $1, UNNEST($2); +$$ LANGUAGE SQL; + +COMMIT; +