Bugzilla – Attachment 2136 Details for
Bug 4510
acq/supplier.pl mistaken about return from GetBooksellerById
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
improved patch
0001-Bug-4510-Script-processes-single-supplier-not-an-arr.patch (text/plain), 14.01 KB, created by
Chris Cormack
on 2010-05-19 17:40:00 UTC
(
hide
)
Description:
improved patch
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2010-05-19 17:40:00 UTC
Size:
14.01 KB
patch
obsolete
>From bb328fa1e69f6a4e4233ad60184c7e691e2cbbbf Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Wed, 19 May 2010 18:28:36 +0100 >Subject: [PATCH] Bug 4510 Script processes single supplier not an array >Content-Type: text/plain; charset="utf-8" > >Script was written as though it had an array rather than >a single supplier. Replaced the excess punctuation with a single >supplier variable > >replaced the C-style currency loop with a simpler perl-style one >we only need 1 loop not multiples >Changed variable name $GST to $tax_rate it's not magical and not a constant >and we all know what tax is >Fetch Contract data once rather than twice from two different modules >Correct size of discount data on display >--- > acqui/supplier.pl | 219 +++++++++----------- > .../prog/en/modules/acqui/supplier.tmpl | 20 ++- > 2 files changed, 116 insertions(+), 123 deletions(-) > >diff --git a/acqui/supplier.pl b/acqui/supplier.pl >index c3a5db4..6c49a90 100755 >--- a/acqui/supplier.pl >+++ b/acqui/supplier.pl >@@ -1,10 +1,8 @@ > #!/usr/bin/perl > >- >-#script to show display basket of orders >- > # Copyright 2000-2002 Katipo Communications > # Copyright 2008-2009 BibLibre SARL >+# Copyright 2010 PTFS Europe Ltd > # > # This file is part of Koha. > # >@@ -43,8 +41,7 @@ To know the bookseller this script has to display details. > use strict; > use warnings; > use C4::Auth; >-use C4::Acquisition; >-use C4::Contract; >+use C4::Contract qw/GetContract/; > use C4::Biblio; > use C4::Output; > use C4::Dates qw/format_date /; >@@ -53,128 +50,118 @@ use CGI; > use C4::Bookseller; > use C4::Budgets; > >-my $query = new CGI; >-my $id = $query->param('supplierid'); >-my @booksellers = GetBookSellerFromId($id) if $id; >-my $count = scalar @booksellers; >-my $op = $query->param('op') || "display"; >-my ($template, $loggedinuser, $cookie) = get_template_and_user( >- { template_name => "acqui/supplier.tmpl", >- query => $query, >- type => "intranet", >- authnotrequired => 0, >- flagsrequired => { acquisition => 'vendors_manage' }, >- debug => 1, >- } >+my $query = CGI->new; >+my $id = $query->param('supplierid'); >+my $supplier = {}; >+if ($id) { >+ $supplier = GetBookSellerFromId($id); >+} >+my $op = $query->param('op') || 'display'; >+my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >+ { template_name => 'acqui/supplier.tmpl', >+ query => $query, >+ type => 'intranet', >+ authnotrequired => 0, >+ flagsrequired => { acquisition => 'vendors_manage' }, >+ debug => 1, >+ } > ); >-my $seller_gstrate = $booksellers[0]->{'gstrate'}; >-# A perl-ism: '0'==false, '0.000'==true, but 0=='0.000' - this accounts for that >-undef $seller_gstrate if ($seller_gstrate == 0); >-my $GST = $seller_gstrate || C4::Context->preference("gist"); >-$GST *= 100; >+my $seller_gstrate = $supplier->{'gstrate'}; > >-my @contracts = GetContracts($id); >-my $contractcount = scalar(@contracts); >-$template->param(hascontracts => 1) if ($contractcount > 0); >+# ensure the scalar isn't flagged as a string >+$seller_gstrate = ( defined $seller_gstrate ) ? $seller_gstrate + 0 : 0; >+my $tax_rate = $seller_gstrate || C4::Context->preference('gist'); >+$tax_rate *= 100; > > #build array for currencies >-if ($op eq "display") { >+if ( $op eq 'display' ) { > >- # get contracts >- my @contracts = @{GetContract( { booksellerid => $id } )}; >+ my $contracts = GetContract( { booksellerid => $id } ); > >- # format dates >- for ( @contracts ) { >- $$_{contractstartdate} = format_date($$_{contractstartdate}); >- $$_{contractenddate} = format_date($$_{contractenddate}); >+ for ( @{$contracts} ) { >+ $_->{contractstartdate} = format_date( $_->{contractstartdate} ); >+ $_->{contractenddate} = format_date( $_->{contractenddate} ); > } > >- $template->param( >- id => $id, >- name => $booksellers[0]->{'name'}, >- postal => $booksellers[0]->{'postal'}, >- address1 => $booksellers[0]->{'address1'}, >- address2 => $booksellers[0]->{'address2'}, >- address3 => $booksellers[0]->{'address3'}, >- address4 => $booksellers[0]->{'address4'}, >- phone => $booksellers[0]->{'phone'}, >- fax => $booksellers[0]->{'fax'}, >- url => $booksellers[0]->{'url'}, >- contact => $booksellers[0]->{'contact'}, >- contpos => $booksellers[0]->{'contpos'}, >- contphone => $booksellers[0]->{'contphone'}, >- contaltphone => $booksellers[0]->{'contaltphone'}, >- contfax => $booksellers[0]->{'contfax'}, >- contemail => $booksellers[0]->{'contemail'}, >- contnotes => $booksellers[0]->{'contnotes'}, >- notes => $booksellers[0]->{'notes'}, >- active => $booksellers[0]->{'active'}, >- gstreg => $booksellers[0]->{'gstreg'}, >- listincgst => $booksellers[0]->{'listincgst'}, >- invoiceincgst => $booksellers[0]->{'invoiceincgst'}, >- gstrate => $booksellers[0]->{'gstrate'}*100, >- discount => $booksellers[0]->{'discount'}, >- invoiceprice => $booksellers[0]->{'invoiceprice'}, >- listprice => $booksellers[0]->{'listprice'}, >- GST => $GST, >- basketcount => $booksellers[0]->{'basketcount'}, >- contracts => \@contracts >- ); >-} >-elsif ($op eq 'delete') { >- &DelBookseller($id); >- print $query->redirect("/cgi-bin/koha/acqui/acqui-home.pl"); >- exit; >+ my $gstrate = defined $supplier->{gstrate} ? $supplier->{gstrate} * 100 : 0; >+ >+ $template->param( >+ id => $id, >+ name => $supplier->{'name'}, >+ postal => $supplier->{'postal'}, >+ address1 => $supplier->{'address1'}, >+ address2 => $supplier->{'address2'}, >+ address3 => $supplier->{'address3'}, >+ address4 => $supplier->{'address4'}, >+ phone => $supplier->{'phone'}, >+ fax => $supplier->{'fax'}, >+ url => $supplier->{'url'}, >+ contact => $supplier->{'contact'}, >+ contpos => $supplier->{'contpos'}, >+ contphone => $supplier->{'contphone'}, >+ contaltphone => $supplier->{'contaltphone'}, >+ contfax => $supplier->{'contfax'}, >+ contemail => $supplier->{'contemail'}, >+ contnotes => $supplier->{'contnotes'}, >+ notes => $supplier->{'notes'}, >+ active => $supplier->{'active'}, >+ gstreg => $supplier->{'gstreg'}, >+ listincgst => $supplier->{'listincgst'}, >+ invoiceincgst => $supplier->{'invoiceincgst'}, >+ gstrate => $gstrate, >+ discount => $supplier->{'discount'}, >+ invoiceprice => $supplier->{'invoiceprice'}, >+ listprice => $supplier->{'listprice'}, >+ GST => $tax_rate, >+ basketcount => $supplier->{'basketcount'}, >+ contracts => $contracts >+ ); >+} elsif ( $op eq 'delete' ) { >+ DelBookseller($id); >+ print $query->redirect('/cgi-bin/koha/acqui/acqui-home.pl'); >+ exit; > } else { > my @currencies = GetCurrencies(); >- my $count = scalar @currencies; >- >- my @loop_pricescurrency; >- my @loop_invoicecurrency; >- for (my $i=0;$i<$count;$i++) { >- if ($booksellers[0]->{'listprice'} eq $currencies[$i]->{'currency'}) { >- push @loop_pricescurrency, { currency => "<option selected=\"selected\" value=\"$currencies[$i]->{'currency'}\">$currencies[$i]->{'currency'}</option>" }; >- } else { >- push @loop_pricescurrency, { currency => "<option value=\"$currencies[$i]->{'currency'}\">$currencies[$i]->{'currency'}</option>"}; >- } >- if ($booksellers[0]->{'invoiceprice'} eq $currencies[$i]->{'currency'}) { >- push @loop_invoicecurrency, { currency => "<option selected=\"selected\" value=\"$currencies[$i]->{'currency'}\">$currencies[$i]->{'currency'}</option>"}; >- } else { >- push @loop_invoicecurrency, { currency => "<option value=\"$currencies[$i]->{'currency'}\">$currencies[$i]->{'currency'}</option>"}; >- } >+ my $loop_currency; >+ for (@currencies) { >+ push @{$loop_currency}, >+ { currency => $_->{currency}, >+ listprice => ( $_->{currency} eq $supplier->{listprice} ), >+ invoiceprice => ( $_->{currency} eq $supplier->{invoiceprice} ), >+ }; > } >- $template->param( >- id => $id, >- name => $booksellers[0]->{'name'}, >- postal => $booksellers[0]->{'postal'}, >- address1 => $booksellers[0]->{'address1'}, >- address2 => $booksellers[0]->{'address2'}, >- address3 => $booksellers[0]->{'address3'}, >- address4 => $booksellers[0]->{'address4'}, >- phone => $booksellers[0]->{'phone'}, >- fax => $booksellers[0]->{'fax'}, >- url => $booksellers[0]->{'url'}, >- contact => $booksellers[0]->{'contact'}, >- contpos => $booksellers[0]->{'contpos'}, >- contphone => $booksellers[0]->{'contphone'}, >- contaltphone => $booksellers[0]->{'contaltphone'}, >- contfax => $booksellers[0]->{'contfax'}, >- contemail => $booksellers[0]->{'contemail'}, >- contnotes => $booksellers[0]->{'contnotes'}, >- notes => $booksellers[0]->{'notes'}, >- active => $id?$booksellers[0]->{'active'}:1, # set active ON by default for supplier add (id empty for add) >- gstreg => $booksellers[0]->{'gstreg'}, >- listincgst => $booksellers[0]->{'listincgst'}, >- invoiceincgst => $booksellers[0]->{'invoiceincgst'}, >- gstrate => $booksellers[0]->{'gstrate'}*100, >- discount => $booksellers[0]->{'discount'}, >- loop_pricescurrency => \@loop_pricescurrency, >- loop_invoicecurrency => \@loop_invoicecurrency, >- GST => $GST, >- enter => 1, >- ); >-} >- > >+ my $gstrate = defined $supplier->{gstrate} ? $supplier->{gstrate} * 100 : 0; >+ $template->param( >+ id => $id, >+ name => $supplier->{'name'}, >+ postal => $supplier->{'postal'}, >+ address1 => $supplier->{'address1'}, >+ address2 => $supplier->{'address2'}, >+ address3 => $supplier->{'address3'}, >+ address4 => $supplier->{'address4'}, >+ phone => $supplier->{'phone'}, >+ fax => $supplier->{'fax'}, >+ url => $supplier->{'url'}, >+ contact => $supplier->{'contact'}, >+ contpos => $supplier->{'contpos'}, >+ contphone => $supplier->{'contphone'}, >+ contaltphone => $supplier->{'contaltphone'}, >+ contfax => $supplier->{'contfax'}, >+ contemail => $supplier->{'contemail'}, >+ contnotes => $supplier->{'contnotes'}, >+ notes => $supplier->{'notes'}, >+ # set active ON by default for supplier add (id empty for add) >+ active => $id ? $supplier->{'active'} : 1, >+ gstreg => $supplier->{'gstreg'}, >+ listincgst => $supplier->{'listincgst'}, >+ invoiceincgst => $supplier->{'invoiceincgst'}, >+ gstrate => $gstrate, >+ discount => $supplier->{'discount'}, >+ loop_currency => $loop_currency, >+ GST => $tax_rate, >+ enter => 1, >+ ); >+} > > output_html_with_http_headers $query, $cookie, $template->output; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tmpl >index 61fea49..e07b303 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tmpl >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tmpl >@@ -88,16 +88,22 @@ if (f.company.value == "") { > <ol> > <li><label for="list_currency">List Prices are</label> > <select name="list_currency" id="list_currency"> >- <!-- TMPL_LOOP NAME="loop_pricescurrency" --> >- <!-- TMPL_VAR NAME="currency" --> >+ <!-- TMPL_LOOP NAME="loop_currency" --> >+ <option value="<!-- TMPL_VAR NAME="currency" -->" >+ <!-- TMPL_IF NAME="listprice" -->selected="1"<!-- /TMPL_IF -->> >+ <!--TMPL_VAR NAME="currency" --></option> > <!-- /TMPL_LOOP --> >- </select></li> >+ </select> >+ </li> > <li><label for="invoice_currency">Invoice Prices are</label> > <select name="invoice_currency" id="invoice_currency"> >- <!-- TMPL_LOOP NAME="loop_invoicecurrency" --> >- <!-- TMPL_VAR NAME="currency" --> >+ <!-- TMPL_LOOP NAME="loop_currency" --> >+ <option value="<!-- TMPL_VAR NAME="currency" -->" >+ <!-- TMPL_IF NAME="invoiceprice" -->selected="1"<!-- /TMPL_IF -->> >+ <!--TMPL_VAR NAME="currency" --></option> > <!-- /TMPL_LOOP --> >- </select></li> >+ </select> >+ </li> > </ol> > <ol class="radio"> > <!-- TMPL_IF NAME="GST" --><li><label for="gstyes" class="radio">Tax Number Registered:</label> >@@ -129,7 +135,7 @@ if (f.company.value == "") { > </ol> > <ol> > <li><label for="discount">Discount</label> >- <input type="text" size="3" id="discount" name="discount" value="<!-- TMPL_VAR NAME="discount" -->" /> %</li> >+ <input type="text" size="6" id="discount" name="discount" value="<!-- TMPL_VAR NAME="discount" -->" /> %</li> > <!-- TMPL_IF NAME="GST" --><li><label for="gstrate">Tax rate</label><input type="text" name="gstrate" id="gstrate" size="5" value="<!-- TMPL_VAR name="gstrate" -->"/>%</li><!-- /TMPL_IF --> > <li><label for="notes">Notes</label> > <textarea cols="40" rows="4" id="notes" name="notes" ><!-- TMPL_VAR NAME="notes" --></textarea></li></ol> >-- >1.6.6.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=2136">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 4510
:
2135
| 2136