basket.pl and template - Many fixes including SQL injection security check,

Signed-off-by: Joshua Ferraro <jmf@liblime.com>

diff --git a/acqui/basket.pl b/acqui/basket.pl
index 0baae79..0e29473 100755 (executable)
--- a/acqui/basket.pl
+++ b/acqui/basket.pl
@@ -29,12 +29,7 @@ use C4::Acquisition;
 use C4::Bookfund;
 use C4::Bookseller;
 use C4::Dates qw/format_date/;
-
-use vars qw($debug);
-
-BEGIN {
-    $debug = $ENV{DEBUG} || 1;
-}
+use C4::Debug;
 
 =head1 NAME
 
@@ -43,8 +38,8 @@ basket.pl
 =head1 DESCRIPTION
 
  This script display all informations about basket for the supplier given
- on input arg. Moreover, it allow to add a new order for this supplier from
- an existing record, a suggestion or from a new record.
+ on input arg.  Moreover, it allows us to add a new order for this supplier from
+ an existing record, a suggestion or a new record.
 
 =head1 CGI PARAMETERS
 
@@ -52,7 +47,7 @@ basket.pl
 
 =item $basketno
 
-this parameter seems to be unused.
+The basket number.
 
 =item supplierid
 
@@ -67,7 +62,28 @@ the supplier this script have to display the basket.
 my $query        = new CGI;
 my $basketno     = $query->param('basketno');
 my $booksellerid = $query->param('supplierid');
-my $order        = $query->param('order');
+my $sort         = $query->param('order');
+
+my @sort_loop;
+if (defined $sort) {
+       foreach (split /\,/, $sort) {
+               my %sorthash = (
+                       string => $_,
+               );
+               # other possibly valid tables for later: aqbookfund biblio biblioitems
+               if (
+                       (/^\s*(aqorderbreakdown)\.(\w+)\s*$/ and $2 eq 'bookfundid'   ) or
+                       (/^\s*(biblioitems)\.(\w+)\s*$/      and $2 eq 'publishercode')
+               ) {
+                       $sorthash{table} = $1;
+                       $sorthash{field} = $2;
+               } else {
+                       $sorthash{error} = 1;
+               }
+               push @sort_loop, \%sorthash;
+       }
+}
+
 my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     {
         template_name   => "acqui/basket.tmpl",
@@ -81,6 +97,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 
 my $basket = GetBasket($basketno);
 
+# FIXME : what about the "discount" percentage?
 # FIXME : the query->param('supplierid') below is probably useless. The bookseller is always known from the basket
 # if no booksellerid in parameter, get it from basket
 # warn "=>".$basket->{booksellerid};
@@ -115,44 +132,31 @@ else {
       "loggedinuser: $loggedinuser; creationdate: %s; authorisedby: %s",
       $basket->{creationdate}, $basket->{authorisedby};
 
-    my @results = GetOrders( $basketno, $order );
+    my @results = GetOrders( $basketno, $sort );
     my $count = scalar @results;
 
-    my $line_total;     # total of each line
     my $sub_total;      # total of line totals
-    my $gist;           # GST
     my $grand_total;    # $subttotal + $gist
-    my $toggle = 0;
 
     # my $line_total_est; # total of each line
     my $sub_total_est;      # total of line totals
     my $sub_total_rrp;      # total of line totals
-    my $gist_est;           # GST
     my $grand_total_est;    # $subttotal + $gist
 
     my $qty_total;
     my @books_loop;
     for ( my $i = 0 ; $i < $count ; $i++ ) {
         my $rrp = $results[$i]->{'listprice'};
+               my $qty = $results[$i]->{'quantity'};
         $rrp = ConvertCurrency( $results[$i]->{'currency'}, $rrp );
-        $sub_total_rrp += $results[$i]->{'quantity'} * $results[$i]->{'rrp'};
-        $line_total = $results[$i]->{'quantity'} * $results[$i]->{'ecost'};
+        $sub_total_rrp += $qty * $results[$i]->{'rrp'};
+        my $line_total = $qty * $results[$i]->{'ecost'};
+               # FIXME: what about the "actual cost" field?
         $sub_total += $line_total;
-        $qty_total += $results[$i]->{'quantity'};
-        my %line;
-        %line = %{ $results[$i] };
-
-        if ( $toggle == 0 ) {
-            $line{color} = '#EEEEEE';
-            $toggle = 1;
-        }
-        else {
-            $line{color} = 'white';
-            $toggle = 0;
-        }
-        $line{order_received} =
-          ( $results[$i]->{'quantity'} eq $results[$i]->{'quantityreceived'} );
-        $line{publishercode} = $results[$i]->{'publishercode'};
+        $qty_total += $qty;
+        my %line = %{ $results[$i] };
+               ($i%2) and $line{toggle} = 1;
+        $line{order_received}= ( $qty eq $results[$i]->{'quantityreceived'} );
         $line{basketno}      = $basketno;
         $line{i}             = $i;
         $line{rrp}           = sprintf( "%.2f", $line{'rrp'} );
@@ -161,16 +165,22 @@ else {
         $line{odd}           = $i % 2;
         push @books_loop, \%line;
     }
-    my $prefgist = C4::Context->preference("gist");
-    $gist            = sprintf( "%.2f", $sub_total * $prefgist );
-    $grand_total     = $sub_total;
-    $grand_total_est = $sub_total_est;
-    unless ( $bookseller->{'listincgst'} ) {
+    my $prefgist = C4::Context->preference("gist") || 0;
+    my $gist     = $sub_total     * $prefgist;
+    my $gist_rrp = $sub_total_rrp * $prefgist;
+    $grand_total     = $sub_total_est = $sub_total;
+    $grand_total_est = $sub_total_est;         # FIXME: Too many things that are ALL the SAME
+       my $temp;
+    if ($temp = $bookseller->{'listincgst'}) {
+               $template->param(listincgst => $temp);
+               $gist = 0;
+       } else {
         $grand_total += $gist;
-        $grand_total_est += sprintf( "%.2f", $sub_total_est * $prefgist );
+        $grand_total_est += $sub_total_est * $prefgist;                # same thing as += gist
     }
-    my $grand_total_rrp = sprintf( "%.2f", $sub_total_rrp );
-    $gist_est = sprintf( "%.2f", $sub_total_est * $prefgist );
+    if ($temp = $bookseller->{'discount'}) {
+               $template->param(discount => sprintf( "%.2f", $temp ));
+       }
     $template->param(
         basketno         => $basketno,
         creationdate     => format_date( $basket->{creationdate} ),
@@ -186,14 +196,18 @@ else {
         address4         => $bookseller->{'address4'},
         entrydate        => format_date( $results[0]->{'entrydate'} ),
         books_loop       => \@books_loop,
+         sort_loop       => \@sort_loop,
         count            => $count,
-        sub_total        => sprintf( "%.2f", $sub_total ),
-        gist             => $gist,
+        gist             => $gist ? sprintf( "%.2f", $gist ) : 0,
+        gist_rate        => sprintf( "%.2f", $prefgist * 100) . '%',
+        gist_est         => sprintf( "%.2f", $sub_total_est * $prefgist ),
+        gist_rrp         => sprintf( "%.2f", $gist_rrp),
+          sub_total      => sprintf( "%.2f", $sub_total ),
         grand_total      => sprintf( "%.2f", $grand_total ),
-        sub_total_est    => $sub_total_est,
-        gist_est         => $gist_est,
-        grand_total_est  => $grand_total_est,
-        grand_total_rrp  => $grand_total_rrp,
+          sub_total_est  => sprintf( "%.2f", $sub_total_est),
+        grand_total_est  => sprintf( "%.2f", $grand_total_est),
+          sub_total_rrp  => sprintf( "%.2f", $sub_total_rrp),
+        grand_total_rrp  => sprintf( "%.2f", $sub_total_rrp + $gist_rrp),
         currency         => $bookseller->{'listprice'},
         qty_total        => $qty_total,
         GST              => $prefgist,

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tmpl
index db774fe..173b48c 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tmpl
@@ -12,7 +12,10 @@
             }
 //]]>
         </script>
-               <!-- /TMPL_UNLESS -->
+<!-- /TMPL_UNLESS -->
+<style type="text/css">
+.sortmsg {font-size: 80%;}
+</style>
 </head>
 <body>
 <!-- TMPL_INCLUDE NAME="header.inc" -->
@@ -46,6 +49,17 @@
     
     <div id="acqui_basket_content">
     <h2>Order Details</h2>
+       <!-- TMPL_IF NAME="sort_loop" -->
+               <!-- TMPL_LOOP NAME="sort_loop" -->
+                       <!-- TMPL_IF name="error" -->
+                       <div class="error">ERROR: Illegal sort requested by &quot;<!-- TMPL_VAR NAME="string" -->&quot;.
+                               <br />You will need to use valid sort criteria to return valid results.</div>
+                       <!-- TMPL_ELSE -->
+                       <div class="sortmsg">Sorted by &quot;<!-- TMPL_VAR NAME="string" -->&quot;.</div>
+                       <!-- /TMPL_IF -->
+               <!-- /TMPL_LOOP -->
+       <!-- /TMPL_IF -->
+
     <!-- TMPL_IF name="books_loop" -->
         <table>
             <tr>
@@ -67,7 +81,7 @@
                 <!-- /TMPL_IF -->
             </tr>
             <!-- TMPL_LOOP NAME="books_loop" -->
-                 <!-- TMPL_IF NAME="order_received" --><tr class="disabled"><!-- TMPL_ELSE --><!-- TMPL_IF NAME="highlight" --><tr class="highlight"><!-- TMPL_ELSE --><tr><!-- /TMPL_IF --><!-- /TMPL_IF -->
+                 <!-- TMPL_IF NAME="order_received" --><tr class="disabled"><!-- TMPL_ELSE --><!-- TMPL_IF NAME="toggle" --><tr class="highlight"><!-- TMPL_ELSE --><tr><!-- /TMPL_IF --><!-- /TMPL_IF -->
                     <td><!-- TMPL_VAR NAME="ordernumber" -->
                                                        <!-- TMPL_IF NAME="order_received" --> (rcvd)<!-- /TMPL_IF --></td>
                         <td>
@@ -101,28 +115,32 @@
                     <input type="hidden" name="number" value="<!-- TMPL_VAR NAME="count" -->" />
                     <input type="hidden" name="basketno" value="<!-- TMPL_VAR NAME="basketno" -->" />
                 </td>
-                <th colspan="3">SubTotal</th>
-              <!--  <th><!-- TMPL_VAR NAME="sub_total_est" --></th> -->
+                <th>SubTotal</th>
+                <th><!-- TMPL_VAR NAME="sub_total_rrp" --></th>
+                <th><!-- currently duplicative <!-- TMPL_VAR NAME="sub_total_est" --> --></th>
                 <th><!-- TMPL_VAR name="qty_total" --></th>
                 <th><!-- TMPL_VAR NAME="sub_total" --></th>
                     <!-- TMPL_IF name="active" -->
                         <!-- TMPL_IF name="closedate" -->
-                          <td colspan="2">&nbsp;</td>
+                          <td colspan="1" rowspan="3">&nbsp;</td>
                         <!-- TMPL_ELSE -->
-                          <td colspan="3">&nbsp;</td>
+                          <td colspan="3" rowspan="3">&nbsp;</td>
                         <!-- /TMPL_IF -->
                     <!-- /TMPL_IF -->
-                
             </tr>
-<!-- TMPL_IF NAME="GST" -->            <tr>
-                <th>GST</th>
+                       <tr>
+                <th>GST (<!-- TMPL_VAR NAME="gist_rate" -->)</th>
+                <th><!-- TMPL_VAR NAME="gist_rrp" --></th>
+<!-- TMPL_UNLESS NAME="listincgst" -->
+                <th><!-- currently duplicative <!-- TMPL_VAR NAME="gist_est" --> --></th>
                 <th>&nbsp;</th>
-                <th><!-- TMPL_VAR NAME="gist_est" --></th>
                 <th><!-- TMPL_VAR NAME="gist" --></th>
-                <th>&nbsp;</th>
-            </tr><!-- /TMPL_IF -->
+<!-- TMPL_ELSE -->
+                               <th colspan="3">**</th>
+<!-- /TMPL_UNLESS -->
+            </tr>
             <tr>
-                <th>TOTAL  (<!-- TMPL_VAR NAME="currency" -->)</th>
+                <th>TOTAL (<!-- TMPL_VAR NAME="currency" -->)</th>
                 <th><!-- TMPL_VAR NAME="grand_total_rrp" --></th>
                 <th>&nbsp;</th>
                 <th><!-- TMPL_VAR name="qty_total" --></th>
@@ -134,8 +152,10 @@
             <tr><td>Basket empty</td></tr>
         </table>
     <!-- /TMPL_IF -->
+       <!-- TMPL_IF NAME="listincgst" --><small class="highlight">** Vendor's listings already include GST.</small>
+    <!-- /TMPL_IF -->
     </div>
-    
+    <br />
     <!-- TMPL_UNLESS name="closedate" -->
     <div id="acqui_basket_add">
         <h2>Add To Order</h2>