Bug 19080: Handle non-existing patrons gratefully

This is a recurrent bug we have over the last years. When a script is
called with non-existent borrowernumber it will crashes.
We need to handle this gracefully instead of letting the script crashes.

On bug 18403 a new subroutine is added to the codebase
(output_and_exit_if_error) to handle this kind of errors correctly.
Since it is not pushed yet, I propose to just redirect to a script that
handle it correctly (circulation.pl) instead of adding this message to
all these scripts.

Test plan:
Hit different scripts from the members module and pass a non-existent
borrowernumber.
You must be redirected to circulation.pl with a friendly message.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

circ/circulation.pl

@@ -261,8 +261,8 @@ if ($findborrower) {
 }
 
 # get the borrower information.....
-if ($borrowernumber) {
-    $patron = Koha::Patrons->find( $borrowernumber );
+$patron ||= Koha::Patrons->find( $borrowernumber ) if $borrowernumber;
+if ($patron) {
     my $overdues = $patron->get_overdues;
     my $issues = $patron->checkouts;
     my $balance = $patron->account->balance;
@@ -443,8 +443,8 @@ if (@$barcodes) {
 ##################################################################################
 # BUILD HTML
 # show all reserves of this borrower, and the position of the reservation ....
-if ($borrowernumber) {
-    my $holds = Koha::Holds->search( { borrowernumber => $borrowernumber } );
+if ($patron) {
+    my $holds = Koha::Holds->search( { borrowernumber => $borrowernumber } ); # FIXME must be Koha::Patron->holds
     my $waiting_holds = $holds->waiting;
     $template->param(
         holds_count  => $holds->count(),

koha-tmpl/intranet-tmpl/prog/en/modules/members/statistics.tt

@@ -3,11 +3,7 @@
 [% USE Branches %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Patrons &rsaquo;
-[% IF ( unknowuser ) %]
-    Patron does not exist
-[% ELSE %]
-    Statistics for [% INCLUDE 'patron-title.inc' %]
-[% END %]
+Statistics for [% INCLUDE 'patron-title.inc' %]
 </title>
 [% INCLUDE 'doc-head-close.inc' %]
 <link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/datatables.css" />
@@ -30,7 +26,7 @@
 <div id="breadcrumbs">
          <a href="/cgi-bin/koha/mainpage.pl">Home</a>
 &rsaquo; <a href="/cgi-bin/koha/members/members-home.pl">Patrons</a>
-&rsaquo; [% IF ( unknowuser ) %]Patron does not exist[% ELSE %]Statistics for [% firstname %] [% surname %] ([% cardnumber %])[% END %]
+&rsaquo; Statistics for [% firstname %] [% surname %] ([% cardnumber %])
 </div>
 
 <div id="doc3" class="yui-t1">

members/boraccount.pl

@@ -54,6 +54,10 @@ my $action = $input->param('action') || '';
 
 #get patron details
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 
 if ( $action eq 'reverse' ) {
   ReversePayment( $input->param('accountlines_id') );

members/deletemem.pl

@@ -75,6 +75,10 @@ my $issues = GetPendingIssues($member);     # FIXME: wasteful call when really,
 my $countissues = scalar(@$issues);
 
 my $patron = Koha::Patrons->find( $member );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $flags = C4::Members::patronflags( $patron->unblessed );
 my $userenv = C4::Context->userenv;
 

members/discharge.pl

@@ -58,86 +58,87 @@ unless ( C4::Context->preference('useDischarge') ) {
    exit;
 }
 
-if ( $input->param('borrowernumber') ) {
-    $borrowernumber = $input->param('borrowernumber');
+$borrowernumber = $input->param('borrowernumber');
 
-    # Getting member data
-    my $patron = Koha::Patrons->find( $borrowernumber );
+my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 
-    my $can_be_discharged = Koha::Patron::Discharge::can_be_discharged({
-        borrowernumber => $borrowernumber
-    });
+my $can_be_discharged = Koha::Patron::Discharge::can_be_discharged({
+    borrowernumber => $borrowernumber
+});
 
-    my $holds = $patron->holds;
-    my $has_reserves = $holds->count;
+my $holds = $patron->holds;
+my $has_reserves = $holds->count;
 
-    # Generating discharge if needed
-    if ( $input->param('discharge') and $can_be_discharged ) {
-        my $is_discharged = Koha::Patron::Discharge::is_discharged({
-            borrowernumber => $borrowernumber,
-        });
-        unless ($is_discharged) {
-            Koha::Patron::Discharge::discharge({
-                borrowernumber => $borrowernumber
-            });
-        }
-        eval {
-            my $pdf_path = Koha::Patron::Discharge::generate_as_pdf(
-                { borrowernumber => $borrowernumber, branchcode => $patron->branchcode } );
-
-            binmode(STDOUT);
-            print $input->header(
-                -type       => 'application/pdf',
-                -charset    => 'utf-8',
-                -attachment => "discharge_$borrowernumber.pdf",
-            );
-            open my $fh, '<', $pdf_path;
-            my @lines = <$fh>;
-            close $fh;
-            print @lines;
-            exit;
-        };
-        if ( $@ ) {
-            carp $@;
-            $template->param( messages => [ {type => 'error', code => 'unable_to_generate_pdf'} ] );
-        }
-    }
-
-    # Already generated discharges
-    my $validated_discharges = Koha::Patron::Discharge::get_validated({
+# Generating discharge if needed
+if ( $input->param('discharge') and $can_be_discharged ) {
+    my $is_discharged = Koha::Patron::Discharge::is_discharged({
         borrowernumber => $borrowernumber,
     });
+    unless ($is_discharged) {
+        Koha::Patron::Discharge::discharge({
+            borrowernumber => $borrowernumber
+        });
+    }
+    eval {
+        my $pdf_path = Koha::Patron::Discharge::generate_as_pdf(
+            { borrowernumber => $borrowernumber, branchcode => $patron->branchcode } );
 
-    $template->param( picture => 1 ) if $patron->image;
-
-    $template->param(
-        # FIXME The patron object should be passed to the template
-        borrowernumber    => $borrowernumber,
-        title             => $patron->title,
-        initials          => $patron->initials,
-        surname           => $patron->surname,
-        borrowernumber    => $borrowernumber,
-        firstname         => $patron->firstname,
-        cardnumber        => $patron->cardnumber,
-        categorycode      => $patron->categorycode,
-        category_type     => $patron->category->category_type,
-        categoryname      => $patron->category->description,
-        address           => $patron->address,
-        streetnumber      => $patron->streetnumber,
-        streettype        => $patron->streettype,
-        address2          => $patron->address2,
-        city              => $patron->city,
-        zipcode           => $patron->zipcode,
-        country           => $patron->country,
-        phone             => $patron->phone,
-        email             => $patron->email,
-        branchcode        => $patron->branchcode,
-        has_reserves      => $has_reserves,
-        can_be_discharged => $can_be_discharged,
-        validated_discharges => $validated_discharges,
-    );
+        binmode(STDOUT);
+        print $input->header(
+            -type       => 'application/pdf',
+            -charset    => 'utf-8',
+            -attachment => "discharge_$borrowernumber.pdf",
+        );
+        open my $fh, '<', $pdf_path;
+        my @lines = <$fh>;
+        close $fh;
+        print @lines;
+        exit;
+    };
+    if ( $@ ) {
+        carp $@;
+        $template->param( messages => [ {type => 'error', code => 'unable_to_generate_pdf'} ] );
+    }
 }
 
+# Already generated discharges
+my $validated_discharges = Koha::Patron::Discharge::get_validated({
+    borrowernumber => $borrowernumber,
+});
+
+$template->param( picture => 1 ) if $patron->image;
+
+$template->param(
+    # FIXME The patron object should be passed to the template
+    borrowernumber    => $borrowernumber,
+    title             => $patron->title,
+    initials          => $patron->initials,
+    surname           => $patron->surname,
+    borrowernumber    => $borrowernumber,
+    firstname         => $patron->firstname,
+    cardnumber        => $patron->cardnumber,
+    categorycode      => $patron->categorycode,
+    category_type     => $patron->category->category_type,
+    categoryname      => $patron->category->description,
+    address           => $patron->address,
+    streetnumber      => $patron->streetnumber,
+    streettype        => $patron->streettype,
+    address2          => $patron->address2,
+    city              => $patron->city,
+    zipcode           => $patron->zipcode,
+    country           => $patron->country,
+    phone             => $patron->phone,
+    email             => $patron->email,
+    branchcode        => $patron->branchcode,
+    has_reserves      => $has_reserves,
+    can_be_discharged => $can_be_discharged,
+    validated_discharges => $validated_discharges,
+);
+
 $template->param( dischargeview => 1, );
 
 output_html_with_http_headers $input, $cookie, $template->output;

members/files.pl

@@ -64,6 +64,11 @@ if ( $op eq 'download' ) {
 }
 else {
     my $patron = Koha::Patrons->find( $borrowernumber );
+    unless ( $patron ) {
+        print $cgi->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+        exit;
+    }
+
     my $patron_category = $patron->category;
     $template->param(%{ $patron->unblessed});
 

members/mancredit.pl

@@ -43,6 +43,10 @@ my $flagsrequired = { borrowers => 1, updatecharges => 1 };
 my $borrowernumber=$input->param('borrowernumber');
 
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $add=$input->param('add');
 
 if ($add){

members/maninvoice.pl

@@ -43,6 +43,11 @@ my $flagsrequired = { borrowers => 1 };
 my $borrowernumber=$input->param('borrowernumber');
 
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
+
 my $add=$input->param('add');
 if ($add){
     if ( checkauth( $input, 0, $flagsrequired, 'intranet' ) ) {

members/member-flags.pl

@@ -27,6 +27,11 @@ my $input = new CGI;
 my $flagsrequired = { permissions => 1 };
 my $member=$input->param('member');
 my $patron = Koha::Patrons->find( $member );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$member");
+    exit;
+}
+
 my $category_type = $patron->category->category_type;
 my $bor = $patron->unblessed;
 if( $category_type eq 'S' )  {

members/member-password.pl

@@ -49,6 +49,11 @@ my $newpassword2 = $input->param('newpassword2');
 my @errors;
 
 my $patron = Koha::Patrons->find( $member );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$member");
+    exit;
+}
+
 my $category_type = $patron->category->category_type;
 my $bor = $patron->unblessed;
 

members/memberentry.pl

@@ -155,6 +155,11 @@ $template->param( "duplicate" => 1 ) if ( $op eq 'duplicate' );
 $template->param( "checked" => 1 ) if ( defined($nodouble) && $nodouble eq 1 );
 if ( $op eq 'modify' or $op eq 'save' or $op eq 'duplicate' ) {
     my $patron = Koha::Patrons->find( $borrowernumber );
+    unless ( $patron ) {
+        print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+        exit;
+    }
+
     $borrower_data = $patron->unblessed;
     $borrower_data->{category_type} = $patron->category->category_type;
 }

members/notices.pl

@@ -34,6 +34,10 @@ my $input=new CGI;
 
 my $borrowernumber = $input->param('borrowernumber');
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $borrower = $patron->unblessed;
 
 my ($template, $loggedinuser, $cookie)

members/pay.pl

@@ -68,6 +68,10 @@ if ( !$borrowernumber ) {
 
 # get borrower details
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $category = $patron->category;
 our $borrower = $patron->unblessed;
 $borrower->{description} = $category->description;

members/paycollect.pl

@@ -50,6 +50,10 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 # get borrower details
 my $borrowernumber = $input->param('borrowernumber');
 my $patron         = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $borrower       = $patron->unblessed;
 my $category       = $patron->category;
 $borrower->{description} = $category->description;

members/printfeercpt.pl

@@ -51,6 +51,10 @@ my $action = $input->param('action') || '';
 my $accountlines_id = $input->param('accountlines_id');
 
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $category = $patron->category;
 my $data = $patron->unblessed;
 $data->{description} = $category->description;

members/printinvoice.pl

@@ -50,6 +50,10 @@ my $action          = $input->param('action') || '';
 my $accountlines_id = $input->param('accountlines_id');
 
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $category = $patron->category;
 my $data = $patron->unblessed;
 $data->{description} = $category->description;

members/purchase-suggestions.pl

@@ -42,8 +42,11 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 
 my $borrowernumber = $input->param('borrowernumber');
 
-# Set informations for the patron
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $category = $patron->category;
 my $data = $patron->unblessed;
 $data->{description} = $category->description;

members/readingrec.pl

@@ -55,15 +55,19 @@ my $patron;
 if ($input->param('cardnumber')) {
     $cardnumber = $input->param('cardnumber');
     $patron = Koha::Patrons->find( { cardnumber => $cardnumber } );
-    $data = $patron->unblessed;
-    $borrowernumber = $data->{'borrowernumber'}; # we must define this as it is used to retrieve other data about the patron
 }
 if ($input->param('borrowernumber')) {
     $borrowernumber = $input->param('borrowernumber');
     $patron = Koha::Patrons->find( $borrowernumber );
-    $data = $patron->unblessed;
 }
 
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
+$data = $patron->unblessed;
+$borrowernumber = $patron->borrowernumber;
+
 my $order = 'date_due desc';
 my $limit = 0;
 my $issues = ();

members/routing-lists.pl

@@ -48,37 +48,38 @@ my $borrowernumber = $query->param('borrowernumber');
 
 my $branch = C4::Context->userenv->{'branch'};
 
-# get the borrower information.....
-my ( $patron, $patron_info );
-if ($borrowernumber) {
-    $patron = Koha::Patrons->find( $borrowernumber );
-    my $category = $patron->category;
-    $patron_info = $patron->unblessed;
-    $patron_info->{description} = $category->description;
-    $patron_info->{category_type} = $category->category_type;
-
-  my $count;
-  my @borrowerSubscriptions;
-  ($count, @borrowerSubscriptions) = GetSubscriptionsFromBorrower($borrowernumber );
-  my @subscripLoop;
-
-    foreach my $num_res (@borrowerSubscriptions) {
-        my %getSubscrip;
-        $getSubscrip{subscriptionid}	= $num_res->{'subscriptionid'};
-        $getSubscrip{title}			= $num_res->{'title'};
-        $getSubscrip{borrowernumber}		= $num_res->{'borrowernumber'};
-        push( @subscripLoop, \%getSubscrip );
-    }
-
-    $template->param(
-        countSubscrip => scalar @subscripLoop,
-        subscripLoop  => \@subscripLoop,
-        routinglistview => 1
-    );
-
-    $template->param( adultborrower => 1 ) if ( $patron_info->{category_type} =~ /^(A|I)$/ );
+my $patron = Koha::Patrons->find( $borrowernumber ) if $borrowernumber;
+unless ( $patron ) {
+    print $query->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
 }
 
+my $category = $patron->category;
+my $patron_info = $patron->unblessed;
+$patron_info->{description} = $category->description;
+$patron_info->{category_type} = $category->category_type;
+
+my $count;
+my @borrowerSubscriptions;
+($count, @borrowerSubscriptions) = GetSubscriptionsFromBorrower($borrowernumber );
+my @subscripLoop;
+
+foreach my $num_res (@borrowerSubscriptions) {
+    my %getSubscrip;
+    $getSubscrip{subscriptionid} = $num_res->{'subscriptionid'};
+    $getSubscrip{title}          = $num_res->{'title'};
+    $getSubscrip{borrowernumber} = $num_res->{'borrowernumber'};
+    push( @subscripLoop, \%getSubscrip );
+}
+
+$template->param(
+    countSubscrip => scalar @subscripLoop,
+    subscripLoop  => \@subscripLoop,
+    routinglistview => 1
+);
+
+$template->param( adultborrower => 1 ) if ( $patron_info->{category_type} =~ /^(A|I)$/ );
+
 ##################################################################################
 
 $template->param(%$patron_info);

members/statistics.pl

@@ -50,8 +50,7 @@ my $borrowernumber = $input->param('borrowernumber');
 # Set informations for the patron
 my $patron = Koha::Patrons->find( $borrowernumber );
 unless ( $patron ) {
-    $template->param (unknowuser => 1);
-    output_html_with_http_headers $input, $cookie, $template->output;
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
     exit;
 }
 

members/summary-print.pl

@@ -44,6 +44,10 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 );
 
 my $patron = Koha::Patrons->find( $borrowernumber );
+unless ( $patron ) {
+    print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+    exit;
+}
 my $category = $patron->category;
 my $data = $patron->unblessed;
 $data->{description} = $category->description;

members/update-child.pl

@@ -73,6 +73,10 @@ if ( $op eq 'multi' ) {
 
 elsif ( $op eq 'update' ) {
     my $patron = Koha::Patrons->find( $borrowernumber );
+    unless ( $patron ) {
+        print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+        exit;
+    }
     my $member = $patron->unblessed;
     $member->{'guarantorid'}  = 0;
     $member->{'categorycode'} = $catcode;

tools/viewlog.pl

@@ -74,6 +74,10 @@ if ( $src eq 'circ' ) {
     use C4::Members::Attributes qw(GetBorrowerAttributes);
     my $borrowernumber = $object;
     my $patron = Koha::Patrons->find( $borrowernumber );
+    unless ( $patron ) {
+        print $input->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber");
+        exit;
+    }
     $template->param( picture => 1 ) if $patron->image;
     my $data = $patron->unblessed;