[Koha-patches] [PATCH] bug 9505 refactor loops in invoices.pl
Colin Campbell
colin.campbell at ptfs-europe.com
Wed Jan 30 12:03:59 CET 2013
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
---
acqui/invoices.pl | 83 +++++++++-------------
.../prog/en/modules/acqui/invoices.tt | 30 ++++----
2 files changed, 50 insertions(+), 63 deletions(-)
diff --git a/acqui/invoices.pl b/acqui/invoices.pl
index b7f360e..6c732e9 100755
--- a/acqui/invoices.pl
+++ b/acqui/invoices.pl
@@ -37,7 +37,7 @@ use C4::Acquisition;
use C4::Bookseller qw/GetBookSeller/;
use C4::Branch;
-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,
suppliername => $supplier,
shipmentdatefrom => $shipmentdatefrom_iso,
@@ -83,24 +83,11 @@ if ( $op and $op eq "do_search" ) {
publicationyear => $publicationyear,
branchcode => $branch
);
- foreach (@invoices) {
- my %row = (
- invoiceid => $_->{invoiceid},
- billingdate => $_->{billingdate},
- invoicenumber => $_->{invoicenumber},
- suppliername => $_->{suppliername},
- 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;
@@ -108,17 +95,17 @@ foreach (@suppliers) {
$selected = 1;
$suppliername = $_->{'name'};
}
- my %row = (
- suppliername => $_->{'name'},
- supplierid => $_->{'id'},
+ push @{$suppliers_loop},
+ {
+ suppliername => $_->{name},
+ supplierid => $_->{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;
@@ -126,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,
- supplier => $supplier,
- 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,
+ supplier => $supplier,
+ 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 21147e5..5e69ebd 100644
--- 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>[% result.invoicenumber %]</td>
- <td>[% result.suppliername %]</td>
+ <td>[% invoice.invoicenumber %]</td>
+ <td>[% invoice.suppliername %]</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&invoiceid=[% result.invoiceid %]&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&invoiceid=[% invoice.invoiceid %]&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>
[% ELSE %]
- <a href="invoice.pl?op=close&invoiceid=[% result.invoiceid %]&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&invoiceid=[% invoice.invoiceid %]&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>
[% END %]
</td>
</tr>
@@ -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 -->
--
1.8.1.2.422.g08c0e7f
More information about the Koha-patches
mailing list