From 40c3f183d5ddb9ab49ed9b593a7989ae8c4b621b Mon Sep 17 00:00:00 2001 From: Jan Kissig Date: Mon, 2 Sep 2024 09:12:10 +0200 Subject: [PATCH] Bug 23426: (follow-up) Tidy --- C4/SIP/ILS/Patron.pm | 325 ++++++++++++++++++++++++------------------ C4/SIP/Sip/MsgType.pm | 29 +++- 2 files changed, 209 insertions(+), 145 deletions(-) diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm index 1483f0ebf9..6c7651d824 100644 --- a/C4/SIP/ILS/Patron.pm +++ b/C4/SIP/ILS/Patron.pm @@ -39,7 +39,7 @@ our $kp; # koha patron =cut sub new { - my ($class, $patron_id) = @_; + my ( $class, $patron_id ) = @_; my $type = ref($class) || $class; my $self; @@ -47,31 +47,41 @@ sub new { if ( ref $patron_id eq "HASH" ) { if ( $patron_id->{borrowernumber} ) { $patron = Koha::Patrons->find( $patron_id->{borrowernumber} ); - } elsif ( $patron_id->{cardnumber} ) { - $patron = Koha::Patrons->find( { cardnumber => $patron_id->{cardnumber} } ); - } elsif ( $patron_id->{userid} ) { + } + elsif ( $patron_id->{cardnumber} ) { + $patron = + Koha::Patrons->find( { cardnumber => $patron_id->{cardnumber} } ); + } + elsif ( $patron_id->{userid} ) { $patron = Koha::Patrons->find( { userid => $patron_id->{userid} } ); } - } else { + } + else { $patron = Koha::Patrons->find( { cardnumber => $patron_id } ) - || Koha::Patrons->find( { userid => $patron_id } ); + || Koha::Patrons->find( { userid => $patron_id } ); } unless ($patron) { - siplog("LOG_DEBUG", "new ILS::Patron(%s): no such patron", $patron_id); + siplog( "LOG_DEBUG", "new ILS::Patron(%s): no such patron", + $patron_id ); return; } $kp = $patron->unblessed; - my $pw = $kp->{password}; - my $flags = C4::Members::patronflags( $kp ); - my $debarred = $patron->is_debarred; - siplog( "LOG_DEBUG", "Debarred = %s : ", ( $debarred || 'undef' ) ); # Do we need more debug info here? + my $pw = $kp->{password}; + my $flags = C4::Members::patronflags($kp); + my $debarred = $patron->is_debarred; + siplog( "LOG_DEBUG", "Debarred = %s : ", ( $debarred || 'undef' ) ) + ; # Do we need more debug info here? my $expired = 0; - if ( $kp->{'dateexpiry'} && C4::Context->preference('NotifyBorrowerDeparture') ) { + if ( $kp->{'dateexpiry'} + && C4::Context->preference('NotifyBorrowerDeparture') ) + { my ( $today_year, $today_month, $today_day ) = Today(); - my ( $warning_year, $warning_month, $warning_day ) = split /-/, $kp->{'dateexpiry'}; - my $days_to_expiry = Date_to_Days( $warning_year, $warning_month, $warning_day ) - - Date_to_Days( $today_year, $today_month, $today_day ); + my ( $warning_year, $warning_month, $warning_day ) = split /-/, + $kp->{'dateexpiry'}; + my $days_to_expiry = + Date_to_Days( $warning_year, $warning_month, $warning_day ) - + Date_to_Days( $today_year, $today_month, $today_day ); my $dt = dt_from_string( $kp->{'dateexpiry'}, 'iso' ); my $dateexpiry = output_pref( { dt => $dt, dateonly => 1 } ); if ( $days_to_expiry < 0 ) { @@ -82,7 +92,10 @@ sub new { } $kp->{opacnote} .= "Your account has expired as of $dateexpiry"; $expired = 1; - } elsif ( $days_to_expiry < C4::Context->preference('NotifyBorrowerDeparture') ) { + } + elsif ( $days_to_expiry < + C4::Context->preference('NotifyBorrowerDeparture') ) + { # borrower card soon to expire, warn the borrower if ( $kp->{opacnote} ) { @@ -92,104 +105,127 @@ sub new { } } my %ilspatron; - my $adr = _get_address($kp); - my $dob = $kp->{dateofbirth}; - $dob and $dob =~ s/-//g; # YYYYMMDD - my $dexpiry = $kp->{dateexpiry}; + my $adr = _get_address($kp); + my $dob = $kp->{dateofbirth}; + $dob and $dob =~ s/-//g; # YYYYMMDD + my $dexpiry = $kp->{dateexpiry}; $dexpiry and $dexpiry =~ s/-//g; # YYYYMMDD - # Get fines and add fines for guarantees (depends on preference NoIssuesChargeGuarantees) - my $patron_charge_limits = $patron->is_patron_inside_charge_limits(); - my $fines_amount = $patron_charge_limits->{noissuescharge}->{charge}; +# Get fines and add fines for guarantees (depends on preference NoIssuesChargeGuarantees) + my $patron_charge_limits = $patron->is_patron_inside_charge_limits(); + my $fines_amount = $patron_charge_limits->{noissuescharge}->{charge}; my $personal_fines_amount = $fines_amount; - my $fee_limit = $patron_charge_limits->{noissuescharge}->{limit} || 5; + my $fee_limit = $patron_charge_limits->{noissuescharge}->{limit} || 5; my $noissueschargeguarantorswithguarantees = - $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; - my $noissueschargeguarantees = $patron_charge_limits->{NoIssuesChargeGuarantees}->{limit}; + $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{limit}; + my $noissueschargeguarantees = + $patron_charge_limits->{NoIssuesChargeGuarantees}->{limit}; - my $fines_msg = ""; + my $fines_msg = ""; my $fine_blocked = 0; if ( $patron_charge_limits->{noissuescharge}->{overlimit} ) { $fine_blocked = 1; $fines_msg .= " -- " . "Patron blocked by fines" if $fine_blocked; - } elsif ($noissueschargeguarantorswithguarantees) { - $fines_amount = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{charge}; - $fine_blocked = $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees}->{overlimit}; - $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on related accounts" if $fine_blocked; - } elsif ($noissueschargeguarantees) { + } + elsif ($noissueschargeguarantorswithguarantees) { + $fines_amount = + $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees} + ->{charge}; + $fine_blocked = + $patron_charge_limits->{NoIssuesChargeGuarantorsWithGuarantees} + ->{overlimit}; + $fines_msg .= " -- " + . "Patron blocked by fines ($fines_amount) on related accounts" + if $fine_blocked; + } + elsif ($noissueschargeguarantees) { if ( $patron->guarantee_relationships->count ) { - $fines_amount += $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}; - $fine_blocked = $patron_charge_limits->{NoIssuesChargeGuarantees}->{overlimit}; - $fines_msg .= " -- " . "Patron blocked by fines ($fines_amount) on guaranteed accounts" if $fine_blocked; + $fines_amount += + $patron_charge_limits->{NoIssuesChargeGuarantees}->{charge}; + $fine_blocked = + $patron_charge_limits->{NoIssuesChargeGuarantees}->{overlimit}; + $fines_msg .= " -- " + . "Patron blocked by fines ($fines_amount) on guaranteed accounts" + if $fine_blocked; } } # Get currency 3 chars max - my $currency = substr Koha::Acquisition::Currencies->get_active->currency, 0, 3; + my $currency = substr Koha::Acquisition::Currencies->get_active->currency, + 0, 3; - my $circ_blocked =( C4::Context->preference('OverduesBlockCirc') ne "noblock" && defined $flags->{ODUES}->{itemlist} ) ? 1 : 0; + my $circ_blocked = + ( C4::Context->preference('OverduesBlockCirc') ne "noblock" + && defined $flags->{ODUES}->{itemlist} ) ? 1 : 0; { - no warnings; # any of these $kp->{fields} being concat'd could be undef - %ilspatron = ( - name => $kp->{firstname} . " " . $kp->{surname}, - id => $kp->{cardnumber}, # to SIP, the id is the BARCODE, not userid - password => $pw, - ptype => $kp->{categorycode}, # 'A'dult. Whatever. - dateexpiry => $dexpiry, - dateexpiry_iso => $kp->{dateexpiry}, - birthdate => $dob, - birthdate_iso => $kp->{dateofbirth}, - branchcode => $kp->{branchcode}, - library_name => "", # only populated if needed, cached here - borrowernumber => $kp->{borrowernumber}, - address => $adr, - home_phone => $kp->{phone}, - email_addr => $kp->{email}, - charge_ok => ( !$debarred && !$expired && !$fine_blocked && !$circ_blocked), - renew_ok => ( !$debarred && !$expired && !$fine_blocked), - recall_ok => ( !$debarred && !$expired && !$fine_blocked), - hold_ok => ( !$debarred && !$expired && !$fine_blocked), - card_lost => ( $kp->{lost} || $kp->{gonenoaddress} || $flags->{LOST} ), - claims_returned => 0, - fines => $personal_fines_amount, - fees => 0, # currently not distinct from fines - recall_overdue => 0, - items_billed => 0, - screen_msg => 'Greetings from Koha. ' . $kp->{opacnote} . $fines_msg, - print_line => '', - items => [], - hold_items => $flags->{WAITING}->{itemlist}, - overdue_items => $flags->{ODUES}->{itemlist}, - too_many_overdue => $circ_blocked, - fine_items => [], - recall_items => [], - unavail_holds => [], - inet => ( !$debarred && !$expired ), - debarred => $debarred, - expired => $expired, - fine_blocked => $fine_blocked, - fee_limit => $fee_limit, - userid => $kp->{userid}, - currency => $currency, - ); + no warnings; # any of these $kp->{fields} being concat'd could be undef + %ilspatron = ( + name => $kp->{firstname} . " " . $kp->{surname}, + id => $kp->{cardnumber}, # to SIP, the id is the BARCODE, not userid + password => $pw, + ptype => $kp->{categorycode}, # 'A'dult. Whatever. + dateexpiry => $dexpiry, + dateexpiry_iso => $kp->{dateexpiry}, + birthdate => $dob, + birthdate_iso => $kp->{dateofbirth}, + branchcode => $kp->{branchcode}, + library_name => "", # only populated if needed, cached here + borrowernumber => $kp->{borrowernumber}, + address => $adr, + home_phone => $kp->{phone}, + email_addr => $kp->{email}, + charge_ok => ( + !$debarred && !$expired && !$fine_blocked && !$circ_blocked + ), + renew_ok => ( !$debarred && !$expired && !$fine_blocked ), + recall_ok => ( !$debarred && !$expired && !$fine_blocked ), + hold_ok => ( !$debarred && !$expired && !$fine_blocked ), + card_lost => + ( $kp->{lost} || $kp->{gonenoaddress} || $flags->{LOST} ), + claims_returned => 0, + fines => $personal_fines_amount, + fees => 0, # currently not distinct from fines + recall_overdue => 0, + items_billed => 0, + screen_msg => 'Greetings from Koha. ' + . $kp->{opacnote} + . $fines_msg, + print_line => '', + items => [], + hold_items => $flags->{WAITING}->{itemlist}, + overdue_items => $flags->{ODUES}->{itemlist}, + too_many_overdue => $circ_blocked, + fine_items => [], + recall_items => [], + unavail_holds => [], + inet => ( !$debarred && !$expired ), + debarred => $debarred, + expired => $expired, + fine_blocked => $fine_blocked, + fee_limit => $fee_limit, + userid => $kp->{userid}, + currency => $currency, + ); } if ( $patron->is_debarred and $patron->debarredcomment ) { $ilspatron{screen_msg} .= " -- " . $patron->debarredcomment; } - if ( $circ_blocked ) { + if ($circ_blocked) { $ilspatron{screen_msg} .= " -- " . "Patron has overdues"; } for (qw(EXPIRED CHARGES CREDITS GNA LOST NOTES)) { - ($flags->{$_}) or next; - if ($_ ne 'NOTES' and $flags->{$_}->{message}) { - $ilspatron{screen_msg} .= " -- " . $flags->{$_}->{message}; # show all but internal NOTES + ( $flags->{$_} ) or next; + if ( $_ ne 'NOTES' and $flags->{$_}->{message} ) { + $ilspatron{screen_msg} .= + " -- " . $flags->{$_}->{message}; # show all but internal NOTES } - if ($flags->{$_}->{noissues}) { + if ( $flags->{$_}->{noissues} ) { foreach my $toggle (qw(charge_ok renew_ok recall_ok hold_ok inet)) { - $ilspatron{$toggle} = 0; # if we get noissues, disable everything + $ilspatron{$toggle} = + 0; # if we get noissues, disable everything } } } @@ -203,25 +239,31 @@ sub new { ); my @messages_array; while ( my $message = $patron_messages->next ) { - my $messagedt = dt_from_string( $message->message_date, 'iso' ); - my $formatted_date = output_pref( { dt => $messagedt, dateonly => 1 } ); + my $messagedt = dt_from_string( $message->message_date, 'iso' ); + my $formatted_date = + output_pref( { dt => $messagedt, dateonly => 1 } ); push @messages_array, $formatted_date . ": " . $message->message; } if (@messages_array) { - $ilspatron{screen_msg} .= " Messages for you: " . join( ' / ', @messages_array ); + $ilspatron{screen_msg} .= + " Messages for you: " . join( ' / ', @messages_array ); } } # FIXME: populate recall_items - $ilspatron{unavail_holds} = _get_outstanding_holds($kp->{borrowernumber}); + $ilspatron{unavail_holds} = _get_outstanding_holds( $kp->{borrowernumber} ); # fine items my $debits = $patron->account->outstanding_debits; my @fine_items; while ( my $debit = $debits->next ) { - push @fine_items, { account_line => sprintf '%s %.02f', $debit->description, $debit->amountoutstanding }; + push @fine_items, + { + account_line => sprintf '%s %.02f', + $debit->description, $debit->amountoutstanding + }; } - $ilspatron{fine_items} =\@fine_items; + $ilspatron{fine_items} = \@fine_items; my $pending_checkouts = $patron->pending_checkouts; my @barcodes; @@ -231,48 +273,49 @@ sub new { $ilspatron{items} = \@barcodes; $self = \%ilspatron; - siplog("LOG_DEBUG", "new ILS::Patron(%s): found patron '%s'", $patron_id,$self->{id}); + siplog( "LOG_DEBUG", "new ILS::Patron(%s): found patron '%s'", + $patron_id, $self->{id} ); bless $self, $type; return $self; } - # 0 means read-only # 1 means read/write my %fields = ( - id => 0, - borrowernumber => 0, - name => 0, - address => 0, - email_addr => 0, - home_phone => 0, - birthdate => 0, - birthdate_iso => 0, - dateexpiry => 0, - dateexpiry_iso => 0, - debarred => 0, - fine_blocked => 0, - ptype => 0, - charge_ok => 0, # for patron_status[0] (inverted) - renew_ok => 0, # for patron_status[1] (inverted) - recall_ok => 0, # for patron_status[2] (inverted) - hold_ok => 0, # for patron_status[3] (inverted) - card_lost => 0, # for patron_status[4] - recall_overdue => 0, - currency => 1, - fee_limit => 0, - screen_msg => 1, - print_line => 1, - too_many_charged => 0, # for patron_status[5] - too_many_overdue => 0, # for patron_status[6] - too_many_renewal => 0, # for patron_status[7] - too_many_claim_return => 0, # for patron_status[8] -# excessive_fines => 0, # for patron_status[10] -# excessive_fees => 0, # for patron_status[11] - recall_overdue => 0, # for patron_status[12] - too_many_billed => 0, # for patron_status[13] - inet => 0, # EnvisionWare extension + id => 0, + borrowernumber => 0, + name => 0, + address => 0, + email_addr => 0, + home_phone => 0, + birthdate => 0, + birthdate_iso => 0, + dateexpiry => 0, + dateexpiry_iso => 0, + debarred => 0, + fine_blocked => 0, + ptype => 0, + charge_ok => 0, # for patron_status[0] (inverted) + renew_ok => 0, # for patron_status[1] (inverted) + recall_ok => 0, # for patron_status[2] (inverted) + hold_ok => 0, # for patron_status[3] (inverted) + card_lost => 0, # for patron_status[4] + recall_overdue => 0, + currency => 1, + fee_limit => 0, + screen_msg => 1, + print_line => 1, + too_many_charged => 0, # for patron_status[5] + too_many_overdue => 0, # for patron_status[6] + too_many_renewal => 0, # for patron_status[7] + too_many_claim_return => 0, # for patron_status[8] + + # excessive_fines => 0, # for patron_status[10] + # excessive_fees => 0, # for patron_status[11] + recall_overdue => 0, # for patron_status[12] + too_many_billed => 0, # for patron_status[13] + inet => 0, # EnvisionWare extension ); our $AUTOLOAD; @@ -401,30 +444,31 @@ sub drop_hold { sub x_items { my $self = shift; my $array_var = shift or return; - my ($start, $end) = @_; + my ( $start, $end ) = @_; my $item_list = []; - if ($self->{$array_var}) { + if ( $self->{$array_var} ) { - if ($start && $start > 1) { + if ( $start && $start > 1 ) { --$start; } else { $start = 0; } - if ( $end && $end < @{$self->{$array_var}} ) { - --$end; + if ( $end && $end < @{ $self->{$array_var} } ) { + --$end; } else { - $end = @{$self->{$array_var}}; + $end = @{ $self->{$array_var} }; --$end; } - @{$item_list} = @{$self->{$array_var}}[ $start .. $end ]; + @{$item_list} = @{ $self->{$array_var} }[ $start .. $end ]; } return $item_list; } + # # List of outstanding holds placed # @@ -440,25 +484,30 @@ sub hold_items { sub overdue_items { my $self = shift; - return $self->x_items('overdue_items', @_); + return $self->x_items( 'overdue_items', @_ ); } + sub charged_items { my $self = shift; - return $self->x_items('items', @_); + return $self->x_items( 'items', @_ ); } + sub fine_items { my $self = shift; - return $self->x_items('fine_items', @_); + return $self->x_items( 'fine_items', @_ ); } + sub recall_items { my $self = shift; - return $self->x_items('recall_items', @_); + return $self->x_items( 'recall_items', @_ ); } + sub unavail_holds { my $self = shift; - return $self->x_items('unavail_holds', @_); + return $self->x_items( 'unavail_holds', @_ ); } + sub block { my ($self, $card_retained, $blocked_card_msg) = @_; foreach my $field ('charge_ok', 'renew_ok', 'recall_ok', 'hold_ok', 'inet') { diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 18dc6cb3f8..038a91fe73 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -957,16 +957,23 @@ sub summary_info { { func => $patron->can("charged_items"), fid => FID_CHARGED_ITEMS }, { func => $patron->can("fine_items"), fid => FID_FINE_ITEMS }, { func => $patron->can("recall_items"), fid => FID_RECALL_ITEMS }, - { func => $patron->can("unavail_holds"), fid => FID_UNAVAILABLE_HOLD_ITEMS }, - { func => $patron->can("fine_items"), fid => FID_FINE_ITEMS }, + { + func => $patron->can("unavail_holds"), + fid => FID_UNAVAILABLE_HOLD_ITEMS + }, + { func => $patron->can("fine_items"), fid => FID_FINE_ITEMS }, ); my $summary_type = index( $summary, 'Y' ); return q{} if $summary_type == -1; # No detailed information required. - return q{} if $summary_type > 6; # Positions 7-9 are not defined in the sip spec, - # and we have no extensions to handle them. + return q{} + if $summary_type > 6; # Positions 7-9 are not defined in the sip spec, + # and we have no extensions to handle them. - siplog( "LOG_DEBUG", "Summary_info: index == '%d', field '%s'", $summary_type, $summary_map[$summary_type]->{fid} ); + siplog( + "LOG_DEBUG", "Summary_info: index == '%d', field '%s'", + $summary_type, $summary_map[$summary_type]->{fid} + ); my $func = $summary_map[$summary_type]->{func}; my $fid = $summary_map[$summary_type]->{fid}; @@ -974,13 +981,21 @@ sub summary_info { # fine items use account_line as key if ( $summary_type == 6 ) { - siplog( "LOG_DEBUG", "summary_info: list = (%s)", join( ", ", map { $_->{account_line} } @{$itemlist} ) ); + siplog( + "LOG_DEBUG", + "summary_info: list = (%s)", + join( ", ", map { $_->{account_line} } @{$itemlist} ) + ); foreach my $i ( @{$itemlist} ) { $resp .= add_field( $fid, $i->{account_line}, $server ); } } else { - siplog( "LOG_DEBUG", "summary_info: list = (%s)", join( ", ", map{ $_->{barcode} } @{$itemlist} ) ); + siplog( + "LOG_DEBUG", + "summary_info: list = (%s)", + join( ", ", map { $_->{barcode} } @{$itemlist} ) + ); foreach my $i ( @{$itemlist} ) { $resp .= add_field( $fid, $i->{barcode}, $server ); } -- 2.39.2