bug 9505 refactor loops in invoices.pl

- Remove an unnecessary loop where output just
  recreated input.
- Remove unnecessary temp variables that obscure code purpose.
- Call the variable containing invoices, invoices
  rather than anonymous and ambiguous results
  reflect namechange in template.
- Lists are passed to template as array refs;
  declare them as scalars as that is how we use them.
- No need to introduce the whole namespace of some C4
  modules for 1 routine.

Test plan:

Note that this patch should not change any visible behavior.

[1] Open the invoice search page.
[2] Verify that the list of suppliers in the drop-down
    on the search form is complete.
[3] Verify that the list of libraries in the drop-down
    on the saerch form is complete.
[4] Perform a search.  Verify that the list of invoices
    is correct.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/acqui/invoices.pl b/acqui/invoices.pl
index 44589ab..f30e374 100755 (executable)
--- a/acqui/invoices.pl
+++ b/acqui/invoices.pl
@@ -33,11 +33,11 @@ use CGI;
 use C4::Auth;
 use C4::Output;
 
-use C4::Acquisition;
+use C4::Acquisition qw/GetInvoices/;
 use C4::Bookseller qw/GetBookSeller/;
-use C4::Branch;
+use C4::Branch qw/GetBranches/;
 
-my $input = new CGI;
+my $input = CGI->new;
 my ( $template, $loggedinuser, $cookie, $flags ) = get_template_and_user(
     {
         template_name   => 'acqui/invoices.tmpl',
@@ -63,13 +63,13 @@ my $publicationyear  = $input->param('publicationyear');
 my $branch           = $input->param('branch');
 my $op               = $input->param('op');
 
-my @results_loop = ();
-if ( $op and $op eq "do_search" ) {
-    my $shipmentdatefrom_iso = C4::Dates->new($shipmentdatefrom)->output("iso");
-    my $shipmentdateto_iso   = C4::Dates->new($shipmentdateto)->output("iso");
-    my $billingdatefrom_iso  = C4::Dates->new($billingdatefrom)->output("iso");
-    my $billingdateto_iso    = C4::Dates->new($billingdateto)->output("iso");
-    my @invoices             = GetInvoices(
+my $invoices = [];
+if ( $op and $op eq 'do_search' ) {
+    my $shipmentdatefrom_iso = C4::Dates->new($shipmentdatefrom)->output('iso');
+    my $shipmentdateto_iso   = C4::Dates->new($shipmentdateto)->output('iso');
+    my $billingdatefrom_iso  = C4::Dates->new($billingdatefrom)->output('iso');
+    my $billingdateto_iso    = C4::Dates->new($billingdateto)->output('iso');
+    @{$invoices} = GetInvoices(
         invoicenumber    => $invoicenumber,
         supplierid       => $supplierid,
         shipmentdatefrom => $shipmentdatefrom_iso,
@@ -83,43 +83,29 @@ if ( $op and $op eq "do_search" ) {
         publicationyear  => $publicationyear,
         branchcode       => $branch
     );
-    foreach (@invoices) {
-        my %row = (
-            invoiceid       => $_->{invoiceid},
-            billingdate     => $_->{billingdate},
-            invoicenumber   => $_->{invoicenumber},
-            suppliername    => $_->{suppliername},
-            booksellerid      => $_->{booksellerid},
-            receivedbiblios => $_->{receivedbiblios},
-            receiveditems   => $_->{receiveditems},
-            subscriptionid  => $_->{subscriptionid},
-            closedate       => $_->{closedate},
-        );
-        push @results_loop, \%row;
-    }
 }
 
 # Build suppliers list
 my @suppliers      = GetBookSeller(undef);
-my @suppliers_loop = ();
+my $suppliers_loop = [];
 my $suppliername;
 foreach (@suppliers) {
     my $selected = 0;
-    if ( $supplierid && $supplierid == $_->{'id'} ) {
-        $selected     = 1;
-        $suppliername = $_->{'name'};
+    if ($supplierid && $supplierid == $_->{id} ) {
+        $selected = 1;
+        $suppliername = $_->{name};
     }
-    my %row = (
-        suppliername => $_->{'name'},
-        booksellerid   => $_->{'id'},
+    push @{$suppliers_loop},
+      {
+        suppliername => $_->{name},
+        booksellerid   => $_->{id},
         selected     => $selected,
-    );
-    push @suppliers_loop, \%row;
+      };
 }
 
 # Build branches list
 my $branches      = GetBranches();
-my @branches_loop = ();
+my $branches_loop = [];
 my $branchname;
 foreach ( sort keys %$branches ) {
     my $selected = 0;
@@ -127,31 +113,31 @@ foreach ( sort keys %$branches ) {
         $selected   = 1;
         $branchname = $branches->{$_}->{'branchname'};
     }
-    my %row = (
+    push @{$branches_loop},
+      {
         branchcode => $_,
-        branchname => $branches->{$_}->{'branchname'},
+        branchname => $branches->{$_}->{branchname},
         selected   => $selected,
-    );
-    push @branches_loop, \%row;
+      };
 }
 
 $template->param(
-    do_search => ( $op and $op eq "do_search" ) ? 1 : 0,
-    results_loop             => \@results_loop,
-    invoicenumber            => $invoicenumber,
-    booksellerid             => $supplierid,
-    suppliername             => $suppliername,
-    billingdatefrom          => $billingdatefrom,
-    billingdateto            => $billingdateto,
-    isbneanissn              => $isbneanissn,
-    title                    => $title,
-    author                   => $author,
-    publisher                => $publisher,
-    publicationyear          => $publicationyear,
-    branch                   => $branch,
-    branchname               => $branchname,
-    suppliers_loop           => \@suppliers_loop,
-    branches_loop            => \@branches_loop,
+    do_search => ( $op and $op eq 'do_search' ) ? 1 : 0,
+    invoices => $invoices,
+    invoicenumber   => $invoicenumber,
+    booksellerid    => $supplierid,
+    suppliername    => $suppliername,
+    billingdatefrom => $billingdatefrom,
+    billingdateto   => $billingdateto,
+    isbneanissn     => $isbneanissn,
+    title           => $title,
+    author          => $author,
+    publisher       => $publisher,
+    publicationyear => $publicationyear,
+    branch          => $branch,
+    branchname      => $branchname,
+    suppliers_loop  => $suppliers_loop,
+    branches_loop   => $branches_loop,
 );
 
 output_html_with_http_headers $input, $cookie, $template->output;

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt
index 90d7fe1..52d2ecc 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoices.tt
@@ -41,7 +41,7 @@ $(document).ready(function() {
     <div class="yui-b">
       <h1>Invoices</h1>
       [% IF ( do_search ) %]
-        [% IF ( results_loop ) %]
+        [% IF invoices %]
           <table id="resultst">
             <thead>
               <tr>
@@ -55,30 +55,30 @@ $(document).ready(function() {
               </tr>
             </thead>
             <tbody>
-              [% FOREACH result IN results_loop %]
+              [% FOREACH invoice IN invoices %]
                 <tr>
-                  <td><a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% result.invoiceid %]">[% result.invoicenumber %]</a></td>
-                  <td><a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% result.booksellerid %]">[% result.suppliername %]</a></td>
+                  <td><a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% invoice.invoiceid %]">[% invoice.invoicenumber %]</a></td>
+                  <td><a href="/cgi-bin/koha/acqui/supplier.pl?booksellerid=[% invoice.booksellerid %]">[% invoice.suppliername %]</a></td>
                   <td>
-                    [% IF (result.billingdate) %]
-                      [% result.billingdate | $KohaDates %]
+                    [% IF invoice.billingdate %]
+                      [% invoice.billingdate | $KohaDates %]
                     [% END %]
                   </td>
-                  <td>[% result.receivedbiblios %]</td>
-                  <td>[% result.receiveditems %]</td>
+                  <td>[% invoice.receivedbiblios %]</td>
+                  <td>[% invoice.receiveditems %]</td>
                   <td>
-                    [% IF ( result.closedate ) %]
-                      Closed on [% result.closedate | $KohaDates %]
+                    [% IF invoice.closedate %]
+                      Closed on [% invoice.closedate | $KohaDates %]
                     [% ELSE %]
                       Open
                     [% END %]
                   </td>
                   <td>
-                    <a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% result.invoiceid %]">Details</a> /
-                    [% IF ( result.closedate ) %]
-                      <a href="invoice.pl?op=reopen&amp;invoiceid=[% result.invoiceid %]&amp;referer=/cgi-bin/koha/acqui/invoices.pl%3Fop=do_search%26invoicenumber=[% invoicenumber %]%26supplier=[% supplier %]%26billingdatefrom=[% billingdatefrom %]%26billingdateto=[% billingdateto %]%26isbneanissn=[% isbneanissn %]%26title=[% title %]%26author=[% author %]%26publisher=[% publisher %]%26publicationyear=[% publicationyear %]%26branch=[% branch %]">Reopen</a>
+                    <a href="/cgi-bin/koha/acqui/invoice.pl?invoiceid=[% invoice.invoiceid %]">Details</a> /
+                    [% IF invoice.closedate %]
+                      <a href="invoice.pl?op=reopen&amp;invoiceid=[% invoice.invoiceid %]&amp;referer=/cgi-bin/koha/acqui/invoices.pl%3Fop=do_search%26invoicenumber=[% invoicenumber %]%26supplier=[% booksellerid %]%26billingdatefrom=[% billingdatefrom %]%26billingdateto=[% billingdateto %]%26isbneanissn=[% isbneanissn %]%26title=[% title %]%26author=[% author %]%26publisher=[% publisher %]%26publicationyear=[% publicationyear %]%26branch=[% branch %]">Reopen</a>
                     [% ELSE %]
-                      <a href="invoice.pl?op=close&amp;invoiceid=[% result.invoiceid %]&amp;referer=/cgi-bin/koha/acqui/invoices.pl%3Fop=do_search%26invoicenumber=[% invoicenumber %]%26supplier=[% supplier %]%26billingdatefrom=[% billingdatefrom %]%26billingdateto=[% billingdateto %]%26isbneanissn=[% isbneanissn %]%26title=[% title %]%26author=[% author %]%26publisher=[% publisher %]%26publicationyear=[% publicationyear %]%26branch=[% branch %]">Close</a>
+                      <a href="invoice.pl?op=close&amp;invoiceid=[% invoice.invoiceid %]&amp;referer=/cgi-bin/koha/acqui/invoices.pl%3Fop=do_search%26invoicenumber=[% invoicenumber %]%26supplier=[% booksellerid %]%26billingdatefrom=[% billingdatefrom %]%26billingdateto=[% billingdateto %]%26isbneanissn=[% isbneanissn %]%26title=[% title %]%26author=[% author %]%26publisher=[% publisher %]%26publicationyear=[% publicationyear %]%26branch=[% branch %]">Close</a>
                     [% END %]
                   </td>
                 </tr>
@@ -92,7 +92,7 @@ $(document).ready(function() {
               [% IF ( invoicenumber ) %]
                 <li>Invoice no.: [% invoicenumber %]</li>
               [% END %]
-              [% IF ( supplier ) %]
+              [% IF booksellerid %]
                 <li>Vendor: [% suppliername %]</li>
               [% END %]
               [% IF ( billingdatefrom ) %]
@@ -131,7 +131,7 @@ $(document).ready(function() {
               [% END %]
             </ul>
           </p>
-        [% END %]<!-- results_loop -->
+        [% END %]<!-- invoices -->
       [% ELSE %]
         <p>Use the search form on the left to find invoices.</p>
       [% END %]<!-- do_search -->