From b1111dda4d8b269e0330798ef3cf007f0ad7c4ee Mon Sep 17 00:00:00 2001 From: Colin Campbell 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 --- 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() {

Invoices

[% IF ( do_search ) %] - [% IF ( results_loop ) %] + [% IF invoices %] @@ -55,30 +55,30 @@ $(document).ready(function() { - [% FOREACH result IN results_loop %] + [% FOREACH invoice IN invoices %] - - + + - - + + @@ -131,7 +131,7 @@ $(document).ready(function() { [% END %]

- [% END %] + [% END %] [% ELSE %]

Use the search form on the left to find invoices.

[% END %] -- 1.8.1.2.422.g08c0e7f
[% result.invoicenumber %][% result.suppliername %][% invoice.invoicenumber %][% invoice.suppliername %] - [% IF (result.billingdate) %] - [% result.billingdate | $KohaDates %] + [% IF invoice.billingdate %] + [% invoice.billingdate | $KohaDates %] [% END %] [% result.receivedbiblios %][% result.receiveditems %][% invoice.receivedbiblios %][% invoice.receiveditems %] - [% IF ( result.closedate ) %] - Closed on [% result.closedate | $KohaDates %] + [% IF invoice.closedate %] + Closed on [% invoice.closedate | $KohaDates %] [% ELSE %] Open [% END %] - Details / - [% IF ( result.closedate ) %] - Reopen + Details / + [% IF invoice.closedate %] + Reopen [% ELSE %] - Close + Close [% END %]