From 47a29c2bef4ac72f2ad9de38e75a2503c2b735f2 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Wed, 19 May 2010 18:42:38 +0100 Subject: [PATCH] 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 --- 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 c3a5db428e..6c49a905fb 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 => "" }; - } else { - push @loop_pricescurrency, { currency => ""}; - } - if ($booksellers[0]->{'invoiceprice'} eq $currencies[$i]->{'currency'}) { - push @loop_invoicecurrency, { currency => ""}; - } else { - push @loop_invoicecurrency, { currency => ""}; - } + 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 61fea49252..e07b303118 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 == "") {
    2. + +
    3. + +
  1. @@ -129,7 +135,7 @@ if (f.company.value == "") {
  1. - " /> %
    2. + " /> %
  2. "/>%
-- 2.39.5