record a trace of shipping calculation communications in the order
authorTony Cook <tony@develop-help.com>
Fri, 31 Jul 2009 05:55:59 +0000 (05:55 +0000)
committertony <tony@45cb6cf1-00bc-42d2-bb5a-07f51df49f94>
Fri, 31 Jul 2009 05:55:59 +0000 (05:55 +0000)
fix null driver to return a defined cost

make shipping optional to match previous behaviour by default

improve error reporting to the checkout page when a shipper can't ship
to a location

schema/bse.sql
site/cgi-bin/modules/BSE/DB/Mysql.pm
site/cgi-bin/modules/BSE/TB/Order.pm
site/cgi-bin/modules/BSE/UI/Shop.pm
site/cgi-bin/modules/Courier.pm
site/cgi-bin/modules/Courier/AustraliaPost.pm
site/cgi-bin/modules/Courier/Fastway.pm
site/cgi-bin/modules/Courier/Null.pm
site/templates/checkoutnew_base.tmpl
site/util/mysql.str

index 4e17eee..9d8a457 100644 (file)
@@ -318,6 +318,9 @@ create table orders (
   -- the name of the shipping method as per $courier->name
   shipping_name varchar(40) not null default '',
 
+  -- trace of the request and response
+  shipping_trace text null,
+
   primary key (id),
   index order_cchash(ccNumberHash),
   index order_userId(userId, orderDate)
index 8f22b6e..8cf2f54 100644 (file)
@@ -150,8 +150,8 @@ SQL
    Orders => 'select * from orders',
    getOrderByPkey => 'select * from orders where id = ?',
    getOrderItemByOrderId => 'select * from order_item where orderId = ?',
-   addOrder => 'insert orders values(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
-   replaceOrder => 'replace orders values(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
+   addOrder => 'insert orders values(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
+   replaceOrder => 'replace orders values(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
    addOrderItem => 'insert order_item values(null,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
    replaceOrderItem => 'replace order_item values(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)',
    getOrderByUserId => 'select * from orders where userId = ?',
index cbcc77a..dc8bfd3 100644 (file)
@@ -25,7 +25,7 @@ sub columns {
            ccOnline ccSuccess ccReceipt ccStatus ccStatusText
            ccStatus2 ccTranId complete delivOrganization billOrganization
            delivStreet2 billStreet2 purchase_order shipping_method
-           shipping_name/;
+           shipping_name shipping_trace/;
 }
 
 =item siteuser
index 8acf69b..2eee186 100644 (file)
@@ -450,44 +450,58 @@ sub req_checkout {
     return $value;
   };
 
-  # Get a list of couriers
-  my $selected = 0;
-  my $sel_cn = $old->("shipping_name") || "";
-  my %fake_order;
-  my %fields = BSE::TB::Order->valid_fields($cfg);
-  for my $name (keys %fields) {
-    $fake_order{$name} = $old->($name);
-  }
-  $fake_order{delivCountry} ||= $cfg->entry("basic", "country", "Australia");
-
+  # shipping handling, if enabled
+  my $shipping_select = ''; # select of shipping types
   my ($delivery_in, $shipping_cost, $shipping_method);
-  my @couriers = Courier::get_couriers($cfg);
-
-  for my $c (@couriers) {
-    $c->set_order(\%fake_order, \@items);
-  }
-
-  @couriers = grep $_->can_deliver, @couriers;
-
-  my ($sel_cour) = grep $_->name eq $sel_cn, @couriers;
-  if ($sel_cour and $fake_order{delivPostCode} and $fake_order{delivSuburb}) {
-    $sel_cour->set_order(\%fake_order, \@items);
-    if ($sel_cour->can_deliver) {
-      $sel_cour->calculate_shipping();
-      $delivery_in = $sel_cour->delivery_in();
-      $shipping_cost = $sel_cour->shipping_cost();
-      $shipping_method = $sel_cour->description();
+  my $shipping_error = '';
+  my $shipping_name = '';
+  my $prompt_ship = $cfg->entry("shop", "shipping", 0);
+  if ($prompt_ship) {
+    # Get a list of couriers
+    my $sel_cn = $old->("shipping_name") || "";
+    my %fake_order;
+    my %fields = BSE::TB::Order->valid_fields($cfg);
+    for my $name (keys %fields) {
+      $fake_order{$name} = $old->($name);
+    }
+    $fake_order{delivCountry} ||= $cfg->entry("basic", "country", "Australia");
+    
+    my @couriers = Courier::get_couriers($cfg);
+    
+    for my $c (@couriers) {
+      $c->set_order(\%fake_order, \@items);
+    }
+    
+    @couriers = grep $_->can_deliver, @couriers;
+    
+    my ($sel_cour) = grep $_->name eq $sel_cn, @couriers;
+    if ($sel_cour and $fake_order{delivPostCode} and $fake_order{delivSuburb}) {
+      $sel_cour->set_order(\%fake_order, \@items);
+      if ($sel_cour->can_deliver) {
+       $sel_cour->calculate_shipping();
+       $delivery_in = $sel_cour->delivery_in();
+       $shipping_cost = $sel_cour->shipping_cost();
+       $shipping_method = $sel_cour->description();
+       $shipping_name = $sel_cour->name;
+       unless (defined $shipping_cost) {
+         $shipping_error = $sel_cour->error_message;
+         $errors->{shipping_name} = $shipping_error;
+       }
+      }
     }
+    
+    $shipping_select = popup_menu
+      (
+       -name => "shipping_name",
+       -values => [ map $_->name, @couriers ],
+       -labels => { map { $_->name => $_->description } @couriers },
+       -default => $sel_cn,
+      );
   }
 
-  
-  my $shipping_select = popup_menu
-    (
-     -name => "shipping_name",
-     -values => [ map $_->name, @couriers ],
-     -labels => { map { $_->name => $_->description } @couriers },
-     -default => $sel_cn,
-    );
+  if (!$message && keys %$errors) {
+    $message = $req->message($errors);
+  }
 
   my $item_index = -1;
   my @options;
@@ -506,10 +520,13 @@ sub req_checkout {
      user => $user ? [ \&tag_hash, $user ] : '',
      affiliate_code => escape_html($affiliate_code),
      error_img => [ \&tag_error_img, $cfg, $errors ],
+     ifShipping => $prompt_ship,
      shipping_select => $shipping_select,
-     delivery_in => $delivery_in,
+     delivery_in => escape_html($delivery_in),
      shipping_cost => $shipping_cost,
-     shipping_method => $shipping_method,
+     shipping_method => escape_html($shipping_method),
+     shipping_error => escape_html($shipping_error),
+     shipping_name => $shipping_name,
     );
   $req->session->{custom} = \%custom_state;
   my $tmp = $acts{total};
@@ -1419,34 +1436,38 @@ sub _fillout_order {
   $values->{gst} = $total_gst;
   $values->{wholesale} = $total_wholesale;
 
-  my ($courier) = Courier::get_couriers($cfg, $cgi->param("shipping_name"));
-  if ($courier) {
+  my $prompt_ship = $cfg->entry("shop", "shipping", 0);
+  if ($prompt_ship) {
+    my ($courier) = Courier::get_couriers($cfg, $cgi->param("shipping_name"));
+    if ($courier) {
       $courier->set_order($values, $items);
       unless ($courier->can_deliver()) {
-          $cgi->param("courier", undef);
-          $$rmsg =
-            "Can't use the selected courier ".
+       $cgi->param("courier", undef);
+       $$rmsg =
+         "Can't use the selected courier ".
             "(". $courier->description(). ") for this order.";
-          return;
+       return;
       }
       $courier->calculate_shipping();
       my $cost = $courier->shipping_cost();
       if (!$cost and $courier->name() ne 'contact') {
-          my $err = $courier->error_message();
-          $$rmsg = "Error calculating shipping cost";
-          $$rmsg .= ": $err" if $err;
-          return;
+       my $err = $courier->error_message();
+       $$rmsg = "Error calculating shipping cost";
+       $$rmsg .= ": $err" if $err;
+       return;
       }
       $values->{shipping_method} = $courier->description();
       $values->{shipping_name} = $courier->name;
       $values->{shipping_cost} = $cost;
+      $values->{shipping_trace} = $courier->trace;
       $values->{delivery_in} = $courier->delivery_in();
       $values->{total} += $values->{shipping_cost};
-  }
-  else {
+    }
+    else {
       # XXX: What to do?
       $$rmsg = "Error: no usable courier found.";
       return;
+    }
   }
 
   my $cust_class = custom_class($cfg);
index b41cc34..0053c92 100644 (file)
@@ -9,7 +9,7 @@ sub new {
     my $self = {};
     my %args = @_;
 
-    @$self{qw(cost days error)} = undef;
+    @$self{qw(cost days error trace)} = undef;
     $self->{config} = $args{config};
     $self->{type} = $args{type};
     $self->{ua} = LWP::UserAgent->new;
@@ -114,6 +114,10 @@ sub error_message {
     return $self->{error};
 }
 
+sub trace {
+    $_[0]{trace};
+}
+
 sub get_couriers {
     my ($cfg, $wanted) = @_;
 
index dbf74f1..581ebe5 100644 (file)
@@ -25,6 +25,8 @@ sub can_deliver {
 sub calculate_shipping {
     my ($self) = @_;
 
+    my $trace = '';
+
     my %data = ();
 
     $data{Service_Type} = $self->{type};
@@ -45,9 +47,14 @@ sub calculate_shipping {
     @data{qw(Length Width Height)} = ($l, $w, $h);
 
     my $u = URI->new($url);
-    my $r = $self->{ua}->post($u, \%data);
 
+    $trace .= "Request URL: $u\nPosted:\n";
+    $trace .= " $_: $data{$_}\n" for keys %data;
+    $trace .= "\n";
+
+    my $r = $self->{ua}->post($u, \%data);
     if ($r->is_success) {
+        $trace .= "Success: [\n" . $r->content . "\n]\n";
         my @lines = split /\r?\n/, $r->content;
         foreach (@lines) {
             if (/^charge=(.*)$/) {
@@ -68,9 +75,11 @@ sub calculate_shipping {
         }
     }
     else {
+        $trace .= "Error: ". $r->status_line . "\n";
         warn $u->as_string(). ": ". $r->status_line, "\n";
         $self->{error} = "Server error";
     }
+    $self->{trace} = $trace;
 }
 
 1;
index c08bc40..845115a 100644 (file)
@@ -22,6 +22,8 @@ sub can_deliver {
 sub calculate_shipping {
     my ($self) = @_;
 
+    my $debug = $self->{config}->entry("debug", "fastway", 0);
+
     my %data = (
         APPNAME => "FW",
         PRGNAME => "FastLocatorResult",
@@ -29,6 +31,8 @@ sub calculate_shipping {
         Country => 1,
     );
 
+    my $trace = '';
+
     $data{CustFranCode} =
         $self->{config}->entry("shipping", "fastwayfranchiseecode") || "";
     $data{pFranchisee} =
@@ -59,7 +63,13 @@ sub calculate_shipping {
     $u->query_form(\%data);
     my $r = $self->{ua}->get($u);
 
+    $trace .= "Request url: $u\n";
+
+    $debug and print STDERR "Fastway request: $u\n";
+
     if ($r->is_success) {
+        $debug and print STDERR "Success: [",$r->content,"]\n";
+        $trace .= "Response: [\n" . $r->content . "\n]\n";
         my $p = XML::Parser->new(
             Style => "Stream",
             Pkg => "Courier::Fastway::Parser"
@@ -73,7 +83,7 @@ sub calculate_shipping {
                 $self->{days} = $props->{days};
             }
             else {
-                $self->{error} = $self->{type} . " service not available";
+                $self->{error} = $self->{type} . " service not available to this location (check your postcode)";
             }
         }
         else {
@@ -82,9 +92,12 @@ sub calculate_shipping {
         }
     }
     else {
+        $trace .= "Error: ". $r->status_line . "\n";
+       $debug and print STDERR "Failure: [",$r->status_line,"]\n";
         warn $u->as_string(). ": ". $r->status_line, "\n";
         $self->{error} = "Server error";
     }
+    $self->{trace} = $trace;
 }
 
 package Courier::Fastway::Parser;
index 910591a..ae968fb 100644 (file)
@@ -20,7 +20,7 @@ sub can_deliver {
 sub calculate_shipping {
     my ($self) = @_;
 
-    $self->{shipping_cost} = 0;
+    $self->{cost} = 0;
     $self->{error} = "OK";
     $self->{days} = 0;
 }
index c9bd385..8b519fb 100644 (file)
@@ -93,12 +93,18 @@ function BSE_validateForm {
             money item retailPrice :></b></font></td>
         </tr>
         <:iterator end items:> 
-        <:if shipping_cost:>
+        <:if Shipping_cost:>
         <tr valign="middle" align="center" bgcolor="#FFFFFF"> 
           <td colspan=2 width="100%" align="left">&nbsp;<font face="Verdana, Arial, Helvetica, sans-serif" size="-2">Shipping charges (for <:shipping_method:><:if delivery_in:>, delivery in <:delivery_in:> days<:or delivery_in:><:eif delivery_in:>)</font></td>
           <td align="right"> <font face="Verdana, Arial, Helvetica, sans-serif" size="-2"><b>$<:money shipping_cost:></b></font></td>
         </tr>
-        <:or shipping_cost:><:eif shipping_cost:>
+        <:or Shipping_cost:><:eif Shipping_cost:>
+        <:if Eq [shipping_name] contact:>
+        <tr valign="middle" align="center" bgcolor="#FFFFFF"> 
+          <td colspan=2 width="100%" align="left">&nbsp;<font face="Verdana, Arial, Helvetica, sans-serif" size="-2">Shipping charges to be determined later</font></td>
+          <td align="right"> <font face="Verdana, Arial, Helvetica, sans-serif" size="-2"><b>$X.XX</b></font></td>
+        </tr>
+        <:or Eq:><:eif Eq:>
       </table>
     </td>
   </tr>
@@ -238,10 +244,13 @@ function BSE_validateForm {
       <td> <font face="Verdana, Arial, Helvetica, sans-serif" size="2"> 
         <textarea name="instructions" rows="5" cols="40" wrap="virtual"><:old instructions:></textarea></font><:error_img instructions:></td>
     </tr>
+<:if Shipping:>
     <tr>
       <td valign="top"><font face="Verdana, Arial, Helvetica, sans-serif" size="2">Shipping<br /> method:</font></td>
-      <td><font face="Verdana, Arial, Helvetica, sans-serif" size="2"><:shipping_select:></font> *</td>
+      <td><font face="Verdana, Arial, Helvetica, sans-serif" size="2"><:shipping_select:></font><:error_img shipping_name:> *
+</td>
     </tr>
+<:or Shipping:><:eif Shipping:>
     <tr> 
       <td valign="top"><font face="Verdana, Arial, Helvetica, sans-serif" size="2">Purchase<br />Order:</font></td>
       <td> <font face="Verdana, Arial, Helvetica, sans-serif" size="2"> 
index bee01d2..2084884 100644 (file)
@@ -336,6 +336,7 @@ Column billStreet2;varchar(127);NO;;
 Column purchase_order;varchar(80);NO;;
 Column shipping_method;varchar(64);NO;;
 Column shipping_name;varchar(40);NO;;
+Column shipping_trace;text;YES;NULL;
 Index PRIMARY;1;[id]
 Index order_cchash;0;[ccNumberHash]
 Index order_userId;0;[userId;orderDate]