From efd50ad414232a5b978220dbba2ce782c191fb17 Mon Sep 17 00:00:00 2001 From: Colin Campbell <colin.campbell@ptfs-europe.com> Date: Wed, 30 Jan 2013 08:50:23 +0000 Subject: [PATCH] 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> --- acqui/invoices.pl | 87 ++++++++----------- .../prog/en/modules/acqui/invoices.tt | 30 ++++---- 2 files changed, 52 insertions(+), 65 deletions(-) diff --git a/acqui/invoices.pl b/acqui/invoices.pl index b7f360e..1a7f4d3 100755 --- 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, 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.7.2.5