LP 1499123: Modify Perl code for csp.ignore_proximity field.
authorJason Stephenson <jason@sigio.com>
Sat, 26 Sep 2015 15:42:35 +0000 (11:42 -0400)
committerKathy Lussier <klussier@masslnc.org>
Tue, 16 Feb 2016 17:45:07 +0000 (12:45 -0500)
* 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 <jason@sigio.com>
Signed-off-by: Kathy Lussier <klussier@masslnc.org>
Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
Open-ILS/src/perlmods/live_t/12-lp1499123_csp_ignore_proximity.t [new file with mode: 0644]

index 3d0ec38..ab89ac4 100644 (file)
@@ -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) = @_;
index c729abc..befe1bc 100644 (file)
@@ -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}}
+                ]
+            }
         }
     });
 
index ac4f05c..1600db1 100644 (file)
@@ -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 (file)
index 0000000..2b993bc
--- /dev/null
@@ -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();