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>
This commit is contained in:
Colin Campbell 2013-01-30 08:50:23 +00:00 committed by Galen Charlton
parent a90a72c62d
commit e294defc29
2 changed files with 56 additions and 70 deletions

View File

@ -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;

View File

@ -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 -->