From 68469120836c06dc5f244d905d35c487049e1825 Mon Sep 17 00:00:00 2001 From: Josh Stompro Date: Thu, 28 Feb 2019 10:41:01 -0600 Subject: [PATCH] LP#1752334 - BadContact, use additional note if provided. If an additional note is provided when marking a contact invalid, then that note should be appended to the penalty note field. This change adds that functionality. This doesn't change the results for staff that are using the invalidate buttons in the client, since there isn't a chance to add an additional note from that interface. But if someone is calling open-ils.actor.invalidate.email via some other method, then you can specify the additional note. I would like to be able to add extra notes when automatically processing bounced emails, to give staff a clue as to why the email failed. Such as " Account over quota for past 3 weeks, ask patron to check their account." Includes a new perl live test to check the new behavior, along with all the open-ils.actor.invalidate.* call options. Testing plan: Before fix. 1. Choose a patron with an email address and note their patron id number. 2. Run an srfsh shell. 3. Login to get an auth token 'login username password' 4. Run the invalidate email command to mark the users email invalid, including an addition note value. srfsh# request open-ils.actor open-ils.actor.invalidate.email "" "" "Testing Additional Note" 5. Take a look at the patron account and see that the penalty message only includes a copy of the email address that was marked invalid. After fix. 1. Restart the opensrf actor service if needed to re-read the changed perl module. osrf_control -l --service open-ils.actor --restart 2. Remove the penalty and re-add email address to test patron's account. 3. Run an srfsh shell. 4. Login to get an auth token 'login username password' 5. Run the invalidate email command to mark the users email invalid, including an addition note value. srfsh# request open-ils.actor open-ils.actor.invalidate.email "" "" "Testing Additional Note" 6. Take a look at the patron account and see that the penalty message note now includes the additional note. Run the live test at Evergreen/Open-ILS/src/perlmods/live_t perl ./36-lp1752334-badcontact.t Signed-off-by: Josh Stompro Signed-off-by: Galen Charlton --- .../src/perlmods/lib/OpenILS/Utils/BadContact.pm | 2 +- .../src/perlmods/live_t/36-lp1752334-badcontact.t | 243 +++++++++++++++++++++ 2 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 Open-ILS/src/perlmods/live_t/36-lp1752334-badcontact.t diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Utils/BadContact.pm b/Open-ILS/src/perlmods/lib/OpenILS/Utils/BadContact.pm index 40dbe19312..78a5b4be38 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Utils/BadContact.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Utils/BadContact.pm @@ -102,7 +102,7 @@ sub mark_users_contact_invalid { my ($result) = $U->simplereq('open-ils.actor', 'open-ils.actor.user.penalty.apply', $editor->authtoken, $usr_penalty, - { message => $_->$contact_type } + { message => $addl_note ? $_->$contact_type.' '.$addl_note : $_->$contact_type } ); # FIXME: this perpetuates a bug; the patron editor UI doesn't handle these error states well diff --git a/Open-ILS/src/perlmods/live_t/36-lp1752334-badcontact.t b/Open-ILS/src/perlmods/live_t/36-lp1752334-badcontact.t new file mode 100644 index 0000000000..f927227cdb --- /dev/null +++ b/Open-ILS/src/perlmods/live_t/36-lp1752334-badcontact.t @@ -0,0 +1,243 @@ +#!perl +use strict; use warnings; +use Test::More tests => 80; + +use OpenILS::Utils::TestUtils; +use OpenILS::Const qw(:const); +use OpenILS::Utils::Fieldmapper; +use Data::Dumper; + +my $script = OpenILS::Utils::TestUtils->new(); +$script->bootstrap(); + +my $U = 'OpenILS::Application::AppUtils'; + +diag("Test LP 1752334 BadContact"); + +use constant { + BR1_ID => 4, + BR3_ID => 6, + PHONE => '218-555-0177', + EMAIL => 'nouser@evergreen-ils.test', + TESTMESSAGE => '123456 TEST Invalidate Message', + PROFILE => 2, #patrons +}; + +### Fields that can be invalidated +# email +# day_phone +# evening_phone +# other_phone + +my @fields = ('email','day_phone','evening_phone','other_phone'); +### Notification data, field index 0 is the penalty type code +my %data =( + email => [ 31,'nouser1@evergreen-ils.test','nouser2@evergreen-ils.test', + 'nouser3@evergreen-ils.test','nouser4@example.com', + 'nouser5@example.test'], + day_phone => [32,'218-555-0177','218-555-0129','218-555-0110','218-555-0196','218-555-0181'], + evening_phone => [33,'701-555-0130','701-555-0104','701-555-0155','701-555-0156','701-555-0143'], + other_phone => [34,'612-555-0111','612-555-0115','612-555-0157','612-555-0162','612-555-0192'], +); + +### Options for invalidation +# Additional Note +# Penalty Org Unit -- ignore +# Notification string - invalidates all occurences of that type of notification. + + +# We are deliberately NOT using the admin user to check for a perm failure. +my $credentials = { + username => 'br1mtownsend', + password => 'maryt1234', + type => 'staff' +}; + +sub remove_penalty_from_patron { + my $penalty = shift; + + #Fetch the ausp object, we have aump + my $ausp = $U->simplereq( + 'open-ils.pcrud', + 'open-ils.pcrud.search.ausp.atomic', + $script->authtoken, + {id => $penalty->id() } + ); + + #Use the ausp to remove the penalty + return $U->simplereq( + 'open-ils.actor', + 'open-ils.actor.user.penalty.remove', + $script->authtoken, + $ausp->[0] + ); + +} + +sub set_notifications { + my ($user,$i) = @_; + #Set notifications + $user->email($data{email}[$i]); + $user->day_phone($data{day_phone}[$i]); + $user->evening_phone($data{evening_phone}[$i]); + $user->other_phone($data{other_phone}[$i]); + $user->ischanged(1); ## Has to be included or update won't happen + + my $resp = $U->simplereq( + 'open-ils.actor', + 'open-ils.actor.patron.update', + $script->authtoken, + $user + ); + + return($resp); +} + +sub invalidate { + my ($userid,$type,$note,$ou,$search) = @_; + my $respi = $U->simplereq( + 'open-ils.actor', + 'open-ils.actor.invalidate.'.$type, + $script->authtoken, + $userid, + $note, + $ou, + $search + ); + return $respi +} + +sub invalidate_all { + my ($userid,$note,$ou,$search) = @_; + + foreach( @fields ){ + my $respi = invalidate($userid,$_,$note,$ou, defined($search) ? $data{$_}[$search] : undef); + is($respi->{textcode},'SUCCESS',$_.' Invalidation was a success'); + } +} + +sub check_all_penalties { + my ($userid,$note,$ou,$search,$i) = @_; + + foreach( @fields ){ + my $respi = check_penalty($userid,$_,$note,$ou, + defined($search) ? $data{$_}[$search] : undef,$i); + + + } +} + +sub check_penalty { + my ($userid,$type,$note,$ou,$search,$i) = @_; + + my $code = $data{$type}[0]; #Penalty Type Code + my $ausp = $U->simplereq( + 'open-ils.pcrud', + 'open-ils.pcrud.search.aump.atomic', + $script->authtoken, + {usr => $userid, standing_penalty => $code, stop_date => undef }, + {limit => 1} + ); + + my $penalty = $ausp->[0]; + #print ref($penalty)."\n"; + my $message = $data{$type}[$i].($note ? ' '.$note : ''); + + isa_ok($penalty, 'Fieldmapper::actor::usr_message_penalty', 'User Penalty Found -- '.$type); + is($penalty->message(), $message, $type.' penalty note matches expected format.'); + + ## Remove penalty + ok( ! ref (remove_penalty_from_patron($penalty)), $type.' invalid notification penalty pemoved'); +} + + +# Log in as staff. +my $authtoken = $script->authenticate($credentials); +ok( + $authtoken, + 'Logged in' +) or BAIL_OUT('Must log in'); + +# Get a cstore editor for later use. +my $editor = $script->editor(authtoken=>$script->authtoken); + + +# Find a patron to use. +my $aus = $U->simplereq( + 'open-ils.pcrud', + 'open-ils.pcrud.search.au.atomic', + $authtoken, + {profile => PROFILE, active => 't', home_ou => BR1_ID }, + {limit => 5} +); +ok(@{$aus} == 5, 'Found 5 patrons'); +my $user = $aus->[0]; +isa_ok( + $user, + 'Fieldmapper::actor::user', + 'Found a patron' +) or BAIL_OUT('Patron not found'); + + +my $resp = set_notifications($user,1); +isa_ok($resp, 'Fieldmapper::actor::user', 'Notifications added patron 1'); + +#print Dumper($resp); + +## Next user +$user = $aus->[1]; +$resp = set_notifications($user,2); +isa_ok($resp, 'Fieldmapper::actor::user', 'Notifications added patron 2'); + +## Users 3,4,5 have the same notifications set +$user = $aus->[2]; +$resp = set_notifications($user,3); +isa_ok($resp, 'Fieldmapper::actor::user', 'Notifications added patron 3'); + +$user = $aus->[3]; +$resp = set_notifications($user,3); +isa_ok($resp, 'Fieldmapper::actor::user', 'Notifications added patron 4'); + +$user = $aus->[4]; +$resp = set_notifications($user,3); +isa_ok($resp, 'Fieldmapper::actor::user', 'Notifications added patron 5'); + +#Invalidate all notifications for user 1 - default settings +diag("Patron 1 - default invalidate settings"); +$user = $aus->[0]; +invalidate_all($user->id(),undef,$user->home_ou(),undef); + +#Invalidate all notifications for user 2 - added note +diag("Patron 2 - Added note"); +$user = $aus->[1]; +invalidate_all($user->id(),TESTMESSAGE,$user->home_ou(),undef); + +#Invalidate notifications for users 3,4,5 - using search method with test message +diag("Patron 3,4,5 - Added note - same contact info"); +$user = $aus->[2]; +invalidate_all(undef,TESTMESSAGE,$user->home_ou(),3); #Search is index to notification data + + +## Check and clear standing penalties +diag("Patron 1 - default invalidate settings"); +$user = $aus->[0]; +check_all_penalties($user->id(),undef,$user->home_ou(),undef,1); + +diag("Patron 2 - Added note"); +$user = $aus->[1]; +check_all_penalties($user->id(),TESTMESSAGE,$user->home_ou(),undef,2); + +diag("Patron 3 - Added note - same contact info"); +$user = $aus->[2]; +check_all_penalties($user->id(),TESTMESSAGE,$user->home_ou(),undef,3); + +diag("Patron 4 - Added note - same contact info"); +$user = $aus->[3]; +check_all_penalties($user->id(),TESTMESSAGE,$user->home_ou(),undef,3); + +diag("Patron 5 - Added note - same contact info"); +$user = $aus->[4]; +check_all_penalties($user->id(),TESTMESSAGE,$user->home_ou(),undef,3); + +# Logout +$script->logout(); # Not a test, just to be pedantic. \ No newline at end of file -- 2.11.0