Bugzilla – Attachment 102585 Details for
Bug 24966
Fix calls to maybe_add where method call does not return a value
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24966: [Alt] Fix calls to maybe_add where method call does not return a value
Bug-24966-Alt-Fix-calls-to-maybeadd-where-method-c.patch (text/plain), 35.97 KB, created by
Kyle M Hall (khall)
on 2020-04-08 18:05:34 UTC
(
hide
)
Description:
Bug 24966: [Alt] Fix calls to maybe_add where method call does not return a value
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2020-04-08 18:05:34 UTC
Size:
35.97 KB
patch
obsolete
>From b7aeeb5e7fc6a9ad00a9ffca8896b73e7d249348 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Wed, 8 Apr 2020 13:43:41 -0400 >Subject: [PATCH] Bug 24966: [Alt] Fix calls to maybe_add where method call > does not return a value > >For reasons unknown to me, a call like: >-- >$resp .= maybe_add( FID_CALL_NUMBER, $item->call_number, $server ); >-- >will not work as expected if the item has no callnumber. > >One would expect the parameters to the subroutine to be: 'CY', under, and a SIPServer object. > >What is actually received is: 'CY', and a SIPServer object. > >We ingest the parameters like so: >-- >sub maybe_add { > my ($fid, $value, $server) = @_; >-- >So, what happens is $value is populated with the server object! > >This can cause bad output like this: >-- >OUTPUT MSG: '101YNN20200324 063701AOBPL|AB32503201584185|AQBPL|AJCat /|CK001|CRn|CSJ 636.8 CLU|CYC4::SIP::SIPServer=HASH(0x1ea0e58)|DAC4::SIP::SIPServer=HASH(0x1ea0e58)|' >-- > >Test Plan: >1) On master, perform a checkin of an item not on hold using the sip cli tester >2) Note some fields contain something like 'C4::SIP::SIPServer=HASH(0x1ea0e58)' >3) Apply this patch >4) Restart the SIP server >5) Perform the SIP checkin again >6) Those fields from step 2 should be gone! > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> > >Signed-off-by: Barbara Johnson <Barbara.Johnson@bedfordtx.gov> >--- > C4/SIP/Sip.pm | 4 +- > C4/SIP/Sip/MsgType.pm | 320 +++++++++++++++++++++--------------------- > 2 files changed, 162 insertions(+), 162 deletions(-) > >diff --git a/C4/SIP/Sip.pm b/C4/SIP/Sip.pm >index a708fa148a..3465346dc3 100644 >--- a/C4/SIP/Sip.pm >+++ b/C4/SIP/Sip.pm >@@ -58,7 +58,7 @@ sub timestamp { > # return constructed field value > # > sub add_field { >- my ($field_id, $value, $server) = @_; >+ my ($server, $field_id, $value) = @_; > > if ( my $hide_fields = $server->{account}->{hide_fields} ) { > my @fields = split( ',', $hide_fields ); >@@ -92,7 +92,7 @@ sub add_field { > # NOTE: if zero is a valid value for your field, don't use maybe_add! > # > sub maybe_add { >- my ($fid, $value, $server) = @_; >+ my ($server, $fid, $value) = @_; > > if ( my $hide_fields = $server->{account}->{hide_fields} ) { > my @fields = split( ',', $hide_fields ); >diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm >index 3a3ea840a4..4e4984dc60 100644 >--- a/C4/SIP/Sip/MsgType.pm >+++ b/C4/SIP/Sip/MsgType.pm >@@ -436,28 +436,28 @@ 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 ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ) ); > > # 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( $server, FID_PATRON_ID, $patron->id ); > > if ( $protocol_version >= 2 ) { >- $resp .= add_field( FID_VALID_PATRON, 'Y', $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON, 'Y' ); > > # Patron password is a required field. >- $resp .= add_field( FID_VALID_PATRON_PWD, sipbool($password_rc), $server ); >- $resp .= maybe_add( FID_CURRENCY, $patron->currency, $server ); >- $resp .= maybe_add( FID_FEE_AMT, $patron->fee_amount, $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON_PWD, sipbool($password_rc) ); >+ $resp .= maybe_add( $server, FID_CURRENCY, $patron->currency ); >+ $resp .= maybe_add( $server, FID_FEE_AMT, $patron->fee_amount ); > } > > my $msg = $patron->screen_msg; > $msg .= ' -- '. INVALID_PW if $patron_pwd && !$password_rc; >- $resp .= maybe_add( FID_SCREEN_MSG, $msg, $server, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $msg, $server ); > >- $resp .= maybe_add( FID_SCREEN_MSG, $patron->{branchcode}, $server ) >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $patron->{branchcode}, $server ) > if ( $server->{account}->{send_patron_home_library_in_af} ); >- $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $patron->print_line ); > > $resp .= $patron->build_patron_attributes_string( $server ); > >@@ -467,19 +467,19 @@ sub build_patron_status { > > # no personal name, and is invalid (if we're using 2.00) > $resp .= 'YYYY' . ( ' ' x 10 ) . $lang . timestamp(); >- $resp .= add_field( FID_PERSONAL_NAME, '', $server ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, '' ); > > # the patron ID is invalid, but it's a required field, so > # just echo it back >- $resp .= add_field( FID_PATRON_ID, $fields->{ (FID_PATRON_ID) }, $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $fields->{ (FID_PATRON_ID) } ); > > ( $protocol_version >= 2 ) >- and $resp .= add_field( FID_VALID_PATRON, 'N', $server ); >+ and $resp .= add_field( $server, FID_VALID_PATRON, 'N' ); > >- $resp .= maybe_add( FID_SCREEN_MSG, INVALID_CARD, $server, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, INVALID_CARD, $server ); > } > >- $resp .= add_field( FID_INST_ID, $fields->{ (FID_INST_ID) }, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $fields->{ (FID_INST_ID) } ); > return $resp; > } > >@@ -551,25 +551,25 @@ sub handle_checkout { > $resp .= timestamp; > > # Now for the variable fields >- $resp .= add_field( FID_INST_ID, $inst, $server ); >- $resp .= add_field( FID_PATRON_ID, $patron_id, $server ); >- $resp .= add_field( FID_ITEM_ID, $item_id, $server ); >- $resp .= add_field( FID_TITLE_ID, $item->title_id, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $inst ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron_id ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item_id ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item->title_id ); > if ( $item->due_date ) { >- $resp .= add_field( FID_DUE_DATE, timestamp( $item->due_date ), $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, timestamp( $item->due_date ) ); > } else { >- $resp .= add_field( FID_DUE_DATE, q{}, $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, q{} ); > } > >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > if ( $protocol_version >= 2 ) { > if ( $ils->supports('security inhibit') ) { >- $resp .= add_field( FID_SECURITY_INHIBIT, $status->security_inhibit, $server ); >+ $resp .= add_field( $server, FID_SECURITY_INHIBIT, $status->security_inhibit ); > } >- $resp .= maybe_add( FID_MEDIA_TYPE, $item->sip_media_type, $server ); >- $resp .= maybe_add( FID_ITEM_PROPS, $item->sip_item_properties, $server ); >+ $resp .= maybe_add( $server, FID_MEDIA_TYPE, $item->sip_media_type ); >+ $resp .= maybe_add( $server, FID_ITEM_PROPS, $item->sip_item_properties ); > > } > } >@@ -580,30 +580,30 @@ sub handle_checkout { > # Checkout Response: not ok, no renewal, don't know mag. media, > # no desensitize > $resp = sprintf( "120NUN%s", timestamp ); >- $resp .= add_field( FID_INST_ID, $inst, $server ); >- $resp .= add_field( FID_PATRON_ID, $patron_id, $server ); >- $resp .= add_field( FID_ITEM_ID, $item_id, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $inst ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron_id ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item_id ); > > # If the item is valid, provide the title, otherwise > # leave it blank >- $resp .= add_field( FID_TITLE_ID, $item ? $item->title_id : '', $server ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item ? $item->title_id : '' ); > > # Due date is required. Since it didn't get checked out, > # it's not due, so leave the date blank >- $resp .= add_field( FID_DUE_DATE, '', $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, '' ); > >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > if ( $protocol_version >= 2 ) { > > # Is the patron ID valid? >- $resp .= add_field( FID_VALID_PATRON, sipbool($patron), $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON, sipbool($patron) ); > > if ( $patron && exists( $fields->{FID_PATRON_PWD} ) ) { > > # Password provided, so we can tell if it was valid or not >- $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $patron->check_password( $fields->{ (FID_PATRON_PWD) } ) ), $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON_PWD, sipbool( $patron->check_password( $fields->{ (FID_PATRON_PWD) } ) ) ); > } > } > } >@@ -612,10 +612,10 @@ sub handle_checkout { > > # Financials : return irrespective of ok status > if ( $status->fee_amount ) { >- $resp .= add_field( FID_FEE_AMT, $status->fee_amount, $server ); >- $resp .= maybe_add( FID_CURRENCY, $status->sip_currency, $server ); >- $resp .= maybe_add( FID_FEE_TYPE, $status->sip_fee_type, $server ); >- $resp .= maybe_add( FID_TRANSACTION_ID, $status->transaction_id, $server ); >+ $resp .= add_field( $server, FID_FEE_AMT, $status->fee_amount ); >+ $resp .= maybe_add( $server, FID_CURRENCY, $status->sip_currency ); >+ $resp .= maybe_add( $server, FID_FEE_TYPE, $status->sip_fee_type ); >+ $resp .= maybe_add( $server, FID_TRANSACTION_ID, $status->transaction_id ); > } > } > >@@ -669,27 +669,27 @@ sub handle_checkin { > > $resp .= $status->alert ? 'Y' : 'N'; > $resp .= timestamp; >- $resp .= add_field( FID_INST_ID, $inst_id, $server ); >- $resp .= add_field( FID_ITEM_ID, $item_id, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $inst_id ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item_id ); > > if ($item) { >- $resp .= add_field( FID_PERM_LOCN, $item->permanent_location, $server ); >- $resp .= maybe_add( FID_TITLE_ID, $item->title_id, $server ); >+ $resp .= add_field( $server, FID_PERM_LOCN, $item->permanent_location ); >+ $resp .= maybe_add( $server, FID_TITLE_ID, $item->title_id ); > } > > if ( $protocol_version >= 2 ) { >- $resp .= maybe_add( FID_SORT_BIN, $status->sort_bin, $server ); >+ $resp .= maybe_add( $server, FID_SORT_BIN, $status->sort_bin ); > if ($patron) { >- $resp .= add_field( FID_PATRON_ID, $patron->id, $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron->id ); > } > if ($item) { >-$resp .= maybe_add( FID_MEDIA_TYPE, $item->sip_media_type, $server ); >-$resp .= maybe_add( FID_ITEM_PROPS, $item->sip_item_properties, $server ); >-$resp .= maybe_add( FID_COLLECTION_CODE, $item->collection_code, $server ); >-$resp .= maybe_add( FID_CALL_NUMBER, $item->call_number, $server ); >-$resp .= maybe_add( FID_HOLD_PATRON_ID, $item->hold_patron_bcode, $server ); >-$resp .= add_field( FID_DESTINATION_LOCATION, $item->destination_loc, $server ) if ( $item->destination_loc || $server->{account}->{ct_always_send} ); >-$resp .= maybe_add( FID_HOLD_PATRON_NAME, $item->hold_patron_name( $server->{account}->{da_field_template} ), $server ); >+$resp .= maybe_add( $server, FID_MEDIA_TYPE, $item->sip_media_type ); >+$resp .= maybe_add( $server, FID_ITEM_PROPS, $item->sip_item_properties ); >+$resp .= maybe_add( $server, FID_COLLECTION_CODE, $item->collection_code ); >+$resp .= maybe_add( $server, FID_CALL_NUMBER, $item->call_number ); >+$resp .= maybe_add( $server, FID_HOLD_PATRON_ID, $item->hold_patron_bcode ); >+$resp .= add_field( $server, FID_DESTINATION_LOCATION, $item->destination_loc ) if ( $item->destination_loc || $server->{account}->{ct_always_send} ); >+$resp .= maybe_add( $server, FID_HOLD_PATRON_NAME, $item->hold_patron_name( $server->{account}->{da_field_template} ) ); > > if ( $status->hold and $status->hold->{branchcode} ne $item->destination_loc ) { > warn 'SIP hold mismatch: $status->hold->{branchcode}=' . $status->hold->{branchcode} . '; $item->destination_loc=' . $item->destination_loc; >@@ -700,12 +700,12 @@ $resp .= maybe_add( FID_HOLD_PATRON_NAME, $item->hold_patron_name( $server-> > } > > if ( $status->alert && $status->alert_type ) { >- $resp .= maybe_add( FID_ALERT_TYPE, $status->alert_type, $server ); >+ $resp .= maybe_add( $server, FID_ALERT_TYPE, $status->alert_type ); > } elsif ( $server->{account}->{cv_send_00_on_success} ) { >- $resp .= add_field( FID_ALERT_TYPE, '00', $server ); >+ $resp .= add_field( $server, FID_ALERT_TYPE, '00' ); > } >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -751,7 +751,7 @@ sub handle_block_patron { > $patron->block( $card_retained, $blocked_card_msg ); > } > >- $resp = build_patron_status( $patron, $patron->language, $fields, $server ); >+ $resp = build_patron_status( $patron, $patron->language, $fields ); > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > return (BLOCK_PATRON); > } >@@ -930,7 +930,7 @@ sub summary_info { > > syslog( "LOG_DEBUG", "summary_info: list = (%s)", join( ", ", map{ $_->{barcode} } @{$itemlist} ) ); > foreach my $i ( @{$itemlist} ) { >- $resp .= add_field( $fid, $i->{barcode}, $server ); >+ $resp .= add_field( $server, $fid, $i->{barcode} ); > } > > return $resp; >@@ -967,19 +967,19 @@ sub handle_patron_info { > $resp .= add_count( 'patron_info/recall_items', scalar @{ $patron->recall_items } ); > $resp .= add_count( 'patron_info/unavail_holds', scalar @{ $patron->unavail_holds } ); > >- $resp .= add_field( FID_INST_ID, ( $ils->institution_id || 'SIP2' ), $server ); >+ $resp .= add_field( $server, FID_INST_ID, ( $ils->institution_id || 'SIP2' ) ); > > # 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 ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron->id ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ) ); > > # TODO: add code for the fields > # hold items limit > # overdue items limit > # charged items limit > >- $resp .= add_field( FID_VALID_PATRON, 'Y', $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON, 'Y' ); > my $password_rc; > if ( defined($patron_pwd) ) { > >@@ -989,12 +989,12 @@ sub handle_patron_info { > } else { > $password_rc = $patron->check_password($patron_pwd); > } >- $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $password_rc ), $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON_PWD, sipbool( $password_rc ) ); > } > >- $resp .= maybe_add( FID_CURRENCY, $patron->currency, $server ); >- $resp .= maybe_add( FID_FEE_AMT, $patron->fee_amount, $server ); >- $resp .= add_field( FID_FEE_LMT, $patron->fee_limit, $server ); >+ $resp .= maybe_add( $server, FID_CURRENCY, $patron->currency ); >+ $resp .= maybe_add( $server, FID_FEE_AMT, $patron->fee_amount ); >+ $resp .= add_field( $server, FID_FEE_LMT, $patron->fee_limit ); > > # TODO: zero or more item details for 2.0 can go here: > # hold_items >@@ -1005,27 +1005,27 @@ sub handle_patron_info { > > $resp .= summary_info( $ils, $patron, $summary, $start, $end, $server ); > >- $resp .= maybe_add( FID_HOME_ADDR, $patron->address, $server ); >- $resp .= maybe_add( FID_EMAIL, $patron->email_addr, $server ); >- $resp .= maybe_add( FID_HOME_PHONE, $patron->home_phone, $server ); >+ $resp .= maybe_add( $server, FID_HOME_ADDR, $patron->address ); >+ $resp .= maybe_add( $server, FID_EMAIL, $patron->email_addr ); >+ $resp .= maybe_add( $server, FID_HOME_PHONE, $patron->home_phone ); > > # SIP 2.0 extensions used by Envisionware > # Other terminals will ignore unrecognized fields (unrecognized field identifiers) >- $resp .= maybe_add( FID_PATRON_BIRTHDATE, $patron->birthdate, $server ); >- $resp .= maybe_add( FID_PATRON_CLASS, $patron->ptype, $server ); >+ $resp .= maybe_add( $server, FID_PATRON_BIRTHDATE, $patron->birthdate ); >+ $resp .= maybe_add( $server, FID_PATRON_CLASS, $patron->ptype ); > > # Custom protocol extension to report patron internet privileges >- $resp .= maybe_add( FID_INET_PROFILE, $patron->inet_privileges, $server ); >+ $resp .= maybe_add( $server, FID_INET_PROFILE, $patron->inet_privileges ); > > my $msg = $patron->screen_msg; > if( defined( $patron_pwd ) && !$password_rc ) { > $msg .= ' -- ' . INVALID_PW; > } >- $resp .= maybe_add( FID_SCREEN_MSG, $msg, $server, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $msg, $server ); > if ( $server->{account}->{send_patron_home_library_in_af} ) { >- $resp .= maybe_add( FID_SCREEN_MSG, $patron->{branchcode}, $server); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $patron->{branchcode}, $server); > } >- $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $patron->print_line ); > > $resp .= $patron->build_patron_attributes_string( $server ); > } else { >@@ -1036,16 +1036,16 @@ sub handle_patron_info { > $resp .= 'YYYY' . ( ' ' x 10 ) . $lang . timestamp(); > $resp .= '0000' x 6; > >- $resp .= add_field( FID_INST_ID, ( $ils->institution_id || 'SIP2' ), $server ); >+ $resp .= add_field( $server, FID_INST_ID, ( $ils->institution_id || 'SIP2' ) ); > > # patron ID is invalid, but field is required, so just echo it back >- $resp .= add_field( FID_PATRON_ID, $fields->{ (FID_PATRON_ID) }, $server ); >- $resp .= add_field( FID_PERSONAL_NAME, '', $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $fields->{ (FID_PATRON_ID) } ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, '' ); > > if ( $protocol_version >= 2 ) { >- $resp .= add_field( FID_VALID_PATRON, 'N', $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON, 'N' ); > } >- $resp .= maybe_add( FID_SCREEN_MSG, INVALID_CARD, $server, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, INVALID_CARD, $server ); > } > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); >@@ -1069,11 +1069,11 @@ sub handle_end_patron_session { > $resp .= $status ? 'Y' : 'N'; > $resp .= timestamp(); > >- $resp .= add_field( FID_INST_ID, $server->{ils}->institution, $server ); >- $resp .= add_field( FID_PATRON_ID, $fields->{ (FID_PATRON_ID) }, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $server->{ils}->institution ); >+ $resp .= add_field( $server, FID_PATRON_ID, $fields->{ (FID_PATRON_ID) } ); > >- $resp .= maybe_add( FID_SCREEN_MSG, $screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $screen_msg ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1147,11 +1147,11 @@ sub handle_fee_paid { > } > > $resp .= ( $status->ok ? 'Y' : 'N' ) . timestamp; >- $resp .= add_field( FID_INST_ID, $inst_id, $server ); >- $resp .= add_field( FID_PATRON_ID, $patron_id, $server ); >- $resp .= maybe_add( FID_TRANSACTION_ID, $status->transaction_id, $server ); >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $inst_id ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron_id ); >+ $resp .= maybe_add( $server, FID_TRANSACTION_ID, $status->transaction_id ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1181,10 +1181,10 @@ sub handle_item_information { > $resp .= timestamp; > > # Just echo back the invalid item id >- $resp .= add_field( FID_ITEM_ID, $fields->{ (FID_ITEM_ID) }, $server ); >+ $resp .= add_field( $server, FID_ITEM_ID, $fields->{ (FID_ITEM_ID) } ); > > # title id is required, but we don't have one >- $resp .= add_field( FID_TITLE_ID, '', $server ); >+ $resp .= add_field( $server, FID_TITLE_ID, '' ); > } else { > > # Valid Item ID, send the good stuff >@@ -1193,35 +1193,35 @@ sub handle_item_information { > $resp .= $item->sip_fee_type; > $resp .= timestamp; > >- $resp .= add_field( FID_ITEM_ID, $item->id, $server ); >- $resp .= add_field( FID_TITLE_ID, $item->title_id, $server ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item->id ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item->title_id ); > >- $resp .= maybe_add( FID_MEDIA_TYPE, $item->sip_media_type, $server ); >- $resp .= maybe_add( FID_PERM_LOCN, $item->permanent_location, $server ); >- $resp .= maybe_add( FID_CURRENT_LOCN, $item->current_location, $server ); >- $resp .= maybe_add( FID_ITEM_PROPS, $item->sip_item_properties, $server ); >+ $resp .= maybe_add( $server, FID_MEDIA_TYPE, $item->sip_media_type ); >+ $resp .= maybe_add( $server, FID_PERM_LOCN, $item->permanent_location ); >+ $resp .= maybe_add( $server, FID_CURRENT_LOCN, $item->current_location ); >+ $resp .= maybe_add( $server, FID_ITEM_PROPS, $item->sip_item_properties ); > > if ( ( $i = $item->fee ) != 0 ) { >- $resp .= add_field( FID_CURRENCY, $item->fee_currency, $server ); >- $resp .= add_field( FID_FEE_AMT, $i, $server ); >+ $resp .= add_field( $server, FID_CURRENCY, $item->fee_currency ); >+ $resp .= add_field( $server, FID_FEE_AMT, $i ); > } >- $resp .= maybe_add( FID_OWNER, $item->owner, $server ); >+ $resp .= maybe_add( $server, FID_OWNER, $item->owner ); > > if ( ( $i = scalar @{ $item->hold_queue } ) > 0 ) { >- $resp .= add_field( FID_HOLD_QUEUE_LEN, $i, $server ); >+ $resp .= add_field( $server, FID_HOLD_QUEUE_LEN, $i ); > } > if ( $item->due_date ) { >- $resp .= add_field( FID_DUE_DATE, timestamp( $item->due_date ), $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, timestamp( $item->due_date ) ); > } > if ( ( $i = $item->recall_date ) != 0 ) { >- $resp .= add_field( FID_RECALL_DATE, timestamp($i), $server ); >+ $resp .= add_field( $server, FID_RECALL_DATE, timestamp($i) ); > } > if ( ( $i = $item->hold_pickup_date ) != 0 ) { >- $resp .= add_field( FID_HOLD_PICKUP_DATE, timestamp($i), $server ); >+ $resp .= add_field( $server, FID_HOLD_PICKUP_DATE, timestamp($i) ); > } > >- $resp .= maybe_add( FID_SCREEN_MSG, $item->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $item->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $item->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $item->print_line ); > } > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); >@@ -1256,7 +1256,7 @@ sub handle_item_status_update { > # Invalid Item ID > $resp .= '0'; > $resp .= timestamp; >- $resp .= add_field( FID_ITEM_ID, $item_id, $server ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item_id ); > } else { > > # Valid Item ID >@@ -1266,13 +1266,13 @@ sub handle_item_status_update { > $resp .= $status->ok ? '1' : '0'; > $resp .= timestamp; > >- $resp .= add_field( FID_ITEM_ID, $item->id, $server ); >- $resp .= add_field( FID_TITLE_ID, $item->title_id, $server ); >- $resp .= maybe_add( FID_ITEM_PROPS, $item->sip_item_properties, $server ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item->id ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item->title_id ); >+ $resp .= maybe_add( $server, FID_ITEM_PROPS, $item->sip_item_properties ); > } > >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1299,10 +1299,10 @@ sub handle_patron_enable { > > # Invalid patron ID > $resp .= 'YYYY' . ( ' ' x 10 ) . '000' . timestamp(); >- $resp .= add_field( FID_PATRON_ID, $patron_id, $server ); >- $resp .= add_field( FID_PERSONAL_NAME, '', $server ); >- $resp .= add_field( FID_VALID_PATRON, 'N', $server ); >- $resp .= add_field( FID_VALID_PATRON_PWD, 'N', $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron_id ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, '' ); >+ $resp .= add_field( $server, FID_VALID_PATRON, 'N' ); >+ $resp .= add_field( $server, FID_VALID_PATRON_PWD, 'N' ); > } else { > > # valid patron >@@ -1314,17 +1314,17 @@ sub handle_patron_enable { > $resp .= patron_status_string($patron); > $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( $server, FID_PATRON_ID, $patron->id ); >+ $resp .= add_field( $server, FID_PERSONAL_NAME, $patron->name( $server->{account}->{ae_field_template} ) ); > if ( defined($patron_pwd) ) { >- $resp .= add_field( FID_VALID_PATRON_PWD, sipbool( $patron->check_password($patron_pwd) ), $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON_PWD, sipbool( $patron->check_password($patron_pwd) ) ); > } >- $resp .= add_field( FID_VALID_PATRON, 'Y', $server ); >- $resp .= maybe_add( FID_SCREEN_MSG, $patron->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $patron->print_line, $server ); >+ $resp .= add_field( $server, FID_VALID_PATRON, 'Y' ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $patron->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $patron->print_line ); > } > >- $resp .= add_field( FID_INST_ID, $ils->institution, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $ils->institution ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1371,23 +1371,23 @@ sub handle_hold { > $resp .= timestamp; > > if ( $status->ok ) { >- $resp .= add_field( FID_PATRON_ID, $status->patron->id, $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $status->patron->id ); > > ( $status->expiration_date ) >- and $resp .= maybe_add( FID_EXPIRATION, timestamp( $status->expiration_date ), $server ); >- $resp .= maybe_add( FID_QUEUE_POS, $status->queue_position, $server ); >- $resp .= maybe_add( FID_PICKUP_LOCN, $status->pickup_location, $server ); >- $resp .= maybe_add( FID_ITEM_ID, $status->item->id, $server ); >- $resp .= maybe_add( FID_TITLE_ID, $status->item->title_id, $server ); >+ and $resp .= maybe_add( $server, FID_EXPIRATION, timestamp( $status->expiration_date ) ); >+ $resp .= maybe_add( $server, FID_QUEUE_POS, $status->queue_position ); >+ $resp .= maybe_add( $server, FID_PICKUP_LOCN, $status->pickup_location ); >+ $resp .= maybe_add( $server, FID_ITEM_ID, $status->item->id ); >+ $resp .= maybe_add( $server, FID_TITLE_ID, $status->item->title_id ); > } else { > > # Not ok. still need required fields >- $resp .= add_field( FID_PATRON_ID, $patron_id, $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron_id ); > } > >- $resp .= add_field( FID_INST_ID, $ils->institution, $server ); >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $ils->institution ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1434,19 +1434,19 @@ sub handle_renew { > } > $resp .= sipbool( $status->desensitize ); > $resp .= timestamp; >- $resp .= add_field( FID_PATRON_ID, $patron->id, $server ); >- $resp .= add_field( FID_ITEM_ID, $item->id, $server ); >- $resp .= add_field( FID_TITLE_ID, $item->title_id, $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron->id ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item->id ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item->title_id ); > if ( $item->due_date ) { >- $resp .= add_field( FID_DUE_DATE, timestamp( $item->due_date ), $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, timestamp( $item->due_date ) ); > } else { >- $resp .= add_field( FID_DUE_DATE, q{}, $server ); >+ $resp .= add_field( $server, FID_DUE_DATE, q{} ); > } > if ( $ils->supports('security inhibit') ) { >- $resp .= add_field( FID_SECURITY_INHIBIT, $status->security_inhibit, $server ); >+ $resp .= add_field( $server, FID_SECURITY_INHIBIT, $status->security_inhibit ); > } >- $resp .= add_field( FID_MEDIA_TYPE, $item->sip_media_type, $server ); >- $resp .= maybe_add( FID_ITEM_PROPS, $item->sip_item_properties, $server ); >+ $resp .= add_field( $server, FID_MEDIA_TYPE, $item->sip_media_type ); >+ $resp .= maybe_add( $server, FID_ITEM_PROPS, $item->sip_item_properties ); > } else { > > # renew failed for some reason >@@ -1457,22 +1457,22 @@ sub handle_renew { > # If we found the patron or the item, the return the ILS > # information, otherwise echo back the information we received > # from the terminal >- $resp .= add_field( FID_PATRON_ID, $patron ? $patron->id : $patron_id, $server ); >- $resp .= add_field( FID_ITEM_ID, $item ? $item->id : $item_id, $server ); >- $resp .= add_field( FID_TITLE_ID, $item ? $item->title_id : $title_id, $server ); >- $resp .= add_field( FID_DUE_DATE, '', $server ); >+ $resp .= add_field( $server, FID_PATRON_ID, $patron ? $patron->id : $patron_id ); >+ $resp .= add_field( $server, FID_ITEM_ID, $item ? $item->id : $item_id ); >+ $resp .= add_field( $server, FID_TITLE_ID, $item ? $item->title_id : $title_id ); >+ $resp .= add_field( $server, FID_DUE_DATE, '' ); > } > > if ( $status->fee_amount ) { >- $resp .= add_field( FID_FEE_AMT, $status->fee_amount, $server ); >- $resp .= maybe_add( FID_CURRENCY, $status->sip_currency, $server ); >- $resp .= maybe_add( FID_FEE_TYPE, $status->sip_fee_type, $server ); >- $resp .= maybe_add( FID_TRANSACTION_ID, $status->transaction_id, $server ); >+ $resp .= add_field( $server, FID_FEE_AMT, $status->fee_amount ); >+ $resp .= maybe_add( $server, FID_CURRENCY, $status->sip_currency ); >+ $resp .= maybe_add( $server, FID_FEE_TYPE, $status->sip_fee_type ); >+ $resp .= maybe_add( $server, FID_TRANSACTION_ID, $status->transaction_id ); > } > >- $resp .= add_field( FID_INST_ID, $ils->institution, $server ); >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $ils->institution ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1517,13 +1517,13 @@ sub handle_renew_all { > } > > $resp .= timestamp; >- $resp .= add_field( FID_INST_ID, $ils->institution, $server ); >+ $resp .= add_field( $server, FID_INST_ID, $ils->institution ); > >- $resp .= join( '', map( add_field( FID_RENEWED_ITEMS, $_ ), @renewed ), $server ); >- $resp .= join( '', map( add_field( FID_UNRENEWED_ITEMS, $_ ), @unrenewed ), $server ); >+ $resp .= join( '', map( add_field( $server, FID_RENEWED_ITEMS, $_ ), @renewed ) ); >+ $resp .= join( '', map( add_field( $server, FID_UNRENEWED_ITEMS, $_ ), @unrenewed ) ); > >- $resp .= maybe_add( FID_SCREEN_MSG, $status->screen_msg, $server, $server ); >- $resp .= maybe_add( FID_PRINT_LINE, $status->print_line, $server ); >+ $resp .= maybe_add( $server, FID_SCREEN_MSG, $status->screen_msg, $server ); >+ $resp .= maybe_add( $server, FID_PRINT_LINE, $status->print_line ); > > $self->write_msg( $resp, undef, $server->{account}->{terminator}, $server->{account}->{encoding} ); > >@@ -1594,7 +1594,7 @@ sub send_acs_status { > } > > # Institution ID >- $msg .= add_field( FID_INST_ID, $account->{institution}, $server ); >+ $msg .= add_field( $server, FID_INST_ID, $account->{institution} ); > > if ( $protocol_version >= 2 ) { > >@@ -1611,10 +1611,10 @@ sub send_acs_status { > if ( length($supported_msgs) < 16 ) { > syslog( "LOG_ERR", 'send_acs_status: supported messages "%s" too short', $supported_msgs ); > } >- $msg .= add_field( FID_SUPPORTED_MSGS, $supported_msgs, $server ); >+ $msg .= add_field( $server, FID_SUPPORTED_MSGS, $supported_msgs ); > } > >- $msg .= maybe_add( FID_SCREEN_MSG, $screen_msg, $server, $server ); >+ $msg .= maybe_add( $server, FID_SCREEN_MSG, $screen_msg, $server ); > > if ( defined( $account->{print_width} ) > && defined($print_line) >@@ -1623,7 +1623,7 @@ sub send_acs_status { > $print_line = substr( $print_line, 0, $account->{print_width} ); > } > >- $msg .= maybe_add( FID_PRINT_LINE, $print_line, $server ); >+ $msg .= maybe_add( $server, FID_PRINT_LINE, $print_line ); > > # Do we want to tell the terminal its location? > >-- >2.24.1 (Apple Git-126)
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 24966
:
101559
|
101632
|
102584
|
102585
|
102689
|
102697
|
102698