[Koha-patches] [PATCH] Bug 4510 Script processes single supplier not an array

Colin Campbell colin.campbell at ptfs-europe.com
Wed May 19 19:42:38 CEST 2010


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




More information about the Koha-patches mailing list