From 4ef66d1a51e0ecf8cbcdddc5306f21f5e66c9704 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 10 Oct 2009 15:06:13 -0700
Subject: [PATCH] More tests, refactor out rate parsing code.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm | 37 ++++++++++++++++++++++++++----
 t/Shop/ShipDriver/USPS.t           | 23 ++++++++++++++-----
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index f9b9295fd..8fe717c5d 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -223,10 +223,9 @@ sub _calculateInsurance {
     my ($self, @shippableUnits) = @_;
     my $insuranceCost  = 0;
     return $insuranceCost unless $self->get('addInsurance') && $self->get('insuranceRates');
-    my @insuranceTable = map { my ($value,$cost) = split /:/, $_; [$value, $cost]; }
-                            split /\r?\n/, $self->get('insuranceRates');
+    my @insuranceTable = _parseInsuranceRates($self->get('insuranceRates'));
     ##Sort by decreasing value for easy post processing
-    @insuranceTable = sort { $b->[0] <=> $a->[0] } @insuranceTable;
+    @insuranceTable = sort { $a->[0] <=> $b->[0] } @insuranceTable;
     foreach my $package (@shippableUnits) {
         my $value = 0;
         ITEM: foreach my $item (@{ $package }) {
@@ -234,18 +233,46 @@ sub _calculateInsurance {
         }
         my $pricePoint;
         POINT: foreach my $point (@insuranceTable) {
-            if ($value > $point->[0]) {
+            if ($value < $point->[0]) {
                 $pricePoint = $point;
                 last POINT;
             }
         }
-        $insuranceCost += defined $pricePoint ? $pricePoint->[1] : 0;
+        if (!defined $pricePoint) {
+            $pricePoint = $insuranceTable[-1];
+        }
+        $insuranceCost += $pricePoint->[1];
     }
     return $insuranceCost;
 }
 
 #-------------------------------------------------------------------
 
+=head2 _parseInsuranceRates ( $rates )
+
+Take the user entered data, a string, and turn it into an array.
+
+=head3 $rates
+
+The rate data entered by the user.  One set of data per line.  Each line has the value of
+shipment, a colon, and the cost of insuring a shipment of that value.
+
+=cut
+
+sub _parseInsuranceRates {
+    my $rates = shift;
+    my @lines = split /\r?\n/, $rates;
+    my @table = ();
+    foreach my $line (@lines) {
+        $line =~ s/\s+//g;
+        my ($value, $cost) = split /:/, $line;
+        push @table, [ $value, $cost ];
+    }
+    return @table;
+}
+
+#-------------------------------------------------------------------
+
 =head2 definition ( $session )
 
 This subroutine returns an arrayref of hashrefs, used to validate data put into
diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index eb5a10c24..b54229474 100644
--- a/t/Shop/ShipDriver/USPS.t
+++ b/t/Shop/ShipDriver/USPS.t
@@ -24,7 +24,7 @@ use Data::Dumper;
 use WebGUI::Test; # Must use this before any other WebGUI modules
 use WebGUI::Session;
 
-plan tests => 51;
+plan tests => 53;
 use_ok('WebGUI::Shop::ShipDriver::USPS')
     or die 'Unable to load module WebGUI::Shop::ShipDriver::USPS';
 
@@ -48,7 +48,8 @@ my $insuranceTable =  <<EOTABLE;
 10:2.00
 15:3.00
 20:4.00
-25:50.00
+25:5.00
+30:6.00
 EOTABLE
 
 my $versionTag = WebGUI::VersionTag->getWorking($session);
@@ -303,12 +304,17 @@ $properties->{addInsurance}   = 1;
 $properties->{insuranceRates} = $insuranceTable;
 $driver->update($properties);
 
-is($driver->_calculateInsurance(@shippableUnits), 1, '_calculateInsurance: one item in cart with quantity=1, calculates insurance');
+is($driver->_calculateInsurance(@shippableUnits), 2, '_calculateInsurance: one item in cart with quantity=1, calculates insurance');
 
 $properties->{addInsurance}   = 0;
 $driver->update($properties);
 is($driver->_calculateInsurance(@shippableUnits), 0, '_calculateInsurance: returns 0 if insurance is not enabled');
 
+$properties->{addInsurance}   = 1;
+$properties->{insuranceRates} = '';
+$driver->update($properties);
+is($driver->_calculateInsurance(@shippableUnits), 0, '_calculateInsurance: returns 0 if rates are not set');
+
 my $xml = $driver->buildXML($cart, @shippableUnits);
 like($xml, qr/<RateV3Request USERID="[^"]+"/, 'buildXML: checking userId is an attribute of the RateV3Request tag');
 like($xml, qr/<Package ID="0"/, 'buildXML: checking ID is an attribute of the Package tag');
@@ -386,7 +392,7 @@ is($cost, 5.25, '_calculateFromXML calculates shipping cost correctly for 1 item
 $bibleItem = $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $nivBible));
 @shippableUnits = $driver->_getShippableUnits($cart);
 
-is(calculateInsurance($driver), 5, '_calculateInsurance: two items in cart with quantity=1, calculates insurance');
+is(calculateInsurance($driver), 7, '_calculateInsurance: two items in cart with quantity=1, calculates insurance');
 
 $xml = $driver->buildXML($cart, @shippableUnits);
 $xmlData = XMLin( $xml,
@@ -804,9 +810,13 @@ SKIP: {
 
 #######################################################################
 #
-# Test PRIORITY VARIABLE shipping setup
+# _calculateInsurance edge case
 #
 #######################################################################
+$cart->empty;
+$bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $gospels));
+@shippableUnits = $driver->_getShippableUnits($cart);
+is(calculateInsurance($driver), 1, '_calculateInsurance: calculates insurance using the first bin');
 
 #----------------------------------------------------------------------------
 # Cleanup
@@ -823,8 +833,9 @@ END {
 
 sub calculateInsurance {
     my $driver = shift;
-    $properties = $driver->get();
+    my $properties = $driver->get();
     $properties->{addInsurance}   = 1;
+    $properties->{insuranceRates} = $insuranceTable;
     $driver->update($properties);
 
     my $insurance = $driver->_calculateInsurance(@shippableUnits);