Bug 4510 Script processes single supplier not an array

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

Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
This commit is contained in:
Colin Campbell 2010-05-19 18:42:38 +01:00 committed by Galen Charlton
parent 3fac59ba6b
commit 47a29c2bef
2 changed files with 116 additions and 123 deletions

View file

@ -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;

View file

@ -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>