Bugzilla – Attachment 14963 Details for
Bug 9505
acqui/invoices.pl contains an unnecessary loop
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
bug 9505 refactor loops in invoices.pl
bug-9505-refactor-loops-in-invoicespl.patch (text/plain), 10.56 KB, created by
Galen Charlton
on 2013-01-30 19:15:19 UTC
(
hide
)
Description:
bug 9505 refactor loops in invoices.pl
Filename:
MIME Type:
Creator:
Galen Charlton
Created:
2013-01-30 19:15:19 UTC
Size:
10.56 KB
patch
obsolete
>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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=14963">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9505
:
14942
|
14943
|
14963
|
18354
|
18505