From 41b5db64b352a9cf33595171ca9c2bee70ca4f1a Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 19 Jun 2019 17:59:14 +0000 Subject: [PATCH] Bug 20816: Add ability to define custom templated fields in SIP patron responses To test: 1 - You will need to enable SIP on your testing instance cp etc/SIPconfig.xml /etc/koha/sites/kohadev/ sudo koha-start-sip add a user listed in the SIPconfig to your system and give them permissions (superlibrarian works) on koha-testing-docker you should be able to start sip with user koha/koha without any adjustments 2 - If you copied the above file you should be set to get custom field DE with dateexpiry Otherwise edit the sip login for the user to have a custom section like: 3 - send a status test using the sip cli tester: perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l kohalibrary --patron 23529001000463 -m patron_status_request 4 - send an information test using the sip cli tester: perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l kohalibrary --patron 23529001000463 -m patron_information 5 - confirm you receive the DE field with a dateexpiry 6 - Add your own custom fields and confirm it works with several 7 - prove -v t/db_dependent/SIP/Patron.t 8 - prove -v t/db_dependent/SIP/ Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Martin Renvoize --- C4/SIP/ILS/Patron.pm | 47 +++++++++++++++++++++++++++++++------ C4/SIP/Sip.pm | 1 - C4/SIP/Sip/MsgType.pm | 17 +++++++++++--- etc/SIPconfig.xml | 1 + t/db_dependent/SIP/Patron.t | 46 +++++++++++++++++++++++++++++++++++- 5 files changed, 100 insertions(+), 12 deletions(-) diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 99546da569..b528501f39 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -15,7 +15,7 @@ use Carp; use Sys::Syslog qw(syslog); use Data::Dumper; -use C4::SIP::Sip qw(add_field); +use C4::SIP::Sip qw(add_field maybe_add); use C4::Debug; use C4::Context; @@ -216,7 +216,14 @@ sub AUTOLOAD { } } -sub name { +=head2 format + +This method uses a template to build a string from a Koha::Patron object +If errors are encountered in processing template we log them and return nothing + +=cut + +sub format { my ( $self, $template ) = @_; if ($template) { @@ -228,12 +235,15 @@ sub name { my $patron = Koha::Patrons->find( $self->{borrowernumber} ); my $output; - $tt->process( \$template, { patron => $patron }, \$output ); + eval { + $tt->process( \$template, { patron => $patron }, \$output ); + }; + if ( $@ ){ + syslog("LOG_DEBUG", "Error processing template: $template"); + return ""; + } return $output; } - else { - return $self->{name}; - } } sub check_password { @@ -528,7 +538,6 @@ sub build_patron_attributes_string { my ( $self, $server ) = @_; my $string = q{}; - if ( $server->{account}->{patron_attribute} ) { my @attributes_to_send = ref $server->{account}->{patron_attribute} eq "ARRAY" @@ -553,6 +562,30 @@ sub build_patron_attributes_string { return $string; } + +=head2 build_custom_field_string + +This method builds the part of the sip message for custom patron fields as defined in the sip config + +=cut + +sub build_custom_field_string { + my ( $self, $server ) = @_; + + my $string = q{}; + + if ( $server->{account}->{custom_patron_field} ) { + my @custom_fields = + ref $server->{account}->{custom_patron_field} eq "ARRAY" + ? @{ $server->{account}->{custom_patron_field} } + : $server->{account}->{custom_patron_field}; + foreach my $custom_field ( @custom_fields ) { + $string .= maybe_add( $custom_field->{field}, $self->format( $custom_field->{template} ) ) if defined $custom_field->{field}; + } + } + return $string; +} + 1; __END__ diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm index a708fa148a..2dc4749214 100644 --- a/C4/SIP/Sip.pm +++ b/C4/SIP/Sip.pm @@ -108,7 +108,6 @@ sub maybe_add { $value =~ s/$regex->{find}/$regex->{replace}/g; } } - return (defined($value) && $value) ? add_field($fid, $value) : ''; } diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 3a3ea840a4..3184ebba96 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -436,7 +436,12 @@ sub build_patron_status { $resp .= patron_status_string($patron); $resp .= $lang . timestamp(); - $resp .= add_field( FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ), $server ); + if ( defined $server->{account}->{ae_field_template} ) { + $resp .= add_field( FID_PERSONAL_NAME, $patron->format( $server->{account}->{ae_field_template}, $server ) ); + } else { + $resp .= add_field( FID_PERSONAL_NAME, $patron->name, $server ); + } + # while the patron ID we got from the SC is valid, let's # use the one returned from the ILS, just in case... @@ -459,6 +464,7 @@ sub build_patron_status { if ( $server->{account}->{send_patron_home_library_in_af} ); $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line, $server ); + $resp .= $patron->build_custom_field_string( $server ); $resp .= $patron->build_patron_attributes_string( $server ); } else { @@ -972,7 +978,11 @@ sub handle_patron_info { # while the patron ID we got from the SC is valid, let's # use the one returned from the ILS, just in case... $resp .= add_field( FID_PATRON_ID, $patron->id, $server ); - $resp .= add_field( FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ), $server ); + if ( defined $server->{account}->{ae_field_template} ) { + $resp .= add_field( FID_PERSONAL_NAME, $patron->format( $server->{account}->{ae_field_template} ), $server ); + } else { + $resp .= add_field( FID_PERSONAL_NAME, $patron->name, $server ); + } # TODO: add code for the fields # hold items limit @@ -1027,6 +1037,7 @@ sub handle_patron_info { } $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line, $server ); + $resp .= $patron->build_custom_field_string( $server ); $resp .= $patron->build_patron_attributes_string( $server ); } else { @@ -1315,7 +1326,7 @@ sub handle_patron_enable { $resp .= $patron->language . timestamp(); $resp .= add_field( FID_PATRON_ID, $patron->id, $server ); - $resp .= add_field( FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ), $server ); + $resp .= add_field( FID_PERSONAL_NAME, $patron->format( $server->{account}->{ae_field_template} ), $server ); if ( defined($patron_pwd) ) { $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $patron->check_password($patron_pwd) ), $server ); } diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml index f1c516f8f2..43f53e2160 100644 --- a/etc/SIPconfig.xml +++ b/etc/SIPconfig.xml @@ -66,6 +66,7 @@ 0 + diff --git a/t/db_dependent/SIP/Patron.t b/t/db_dependent/SIP/Patron.t index 0f20ac3e5c..d517c7693b 100755 --- a/t/db_dependent/SIP/Patron.t +++ b/t/db_dependent/SIP/Patron.t @@ -4,7 +4,7 @@ # This needs to be extended! Your help is appreciated.. use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 6; use Koha::Database; use Koha::Patrons; @@ -116,6 +116,50 @@ subtest "Test build_patron_attribute_string" => sub { is( $attribute_string, "XYTest Attribute|YZAnother Test Attribute|", 'Attribute field generated correctly with multiple params' ); }; +subtest "Test build_custom_field_string" => sub { + + plan tests => 5; + + my $patron = $builder->build_object( { class => 'Koha::Patrons',value=>{surname => "Duck", firstname => "Darkwing"} } ); + + + my $ils_patron = C4::SIP::ILS::Patron->new( $patron->cardnumber ); + + my $server = {}; + $server->{account}->{custom_patron_field}->{field} = "DW"; + my $attribute_string = $ils_patron->build_custom_field_string( $server ); + is( $attribute_string, "", 'Custom field not generated if no value passed' ); + + $server = {}; + $server->{account}->{custom_patron_field}->{template} = "[% patron.surname %]"; + $attribute_string = $ils_patron->build_custom_field_string( $server ); + is( $attribute_string, "", 'Custom field not generated if no field passed' ); + + + $server = {}; + $server->{account}->{custom_patron_field}->{field} = "DW"; + $server->{account}->{custom_patron_field}->{template} = "[% patron.firstname %] [% patron.surname %], let's get dangerous!"; + $attribute_string = $ils_patron->build_custom_field_string( $server ); + is( $attribute_string, "DWDarkwing Duck, let's get dangerous!|", 'Custom field processed correctly' ); + + $server = {}; + $server->{account}->{custom_patron_field}->[0]->{field} = "DW"; + $server->{account}->{custom_patron_field}->[0]->{template} = "[% patron.firstname %] [% patron.surname %], let's get dangerous!"; + $server->{account}->{custom_patron_field}->[1]->{field} = "LM"; + $server->{account}->{custom_patron_field}->[1]->{template} = "Launchpad McQuack crashed on [% patron.dateexpiry %]"; + $attribute_string = $ils_patron->build_custom_field_string( $server ); + is( $attribute_string, "DWDarkwing Duck, let's get dangerous!|LMLaunchpad McQuack crashed on ".$patron->dateexpiry."|", 'Custom fields processed correctly when multiple exist' ); + + $server = {}; + $server->{account}->{custom_patron_field}->[0]->{field} = "DW"; + $server->{account}->{custom_patron_field}->[0]->{template} = "[% IF (patron.firstname) %] patron.surname, let's get dangerous!"; + $server->{account}->{custom_patron_field}->[1]->{field} = "LM"; + $server->{account}->{custom_patron_field}->[1]->{template} = "Launchpad McQuack crashed on [% patron.dateexpiry %]"; + $attribute_string = $ils_patron->build_custom_field_string( $server ); + is( $attribute_string, "LMLaunchpad McQuack crashed on ".$patron->dateexpiry."|", 'Custom fields processed correctly, bad template generate no text' ); + +}; + subtest "update_lastseen tests" => sub { plan tests => 2; -- 2.39.5