From: Jason Stephenson Date: Sat, 26 Sep 2015 15:42:35 +0000 (-0400) Subject: LP 1499123: Modify Perl code for csp.ignore_proximity field. X-Git-Tag: sprint4-merge-nov22~662 X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=831a808746308a174f1209d3bec9c2284602798b;p=working%2FEvergreen.git LP 1499123: Modify Perl code for csp.ignore_proximity field. * Add get_org_unit_proximity function to AppUtils. First, we add a helper function to OpenILS::Application::AppUtils that returns the proximity value between a "from" org_unit and a "to" org_unit. It takes a CStoreEditor and the ids of the two org_units as arguments. * Use csp.ignore_proximity in O::A::Circ::Circulate::Circulator. Modify the check_hold_fulfill_blocks method of the Circulator object to take the csp.ignore_proximity into account. The new code first calculates the proximity of the circ_lib and the copy's circ_lib with the patron's home_ou. It then modifies the main query in the function to check if the csp object's ignore_proximity is null or greater than either of the two calculated proximity values. * Teach SIP::Patron about csp.ignore_proximity. We modify SIP::Patron::flesh_user_penalties to not report penalties within csp.ignore_proximity of the user's home_ou. In order to have a notion of "here" for the proximity check, we modify SIP::Patron->new to assign its authtoken argument, if any, to the CStoreEditor. We then use this authtoken to retrieve the authsession user so that we may use the authsession user's ws_ou or home_ou as a context ou for penalty lookup and filtering based on the csp.ignore_proximity in flesh_user_penalties. If we're not given the authtoken, we fall back to using the patron's home_ou and the penalty's context ou for the proximity lookup. This assumes, of course, that the authsession user's ws_ou or home_ou have a logical relationship with the desired transaction ou. For most self-checks this will likely be true. For other uses of the SIP protocol, this is less likely to be true. * Add Perl live tests. Add tests for basic checkout and hold functionality as well as for the OpenILS::SIP::Patron->flesh_user_penalties() changes. Signed-off-by: Jason Stephenson Signed-off-by: Kathy Lussier --- diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index 3d0ec381b0..ab89ac4ac2 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm @@ -1496,6 +1496,23 @@ sub org_unit_ancestor_at_depth { return ($resp) ? $resp->{id} : undef; } +# Returns the proximity value between two org units. +sub get_org_unit_proximity { + my ($class, $e, $from_org, $to_org) = @_; + $e = OpenILS::Utils::CStoreEditor->new unless ($e); + my $r = $e->json_query( + { + select => {aoup => ['prox']}, + from => 'aoup', + where => {from_org => $from_org, to_org => $to_org} + } + ); + if (ref($r) eq 'ARRAY' && @$r) { + return $r->[0]->{prox}; + } + return undef; +} + # returns the user's configured locale as a string. Defaults to en-US if none is configured. sub get_user_locale { my($self, $user_id, $e) = @_; diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm index c729abc192..befe1bcb75 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm @@ -1482,6 +1482,22 @@ sub bail_on_events { sub check_hold_fulfill_blocks { my $self = shift; + # With the addition of ignore_proximity in csp, we need to fetch + # the proximity of both the circ_lib and the copy's circ_lib to + # the patron's home_ou. + my ($ou_prox, $copy_prox); + my $home_ou = (ref($self->patron->home_ou)) ? $self->patron->home_ou->id : $self->patron->home_ou; + $ou_prox = $U->get_org_unit_proximity($self->editor, $home_ou, $self->circ_lib); + $ou_prox = -1 unless (defined($ou_prox)); + my $copy_ou = (ref($self->copy->circ_lib)) ? $self->copy->circ_lib->id : $self->copy->circ_lib; + if ($copy_ou == $self->circ_lib) { + # Save us the time of an extra query. + $copy_prox = $ou_prox; + } else { + $copy_prox = $U->get_org_unit_proximity($self->editor, $home_ou, $copy_ou); + $copy_prox = -1 unless (defined($copy_prox)); + } + # See if the user has any penalties applied that prevent hold fulfillment my $pens = $self->editor->json_query({ select => {csp => ['name', 'label']}, @@ -1495,7 +1511,14 @@ sub check_hold_fulfill_blocks { {stop_date => {'>' => 'now'}} ] }, - '+csp' => {block_list => {'like' => '%FULFILL%'}} + '+csp' => { + block_list => {'like' => '%FULFILL%'}, + '-or' => [ + {ignore_proximity => undef}, + {ignore_proximity => {'<' => $ou_prox}}, + {ignore_proximity => {'<' => $copy_prox}} + ] + } } }); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm index ac4f05c3b2..1600db1649 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm @@ -51,6 +51,12 @@ sub new { syslog("LOG_DEBUG", "OILS: new OpenILS Patron(%s => %s): searching...", $key, $patron_id); my $e = OpenILS::SIP->editor(); + # Pass the authtoken, if any, to the editor so that we can use it + # to fake a context org_unit for the csp.ignore_proximity in + # flesh_user_penalties, below. + unless ($e->authtoken()) { + $e->authtoken($args{authtoken}) if ($args{authtoken}); + } my $usr_flesh = { flesh => 2, @@ -143,9 +149,22 @@ sub get_act_who { sub flesh_user_penalties { my ($self, $user, $e) = @_; - $user->standing_penalties( + # Use the ws_ou or home_ou of the authsession user, if any, as a + # context org_unit for the penalties and the csp.ignore_proximity. + my $here; + if ($e->authtoken()) { + my $auth_usr = $e->checkauth(); + if ($auth_usr) { + $here = $auth_usr->ws_ou() || $auth_usr->home_ou(); + } + } + + # Get the "raw" list of user's penalties and flesh the + # standing_penalty field, so we can filter them based on + # csp.ignore_proximity. + my $raw_penalties = $e->search_actor_user_standing_penalty([ - { + { usr => $user->id, '-or' => [ @@ -158,21 +177,20 @@ sub flesh_user_penalties { in => { select => { aou => [{ - column => 'id', - transform => 'actor.org_unit_ancestors', + column => 'id', + transform => 'actor.org_unit_ancestors', result_field => 'id' }] }, from => 'aou', - # at this point, there is no concept of "here", so fetch penalties - # for the patron's home lib plus ancestors - where => {id => $user->home_ou}, + # Use "here" or user's home_ou. + where => {id => ($here) ? $here : $user->home_ou}, distinct => 1 } }, - # in addition to fines and excessive overdue penalties, + # in addition to fines and excessive overdue penalties, # we only care about penalties that result in blocks standing_penalty => { in => { @@ -187,8 +205,28 @@ sub flesh_user_penalties { } } }, - ]) - ); + { + flesh => 1, + flesh_fields => {ausp => ['standing_penalty']} + } + ]); + # We filter the raw penalties that apply into this array. + my $applied_penalties = []; + if (ref($raw_penalties) eq 'ARRAY' && @$raw_penalties) { + my $here_prox = ($here) ? $U->get_org_unit_proximity($e, $here, $user->home_ou()) + : undef; + # Filter out those that do not apply and deflesh the standing_penalty. + $applied_penalties = [map + { $_->standing_penalty($_->standing_penalty->id()) } + grep { + !defined($_->standing_penalty->ignore_proximity()) + || ((defined($here_prox)) + ? $_->standing_penalty->ignore_proximity() < $here_prox + : $_->standing_penalty->ignore_proximity() < + $U->get_org_unit_proximity($e, $_->org_unit(), $user->home_ou())) + } @$raw_penalties]; + } + $user->standing_penalties($applied_penalties); } sub id { diff --git a/Open-ILS/src/perlmods/live_t/12-lp1499123_csp_ignore_proximity.t b/Open-ILS/src/perlmods/live_t/12-lp1499123_csp_ignore_proximity.t new file mode 100644 index 0000000000..2b993bcfeb --- /dev/null +++ b/Open-ILS/src/perlmods/live_t/12-lp1499123_csp_ignore_proximity.t @@ -0,0 +1,252 @@ +#!perl +use strict; use warnings; + +use Test::More tests => 28; +use Data::Dumper; + +diag("Test config.standing_penalty.ignore_proximity feature."); + +use OpenILS::Utils::TestUtils; +use OpenILS::SIP::Patron; +my $script = OpenILS::Utils::TestUtils->new(); +our $apputils = 'OpenILS::Application::AppUtils'; + +use constant WORKSTATION_NAME => 'BR1-test-lp1499123_csp_ignore_proximity.t'; +use constant WORKSTATION_LIB => 4; + +# Because this may run multiple times, without a DB reload, we search +# for the workstation before registering it. +sub find_workstation { + my $r = $apputils->simplereq( + 'open-ils.actor', + 'open-ils.actor.workstation.list', + $script->authtoken, + WORKSTATION_LIB + ); + if ($r->{&WORKSTATION_LIB}) { + return scalar(grep {$_->name() eq WORKSTATION_NAME} @{$r->{&WORKSTATION_LIB}}); + } + return 0; +} + +sub retrieve_staff_chr { + my $e = shift; + my $staff_chr = $e->retrieve_config_standing_penalty(25); + return $staff_chr; +} + +sub update_staff_chr { + my $e = shift; + my $penalty = shift; + $e->xact_begin; + my $r = $e->update_config_standing_penalty($penalty) || $e->event(); + if (ref($r)) { + $e->rollback(); + } else { + $e->commit; + } + return $r; +} + +sub retrieve_user_by_barcode { + my $barcode = shift; + return $apputils->simplereq( + 'open-ils.actor', + 'open-ils.actor.user.fleshed.retrieve_by_barcode', + $script->authtoken, + $barcode + ); +} + +sub retrieve_copy_by_barcode { + my $editor = shift; + my $barcode = shift; + my $r = $editor->search_asset_copy({barcode => $barcode}); + if (ref($r) eq 'ARRAY' && @$r) { + return $r->[0]; + } + return undef; +} + +sub apply_staff_chr_to_patron { + my ($staff, $patron) = @_; + my $penalty = Fieldmapper::actor::user_standing_penalty->new(); + $penalty->standing_penalty(25); + $penalty->usr($patron->id()); + $penalty->set_date('now'); + $penalty->staff($staff->id()); + $penalty->org_unit(1); # Consortium-wide. + $penalty->note('LP 1499123 csp.ignore_proximity test'); + my $r = $apputils->simplereq( + 'open-ils.actor', + 'open-ils.actor.user.penalty.apply', + $script->authtoken, + $penalty + ); + if (ref($r)) { + undef($penalty); + } else { + $penalty->id($r); + } + return $penalty; +} + +sub remove_staff_chr_from_patron { + my $penalty = shift; + return $apputils->simplereq( + 'open-ils.actor', + 'open-ils.actor.user.penalty.remove', + $script->authtoken, + $penalty + ); +} + +sub checkout_permit_test { + my $patron = shift; + my $copy_barcode = shift; + my $r = $apputils->simplereq( + 'open-ils.circ', + 'open-ils.circ.checkout.permit', + $script->authtoken, + { + patron => $patron->id(), + barcode => $copy_barcode + } + ); + if (ref($r) eq 'HASH' && $r->{textcode} eq 'SUCCESS') { + return 1; + } + return 0; +} + +sub copy_hold_permit_test { + my $editor = shift; + my $patron = shift; + my $copy_barcode = shift; + my $copy = retrieve_copy_by_barcode($editor, $copy_barcode); + if ($copy) { + my $r = $apputils->simplereq( + 'open-ils.circ', + 'open-ils.circ.title_hold.is_possible', + $script->authtoken, + { + patronid => $patron->id(), + pickup_lib => 4, + copy_id => $copy->id(), + hold_type => 'C' + } + ); + if (ref($r) && defined $r->{success}) { + return $r->{success}; + } + } + return undef; +} + +sub patron_sip_test { + my $patron_id = shift; + my $patron = OpenILS::SIP::Patron->new(usr => $patron_id, authtoken => $script->authtoken); + return scalar(@{$patron->{user}->standing_penalties()}); +} + +# In concerto, we need to register a workstation. +$script->authenticate({ + username => 'admin', + password => 'demo123', + type => 'staff', +}); +ok($script->authtoken, 'Initial Login'); + +SKIP: { + my $ws = find_workstation(); + skip 'Workstation exists', 1 if ($ws); + $ws = $script->register_workstation(WORKSTATION_NAME, WORKSTATION_LIB) unless ($ws); + ok(! ref $ws, 'Registered a new workstation'); +} + +$script->logout(); +$script->authenticate({ + username => 'admin', + password => 'demo123', + type => 'staff', + workstation => WORKSTATION_NAME +}); +ok($script->authtoken, 'Login with workstaion'); + +# Get a CStoreEditor for later use. +my $editor = $script->editor(authtoken=>$script->authtoken); +my $staff = $editor->checkauth(); +ok(ref($staff), 'Got a staff user'); + +# We retrieve STAFF_CHR block and check that it has an undefined +# ignore_proximity. +my $staff_chr = retrieve_staff_chr($editor); +isa_ok($staff_chr, 'Fieldmapper::config::standing_penalty', 'STAFF_CHR'); +is($staff_chr->name, 'STAFF_CHR', 'Penalty name is STAFF_CHR'); +is($staff_chr->ignore_proximity, undef, 'STAFF_CHR ignore_proximity is undefined'); + +# We set the ignore_proximity to 0. +$staff_chr->ignore_proximity(0); +ok(! ref update_staff_chr($editor, $staff_chr), 'Update of STAFF_CHR'); + +# We need a patron with no penalties to test holds and circulation. +my $patron = retrieve_user_by_barcode("99999350419"); +isa_ok($patron, 'Fieldmapper::actor::user', 'Patron'); + +# Patron should have no penalties. +ok(! scalar(@{$patron->standing_penalties()}), 'Patron has no penalties'); + +# Add the STAFF_CHR to the patron +my $penalty = apply_staff_chr_to_patron($staff, $patron); +ok(ref $penalty, 'Added STAFF_CHR to patron'); +is(patron_sip_test($patron->id()), 0, 'SIP says patron has no penalties'); + +# See if we can place a hold on a copy owned by BR1. +is(copy_hold_permit_test($editor, $patron, "CONC4300036"), 1, 'Can place hold on copy from BR1'); +# We should not be able to place a hold on a copy owned by a different branch. +is(copy_hold_permit_test($editor, $patron, "CONC51000636"), 0, 'Cannot place hold on copy from BR2'); + +# See if we can check out a copy owned by branch 4 out to the patron. +# This should succeed. +ok(checkout_permit_test($patron, "CONC4300036"), 'Can checkout copy from BR1'); + +# We should not be able to checkout a copy owned by a different branch. +ok(!checkout_permit_test($patron, "CONC51000636"), 'Cannot checkout copy from BR2'); + +# We reset the ignore_proximity of STAFF_CHR. +$staff_chr->clear_ignore_proximity(); +ok(! ref update_staff_chr($editor, $staff_chr), 'Reset of STAFF_CHR'); +is(patron_sip_test($patron->id()), 1, 'SIP says patron has one penalty'); + +# See if we can place a hold on a copy owned by BR1. +is(copy_hold_permit_test($editor, $patron, "CONC4300036"), 0, 'Cannot place hold on copy from BR1'); +# We should not be able to place a hold on a copy owned by a different branch. +is(copy_hold_permit_test($editor, $patron, "CONC51000636"), 0, 'Cannot place hold on copy from BR2'); + +# See if we can check out a copy owned by branch 4 out to the patron. +# This should succeed. +ok(!checkout_permit_test($patron, "CONC4300036"), 'Cannot checkout copy from BR1'); + +# We should not be able to checkout a copy owned by a different branch. +ok(!checkout_permit_test($patron, "CONC51000636"), 'Cannot checkout copy from BR2'); + +# We remove the STAFF_CHR from our test patron. +my $r = remove_staff_chr_from_patron($penalty); +ok( ! ref $r, 'STAFF_CHR removed from patron'); + +# Do the checks again, all should pass. +is(patron_sip_test($patron->id()), 0, 'SIP says patron has no penalties'); + +# See if we can place a hold on a copy owned by BR1. +is(copy_hold_permit_test($editor, $patron, "CONC4300036"), 1, 'Can place hold on copy from BR1'); +# We should now be able to place a hold on a copy owned by a different branch. +is(copy_hold_permit_test($editor, $patron, "CONC51000636"), 1, 'Can place hold on copy from BR2'); + +# See if we can check out a copy owned by branch 4 out to the patron. +# This should succeed. +ok(checkout_permit_test($patron, "CONC4300036"), 'Can checkout copy from BR1'); + +# We should not be able to checkout a copy owned by a different branch. +ok(checkout_permit_test($patron, "CONC51000636"), 'Can checkout copy from BR2'); + +$script->logout();