Bugzilla – Attachment 104622 Details for
Bug 20816
Add ability to send custom field(s) containing patron information in SIP patron responses
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20816: [19.11.x] Add ability to define custom templated fields in SIP patron responses
Bug-20816-1911x-Add-ability-to-define-custom-templ.patch (text/plain), 15.98 KB, created by
Nick Clemens (kidclamp)
on 2020-05-08 21:52:03 UTC
(
hide
)
Description:
Bug 20816: [19.11.x] Add ability to define custom templated fields in SIP patron responses
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2020-05-08 21:52:03 UTC
Size:
15.98 KB
patch
obsolete
>From 0c91f9a55d0cc7b2c755891c6c21401b115671e3 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 19 Jun 2019 17:59:14 +0000 >Subject: [PATCH] Bug 20816: [19.11.x] 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: > <login id="koha" password="koha" delimiter="|" error-detect="enabled" institution="kohalibrary" encoding="utf8" > > <custom_patron_field field="DE" template="[% patron.dateexpiry %]" /> > </login> > 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 > <custom_patron_field field="EW" template="Phone: [% patron.phone %] Email: [% patron.email %]" /> > 7 - prove -v t/db_dependent/SIP/Patron.t > 8 - prove -v t/db_dependent/SIP/ > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Bug 20816: Make SIP tests pass under ES > >Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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/Message.t | 17 +++++++++------ > t/db_dependent/SIP/Patron.t | 46 ++++++++++++++++++++++++++++++++++++++- > t/db_dependent/SIP/Transaction.t | 21 +++++++----------- > 7 files changed, 118 insertions(+), 32 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 425ff76552..df952f67b8 100644 >--- a/C4/SIP/Sip/MsgType.pm >+++ b/C4/SIP/Sip/MsgType.pm >@@ -435,7 +435,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... >@@ -458,6 +463,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 { >@@ -971,7 +977,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 >@@ -1026,6 +1036,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 { > >@@ -1274,7 +1285,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 1e5c2314a2..883cc15aed 100644 >--- a/etc/SIPconfig.xml >+++ b/etc/SIPconfig.xml >@@ -65,6 +65,7 @@ > <syspref_overrides> > <AllFinesNeedOverride>0</AllFinesNeedOverride> > </syspref_overrides> >+ <custom_patron_field field="DE" template="[% patron.dateexpiry %]" /> > </login> > </accounts> > >diff --git a/t/db_dependent/SIP/Message.t b/t/db_dependent/SIP/Message.t >index bc8e87a4b3..75ecb7f336 100755 >--- a/t/db_dependent/SIP/Message.t >+++ b/t/db_dependent/SIP/Message.t >@@ -326,11 +326,14 @@ sub test_checkin_v2 { > my $card1 = $patron1->{cardnumber}; > my $sip_patron1 = C4::SIP::ILS::Patron->new( $card1 ); > $findpatron = $sip_patron1; >- my $item = $builder->build({ >- source => 'Item', >- value => { damaged => 0, withdrawn => 0, itemlost => 0, restricted => 0, homebranch => $branchcode, holdingbranch => $branchcode }, >+ my $item_object = $builder->build_sample_item({ >+ damaged => 0, >+ withdrawn => 0, >+ itemlost => 0, >+ restricted => 0, >+ homebranch => $branchcode, >+ holdingbranch => $branchcode, > }); >- my $item_object = Koha::Items->find( $item->{itemnumber} ); > > my $mockILS = $mocks->{ils}; > my $server = { ils => $mockILS, account => {} }; >@@ -364,7 +367,7 @@ sub test_checkin_v2 { > $siprequest = CHECKIN . 'N' . 'YYYYMMDDZZZZHHMMSS' . > siprequestdate( $today->clone->add( days => 1) ) . > FID_INST_ID . $branchcode . '|'. >- FID_ITEM_ID . $item->{barcode} . '|' . >+ FID_ITEM_ID . $item_object->barcode . '|' . > FID_TERMINAL_PWD . 'ignored' . '|'; > undef $response; > $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 ); >@@ -443,7 +446,7 @@ sub test_checkin_v2 { > $server->{account}->{ct_always_send} = 0; > > # Checkin at wrong branch: issue item and switch branch, and checkin >- my $issue = Koha::Checkout->new({ branchcode => $branchcode, borrowernumber => $patron1->{borrowernumber}, itemnumber => $item->{itemnumber} })->store; >+ my $issue = Koha::Checkout->new({ branchcode => $branchcode, borrowernumber => $patron1->{borrowernumber}, itemnumber => $item_object->itemnumber })->store; > $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); > undef $response; >@@ -452,7 +455,7 @@ sub test_checkin_v2 { > is( substr($response,2,1), '0', 'OK flag is false when we check in at the wrong branch and we do not allow it' ); > is( substr($response,5,1), 'Y', 'Alert flag is set' ); > check_field( $respcode, $response, FID_SCREEN_MSG, 'Checkin failed', 'Check screen msg' ); >- $branchcode = $item->{homebranch}; # switch back >+ $branchcode = $item_object->homebranch; # switch back > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); > > # Data corrupted: add same issue_id to old_issues >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; > >diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t >index 5a46e6f1b3..d2f08aba64 100755 >--- a/t/db_dependent/SIP/Transaction.t >+++ b/t/db_dependent/SIP/Transaction.t >@@ -49,25 +49,20 @@ subtest fill_holds_at_checkout => sub { > t::lib::Mocks::mock_userenv({ branchcode => $branch->{branchcode}, flags => 1 }); > > my $itype = $builder->build({ source => 'Itemtype', value =>{notforloan=>0} }); >- my $biblio = $builder->build({ source => 'Biblio' }); >- my $biblioitem = $builder->build({ source => 'Biblioitem', value=>{biblionumber=>$biblio->{biblionumber}} }); >- my $item1 = $builder->build({ source => 'Item', value => { >+ my $item1 = $builder->build_sample_item({ > barcode => 'barcode4test', > homebranch => $branch->{branchcode}, > holdingbranch => $branch->{branchcode}, >- biblionumber => $biblio->{biblionumber}, > itype => $itype->{itemtype}, > notforloan => 0, >- } >- }); >- my $item2 = $builder->build({ source => 'Item', value => { >+ })->unblessed; >+ my $item2 = $builder->build_sample_item({ > homebranch => $branch->{branchcode}, > holdingbranch => $branch->{branchcode}, >- biblionumber => $biblio->{biblionumber}, >+ biblionumber => $item1->{biblionumber}, > itype => $itype->{itemtype}, > notforloan => 0, >- } >- }); >+ })->unblessed; > > Koha::IssuingRule->new({ > categorycode => $borrower->{categorycode}, >@@ -80,9 +75,9 @@ subtest fill_holds_at_checkout => sub { > lengthunit => 'days', > })->store; > >- my $reserve1 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$biblio->{biblionumber}); >- my $reserve2 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$biblio->{biblionumber}); >- my $bib = Koha::Biblios->find( $biblio->{biblionumber} ); >+ my $reserve1 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$item1->{biblionumber}); >+ my $reserve2 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$item1->{biblionumber}); >+ my $bib = Koha::Biblios->find( $item1->{biblionumber} ); > is( $bib->holds->count(), 2, "Bib has 2 holds"); > > my $sip_patron = C4::SIP::ILS::Patron->new( $borrower->{cardnumber} ); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20816
:
90813
|
93493
|
93494
|
96811
|
96812
|
102447
|
102469
|
102470
|
103385
|
103386
| 104622