From c98542ac6b93fedbb4a218c57072e4473b36f811 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. 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 Signed-off-by: Jonathan Druart --- acqui/invoices.pl | 94 +++++++++----------- .../prog/en/modules/acqui/invoices.tt | 32 +++---- 2 files changed, 56 insertions(+), 70 deletions(-) diff --git a/acqui/invoices.pl b/acqui/invoices.pl index 44589ab..f30e374 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, 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 --- 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 %] - - + + - - + + @@ -92,7 +92,7 @@ $(document).ready(function() { [% IF ( invoicenumber ) %]
  • Invoice no.: [% invoicenumber %]
    • [% END %] - [% IF ( supplier ) %] + [% IF booksellerid %]
  • Vendor: [% suppliername %]
    • [% END %] [% IF ( billingdatefrom ) %] @@ -131,7 +131,7 @@ $(document).ready(function() { [% END %]

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

    Use the search form on the left to find invoices.

    [% END %] -- 1.7.10.4
    [% 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 %]