From 4ee1f56aa3112a5ec16e0116e15163ad0bd90928 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 26 Sep 2009 11:18:32 -0700
Subject: [PATCH 01/11] Add UI and i18n for USPS insurance.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm         |  6 ++++++
 lib/WebGUI/i18n/English/ShipDriver_USPS.pm | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index 23e5faba3..c6eb02903 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -262,6 +262,12 @@ sub definition {
             options      => \%shippingTypes,
             defaultValue => 'PARCEL',
         },
+        addInsurance => {
+            fieldType    => 'yesNo',
+            label        => $i18n->get('add insurance'),
+            hoverHelp    => $i18n->get('add insurance help'),
+            defaultValue => 0,
+        },
 ##Note, if a flat fee is added to this driver, then according to the license
 ##terms the website must display a note to the user (shop customer) that additional
 ##fees have been added.
diff --git a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
index 3b9392784..e5d0fc6f6 100644
--- a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
+++ b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
@@ -94,6 +94,18 @@ our $I18N = {
         context => q|Label for a type of shipping from the USPS.|,
     },
 
+    'add insurance' => {
+        message => q|Ship with insurance?|,
+        lastUpdated => 1253988886,
+        context => q|Label for a type of shipping from the USPS.|,
+    },
+
+    'add insurance help' => {
+        message => q|If set to yes, the shipping plugin will ask the USPS for the cost of insuring this shipment.  The cost will be added to the total cost of shipping.  If insurance is not available, then the option to use this driver will not be presented to the user.|,
+        lastUpdated => 1253988884,
+        context => q|Label for a type of shipping from the USPS.|,
+    },
+
 };
 
 1;

From 2b4bfb8eece8af77f955aab88ff6b0dcda204806 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sun, 27 Sep 2009 14:25:50 -0700
Subject: [PATCH 02/11] Test whether ValueOfContents works in Domestic rate
 estimates.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm |  7 ++++
 t/Shop/ShipDriver/USPS.t           | 60 ++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index c6eb02903..a717d8daa 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -61,15 +61,19 @@ sub buildXML {
         next PACKAGE unless scalar @{ $package };
         tie my %packageData, 'Tie::IxHash';
         my $weight = 0;
+        my $cost   = 0;
         foreach my $item (@{ $package }) {
             my $sku = $item->getSku;
             my $itemWeight = $sku->getWeight();
+            my $itemCost   = $sku->getPrice();
             ##Items that ship separately with a quantity > 1 are rate estimated as 1 item and then the
             ##shipping cost is multiplied by the quantity.
             if (! $sku->shipsSeparately ) {
                 $itemWeight *= $item->get('quantity');
+                $itemCost   *= $item->get('quantity');
             }
             $weight += $itemWeight;
+            $cost   += $itemCost;
         }
         my $pounds = int($weight);
         my $ounces = int(16 * ($weight - $pounds));
@@ -89,6 +93,9 @@ sub buildXML {
         }
         $packageData{Size}            = [ 'REGULAR'               ];
         $packageData{Machinable}      = [ 'true'                  ];
+        if ($self->get('addInsurance')) {
+            $packageData{ValueOfContents} = $cost;
+        }
         push @{ $xmlTop->{Package} }, \%packageData;
     }
     my $xml = XMLout(\%xmlHash,
diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index fa63039dc..1c8f5637f 100644
--- a/t/Shop/ShipDriver/USPS.t
+++ b/t/Shop/ShipDriver/USPS.t
@@ -755,6 +755,66 @@ SKIP: {
 }
 
 
+$properties = $driver->get();
+$properties->{addInsurance} = 1;
+$driver->update($properties);
+
+$xml = $driver->buildXML($cart, @shippableUnits);
+my $xmlData = XMLin($xml,
+    KeepRoot   => 1,
+    ForceArray => ['Package'],
+);
+cmp_deeply(
+    $xmlData,
+    {
+        RateV3Request => {
+            USERID => $userId,
+            Package => [
+                {
+                    ID => 0,
+                    ZipDestination => '53715',    ZipOrigination  => '97123',
+                    Pounds         => '1',        Ounces          => '8',
+                    Size           => 'REGULAR',  Service         => 'PRIORITY',
+                    Machinable     => 'true',     ValueOfContents => 7.50,
+                },
+            ],
+        }
+    },
+    'buildXML: contains ValueOfContents when insurance is requested'
+);
+like($xml, qr/RateV3Request USERID.+?Package ID=.+?Service.+?ZipOrigination.+?ZipDestination.+?Pounds.+?Ounces.+?Size.+?Machinable/, '... and tag order');
+
+SKIP: {
+
+    skip 'No userId for testing', 2 unless $hasRealUserId;
+
+    my $response = $driver->_doXmlRequest($xml);
+    ok($response->is_success, '... _doXmlRequest to USPS successful');
+    my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],);
+    diag $response->content;
+#    cmp_deeply(
+#        $xmlData,
+#        {
+#            Package => [
+#                {
+#                    ID             => 0,
+#                    ZipOrigination => ignore(), ZipDestination => ignore(),
+#                    Ounces         => ignore(), Pounds         => ignore(),
+#                    Size           => ignore(), Zone           => ignore(),
+#                    Postage        => {
+#                        CLASSID     => ignore(),
+#                        MailService => ignore(),
+#                        Rate        => num(8,8),  ##A number around 10...
+#                    }
+#                },
+#            ],
+#        },
+#        '... returned data from USPS in correct format.  If this test fails, the driver may need to be updated'
+#    );
+
+}
+
+
 #----------------------------------------------------------------------------
 # Cleanup
 END {

From e81ec9718c76e349e7f9f78d3061fc4e441afc51 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Tue, 29 Sep 2009 19:28:07 -0700
Subject: [PATCH 03/11] Cleanup previous attempt at insurance.  Begin new
 attempt.

Add a field to the driver where the user can enter in insurance data with i18n.
Add a method to calculate the cost of insurance for all packages.
---
 lib/WebGUI/Shop/ShipDriver/USPS.pm         | 33 ++++++++++--
 lib/WebGUI/i18n/English/ShipDriver_USPS.pm | 14 ++++-
 t/Shop/ShipDriver/USPS.t                   | 62 +++-------------------
 3 files changed, 50 insertions(+), 59 deletions(-)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index a717d8daa..2ed5fd888 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -93,9 +93,6 @@ sub buildXML {
         }
         $packageData{Size}            = [ 'REGULAR'               ];
         $packageData{Machinable}      = [ 'true'                  ];
-        if ($self->get('addInsurance')) {
-            $packageData{ValueOfContents} = $cost;
-        }
         push @{ $xmlTop->{Package} }, \%packageData;
     }
     my $xml = XMLout(\%xmlHash,
@@ -158,6 +155,7 @@ sub calculate {
     }
     ##Summarize costs from returned data
     $cost = $self->_calculateFromXML($xmlData, @shippableUnits);
+    $cost += $self->_calculateInsurance(@shippableUnits);
     return $cost;
 }
 
@@ -215,6 +213,29 @@ sub _calculateFromXML {
 
 #-------------------------------------------------------------------
 
+=head2 _calculateInsurance ( @shippableUnits )
+
+Takes data from the USPS and returns the calculated shipping price.
+
+=head3 @shippableUnits
+
+The set of shippable units, which are required to do quantity and cost lookups.
+
+=cut
+
+sub _calculateInsurance {
+    my ($self, @shippableUnits) = @_;
+    my $cost = 0;
+    return $cost unless $self-get('addInsurance') && $self->get('insuranceRates');
+    my @insuranceTable = map { my ($cost,$value) = split /:/, $_; [$cost, $value]; }
+                            split /\r?\n/, $self->get('insuranceRates');
+    ##Sort by increasing value for easy post processing
+    my @insuranceTable = map { $_->[1] } sort { $a <=> $b } map { [ $_->[0], $_ ] } @insuranceTable;
+    return $cost;
+}
+
+#-------------------------------------------------------------------
+
 =head2 definition ( $session )
 
 This subroutine returns an arrayref of hashrefs, used to validate data put into
@@ -275,6 +296,12 @@ sub definition {
             hoverHelp    => $i18n->get('add insurance help'),
             defaultValue => 0,
         },
+        insuranceRates => {
+            fieldType    => 'textarea',
+            label        => $i18n->get('insurance rates'),
+            hoverHelp    => $i18n->get('insurance rates help'),
+            defaultValue => "50:1.75\n100:2.25",
+        },
 ##Note, if a flat fee is added to this driver, then according to the license
 ##terms the website must display a note to the user (shop customer) that additional
 ##fees have been added.
diff --git a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
index e5d0fc6f6..b31b228bf 100644
--- a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
+++ b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
@@ -97,7 +97,7 @@ our $I18N = {
     'add insurance' => {
         message => q|Ship with insurance?|,
         lastUpdated => 1253988886,
-        context => q|Label for a type of shipping from the USPS.|,
+        context => q|Label for the edit screen.|,
     },
 
     'add insurance help' => {
@@ -106,6 +106,18 @@ our $I18N = {
         context => q|Label for a type of shipping from the USPS.|,
     },
 
+    'insurance rates' => {
+        message => q|Insurance Rate Table|,
+        lastUpdated => 1253988886,
+        context => q|Label for the edit screen.|,
+    },
+
+    'insurance rates help' => {
+        message => q|Enter in one field per line with the format, value:cost.<br />value is the value of the contents.<br />cost is the cost of insurance at that value.|,
+        lastUpdated => 1253988884,
+        context => q|Help for the insurance rate field.|,
+    },
+
 };
 
 1;
diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index 1c8f5637f..6f8ad6cf7 100644
--- a/t/Shop/ShipDriver/USPS.t
+++ b/t/Shop/ShipDriver/USPS.t
@@ -757,63 +757,15 @@ SKIP: {
 
 $properties = $driver->get();
 $properties->{addInsurance} = 1;
+$properties->{insuranceTable} = <<EOTABLE;
+5:1.00
+10:2.00
+15:3.00
+20:4.00
+25:50.00
+EOTABLE
 $driver->update($properties);
 
-$xml = $driver->buildXML($cart, @shippableUnits);
-my $xmlData = XMLin($xml,
-    KeepRoot   => 1,
-    ForceArray => ['Package'],
-);
-cmp_deeply(
-    $xmlData,
-    {
-        RateV3Request => {
-            USERID => $userId,
-            Package => [
-                {
-                    ID => 0,
-                    ZipDestination => '53715',    ZipOrigination  => '97123',
-                    Pounds         => '1',        Ounces          => '8',
-                    Size           => 'REGULAR',  Service         => 'PRIORITY',
-                    Machinable     => 'true',     ValueOfContents => 7.50,
-                },
-            ],
-        }
-    },
-    'buildXML: contains ValueOfContents when insurance is requested'
-);
-like($xml, qr/RateV3Request USERID.+?Package ID=.+?Service.+?ZipOrigination.+?ZipDestination.+?Pounds.+?Ounces.+?Size.+?Machinable/, '... and tag order');
-
-SKIP: {
-
-    skip 'No userId for testing', 2 unless $hasRealUserId;
-
-    my $response = $driver->_doXmlRequest($xml);
-    ok($response->is_success, '... _doXmlRequest to USPS successful');
-    my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],);
-    diag $response->content;
-#    cmp_deeply(
-#        $xmlData,
-#        {
-#            Package => [
-#                {
-#                    ID             => 0,
-#                    ZipOrigination => ignore(), ZipDestination => ignore(),
-#                    Ounces         => ignore(), Pounds         => ignore(),
-#                    Size           => ignore(), Zone           => ignore(),
-#                    Postage        => {
-#                        CLASSID     => ignore(),
-#                        MailService => ignore(),
-#                        Rate        => num(8,8),  ##A number around 10...
-#                    }
-#                },
-#            ],
-#        },
-#        '... returned data from USPS in correct format.  If this test fails, the driver may need to be updated'
-#    );
-
-}
-
 
 #----------------------------------------------------------------------------
 # Cleanup

From 5c84eb930686242ef29315b6d762b28a22fea659 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Wed, 7 Oct 2009 22:16:30 -0700
Subject: [PATCH 04/11] Finish building insurance calculation method.  Needs
 tests.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index 2ed5fd888..38181777c 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -61,19 +61,15 @@ sub buildXML {
         next PACKAGE unless scalar @{ $package };
         tie my %packageData, 'Tie::IxHash';
         my $weight = 0;
-        my $cost   = 0;
         foreach my $item (@{ $package }) {
             my $sku = $item->getSku;
             my $itemWeight = $sku->getWeight();
-            my $itemCost   = $sku->getPrice();
             ##Items that ship separately with a quantity > 1 are rate estimated as 1 item and then the
             ##shipping cost is multiplied by the quantity.
             if (! $sku->shipsSeparately ) {
                 $itemWeight *= $item->get('quantity');
-                $itemCost   *= $item->get('quantity');
             }
             $weight += $itemWeight;
-            $cost   += $itemCost;
         }
         my $pounds = int($weight);
         my $ounces = int(16 * ($weight - $pounds));
@@ -226,12 +222,23 @@ The set of shippable units, which are required to do quantity and cost lookups.
 sub _calculateInsurance {
     my ($self, @shippableUnits) = @_;
     my $cost = 0;
-    return $cost unless $self-get('addInsurance') && $self->get('insuranceRates');
-    my @insuranceTable = map { my ($cost,$value) = split /:/, $_; [$cost, $value]; }
+    return $cost unless $self->get('addInsurance') && $self->get('insuranceRates');
+    my @insuranceTable = map { my ($value,$cost) = split /:/, $_; [$value, $cost]; }
                             split /\r?\n/, $self->get('insuranceRates');
-    ##Sort by increasing value for easy post processing
-    my @insuranceTable = map { $_->[1] } sort { $a <=> $b } map { [ $_->[0], $_ ] } @insuranceTable;
-    return $cost;
+    ##Sort by decreasing value for easy post processing
+    @insuranceTable = sort { $b->[0] <=> $a->[0] } @insuranceTable;
+    my $insuranceCost  = 0;
+    foreach my $package (@shippableUnits) {
+        ITEM: foreach my $item (@{ $package }) {
+            $cost += $item->getSku->getPrice() * $item->get('quantity');
+        }
+        my $pricePoint;
+        POINT: foreach $pricePoint (@insuranceTable) {
+            last POINT if $cost <= $pricePoint->[0];
+        }
+        $insuranceCost += $pricePoint->[1];
+    }
+    return $insuranceCost;
 }
 
 #-------------------------------------------------------------------

From 1f1f3c38ae0ad840e23656186c97dde48727b318 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Thu, 8 Oct 2009 12:54:11 -0700
Subject: [PATCH 05/11] Make sure to access the correct $SESSION.

---
 t/lib/WebGUI/Test.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib/WebGUI/Test.pm b/t/lib/WebGUI/Test.pm
index 04b6eb9d2..649f73356 100644
--- a/t/lib/WebGUI/Test.pm
+++ b/t/lib/WebGUI/Test.pm
@@ -139,7 +139,7 @@ sub cleanup {
     pop @guarded
         while @guarded;
 
-    if ( $SESSION ) {
+    if ( our $SESSION ) {
         $SESSION->var->end;
         $SESSION->close;
         undef $SESSION;

From 829c8ce9d869bc611930090884182e55194af9f8 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Fri, 9 Oct 2009 16:28:20 -0700
Subject: [PATCH 06/11] USPS Insurance estimation code and tests.  Basically
 working.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm | 17 +++---
 t/Shop/ShipDriver/USPS.t           | 87 ++++++++++++++++++++++++------
 2 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index 38181777c..f9b9295fd 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -221,22 +221,25 @@ The set of shippable units, which are required to do quantity and cost lookups.
 
 sub _calculateInsurance {
     my ($self, @shippableUnits) = @_;
-    my $cost = 0;
-    return $cost unless $self->get('addInsurance') && $self->get('insuranceRates');
+    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');
     ##Sort by decreasing value for easy post processing
     @insuranceTable = sort { $b->[0] <=> $a->[0] } @insuranceTable;
-    my $insuranceCost  = 0;
     foreach my $package (@shippableUnits) {
+        my $value = 0;
         ITEM: foreach my $item (@{ $package }) {
-            $cost += $item->getSku->getPrice() * $item->get('quantity');
+            $value += $item->getSku->getPrice() * $item->get('quantity');
         }
         my $pricePoint;
-        POINT: foreach $pricePoint (@insuranceTable) {
-            last POINT if $cost <= $pricePoint->[0];
+        POINT: foreach my $point (@insuranceTable) {
+            if ($value > $point->[0]) {
+                $pricePoint = $point;
+                last POINT;
+            }
         }
-        $insuranceCost += $pricePoint->[1];
+        $insuranceCost += defined $pricePoint ? $pricePoint->[1] : 0;
     }
     return $insuranceCost;
 }
diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index 6f8ad6cf7..eb5a10c24 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 => 46;
+plan tests => 51;
 use_ok('WebGUI::Shop::ShipDriver::USPS')
     or die 'Unable to load module WebGUI::Shop::ShipDriver::USPS';
 
@@ -42,8 +42,15 @@ $session->user({user => $user});
 # put your tests here
 
 
-my $storage;
 my ($driver, $cart);
+my $insuranceTable =  <<EOTABLE;
+5:1.00
+10:2.00
+15:3.00
+20:4.00
+25:50.00
+EOTABLE
+
 my $versionTag = WebGUI::VersionTag->getWorking($session);
 
 my $home = WebGUI::Asset->getDefault($session);
@@ -92,8 +99,16 @@ my $nivBible = $bible->setCollateral('variantsJSON', 'variantId', 'new',
     }
 );
 
+my $gospels = $bible->setCollateral('variantsJSON', 'variantId', 'new',
+    {
+        shortdesc => 'Gospels from the new Testament',
+        price     => 1.50,       varSku    => 'gospels',
+        weight    => 2.0,        quantity  => 999999,
+    }
+);
+
 $versionTag->commit;
-WebGUI::Test->tagsToRollback($versionTag);
+addToCleanup($versionTag);
 
 #######################################################################
 #
@@ -283,6 +298,17 @@ $driver->update($properties);
 $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer));
 my @shippableUnits = $driver->_getShippableUnits($cart);
 
+$properties = $driver->get();
+$properties->{addInsurance}   = 1;
+$properties->{insuranceRates} = $insuranceTable;
+$driver->update($properties);
+
+is($driver->_calculateInsurance(@shippableUnits), 1, '_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');
+
 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');
@@ -359,8 +385,10 @@ is($cost, 5.25, '_calculateFromXML calculates shipping cost correctly for 1 item
 
 $bibleItem = $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $nivBible));
 @shippableUnits = $driver->_getShippableUnits($cart);
-$xml = $driver->buildXML($cart, @shippableUnits);
 
+is(calculateInsurance($driver), 5, '_calculateInsurance: two items in cart with quantity=1, calculates insurance');
+
+$xml = $driver->buildXML($cart, @shippableUnits);
 $xmlData = XMLin( $xml,
     KeepRoot   => 1,
     ForceArray => ['Package'],
@@ -458,6 +486,8 @@ is($cost, 12.25, '_calculateFromXML calculates shipping cost correctly for 2 ite
 $bibleItem->setQuantity(2);
 @shippableUnits = $driver->_getShippableUnits($cart);
 
+is(calculateInsurance($driver), 51, '_calculateInsurance: two items in cart with quantity=2, calculates insurance');
+
 $cost = $driver->_calculateFromXML({
     Package => [
         {
@@ -481,6 +511,7 @@ is($cost, 19.25, '_calculateFromXML calculates shipping cost correctly for 2 ite
 $rockHammer2 = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer));
 $rockHammer2->update({shippingAddressId => $wucAddress->getId});
 @shippableUnits = $driver->_getShippableUnits($cart);
+is(calculateInsurance($driver), 54, '_calculateInsurance: calculates insurance');
 $xml = $driver->buildXML($cart, @shippableUnits);
 
 $xmlData = XMLin( $xml,
@@ -575,6 +606,12 @@ SKIP: {
 
 }
 
+#######################################################################
+#
+# Test Priority shipping setup
+#
+#######################################################################
+
 $cart->empty;
 $properties = $driver->get();
 $properties->{shipType} = 'PRIORITY';
@@ -637,6 +674,12 @@ SKIP: {
 
 }
 
+#######################################################################
+#
+# Test EXPRESS shipping setup
+#
+#######################################################################
+
 $properties = $driver->get();
 $properties->{shipType} = 'EXPRESS';
 $driver->update($properties);
@@ -695,6 +738,11 @@ SKIP: {
 
 }
 
+#######################################################################
+#
+# Test PRIORITY VARIABLE shipping setup
+#
+#######################################################################
 
 $properties = $driver->get();
 $properties->{shipType} = 'PRIORITY VARIABLE';
@@ -754,18 +802,11 @@ SKIP: {
 
 }
 
-
-$properties = $driver->get();
-$properties->{addInsurance} = 1;
-$properties->{insuranceTable} = <<EOTABLE;
-5:1.00
-10:2.00
-15:3.00
-20:4.00
-25:50.00
-EOTABLE
-$driver->update($properties);
-
+#######################################################################
+#
+# Test PRIORITY VARIABLE shipping setup
+#
+#######################################################################
 
 #----------------------------------------------------------------------------
 # Cleanup
@@ -779,3 +820,17 @@ END {
         $cart->delete;
     }
 }
+
+sub calculateInsurance {
+    my $driver = shift;
+    $properties = $driver->get();
+    $properties->{addInsurance}   = 1;
+    $driver->update($properties);
+
+    my $insurance = $driver->_calculateInsurance(@shippableUnits);
+
+    $properties->{addInsurance}   = 0;
+    $driver->update($properties);
+
+    return $insurance;
+}

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 07/11] 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);

From 0273e2da572517337131c6fd982ad80b401c1055 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 10 Oct 2009 16:27:12 -0700
Subject: [PATCH 08/11] Make instructions about rate formats more explicit.

---
 lib/WebGUI/i18n/English/ShipDriver_USPS.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
index b31b228bf..036ee49d2 100644
--- a/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
+++ b/lib/WebGUI/i18n/English/ShipDriver_USPS.pm
@@ -113,7 +113,7 @@ our $I18N = {
     },
 
     'insurance rates help' => {
-        message => q|Enter in one field per line with the format, value:cost.<br />value is the value of the contents.<br />cost is the cost of insurance at that value.|,
+        message => q|Enter in one field per line with the format, value:cost.<br />value is the value of the contents.<br />cost is the cost of insurance at that value.<br />value and cost should look like numbers with a decimal point, like 0.50 or 1.00|,
         lastUpdated => 1253988884,
         context => q|Help for the insurance rate field.|,
     },

From 23b1cb47c5a1088b0d43357a932dc65caaf561bf Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 10 Oct 2009 16:27:33 -0700
Subject: [PATCH 09/11] Parse tests, handling spaces.

---
 lib/WebGUI/Shop/ShipDriver/USPS.pm |  6 ++++-
 t/Shop/ShipDriver/USPS.t           | 36 +++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
index 8fe717c5d..50a2f3fc9 100644
--- a/lib/WebGUI/Shop/ShipDriver/USPS.pm
+++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -261,7 +261,11 @@ shipment, a colon, and the cost of insuring a shipment of that value.
 
 sub _parseInsuranceRates {
     my $rates = shift;
-    my @lines = split /\r?\n/, $rates;
+    $rates =~ tr/\r//d;
+    my $number = qr/\d+(?:\.\d+)?/;
+    my $rate   = qr{ \s* $number \s* : \s* $number \s* }x;
+    return () if ($rates !~ m{ \A (?: $rate \r?\n )* $rate (?:\r\n)? \Z }x);
+    my @lines = split /\n/, $rates;
     my @table = ();
     foreach my $line (@lines) {
         $line =~ s/\s+//g;
diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index b54229474..65bd7149e 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 => 53;
+plan tests => 64;
 use_ok('WebGUI::Shop::ShipDriver::USPS')
     or die 'Unable to load module WebGUI::Shop::ShipDriver::USPS';
 
@@ -492,7 +492,7 @@ is($cost, 12.25, '_calculateFromXML calculates shipping cost correctly for 2 ite
 $bibleItem->setQuantity(2);
 @shippableUnits = $driver->_getShippableUnits($cart);
 
-is(calculateInsurance($driver), 51, '_calculateInsurance: two items in cart with quantity=2, calculates insurance');
+is(calculateInsurance($driver), 8, '_calculateInsurance: two items in cart with quantity=2, calculates insurance');
 
 $cost = $driver->_calculateFromXML({
     Package => [
@@ -517,7 +517,7 @@ is($cost, 19.25, '_calculateFromXML calculates shipping cost correctly for 2 ite
 $rockHammer2 = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer));
 $rockHammer2->update({shippingAddressId => $wucAddress->getId});
 @shippableUnits = $driver->_getShippableUnits($cart);
-is(calculateInsurance($driver), 54, '_calculateInsurance: calculates insurance');
+is(calculateInsurance($driver), 12, '_calculateInsurance: calculates insurance');
 $xml = $driver->buildXML($cart, @shippableUnits);
 
 $xmlData = XMLin( $xml,
@@ -818,6 +818,36 @@ $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $gospels));
 @shippableUnits = $driver->_getShippableUnits($cart);
 is(calculateInsurance($driver), 1, '_calculateInsurance: calculates insurance using the first bin');
 
+#######################################################################
+#
+# _parseInsuranceRates
+#
+#######################################################################
+
+my @rates;
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("");
+cmp_deeply(\@rates, [], '_parseInsuranceRates: empty string returns empty array');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates();
+cmp_deeply(\@rates, [], '_parseInsuranceRates: undef returns empty array');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2");
+cmp_deeply(\@rates, [], '... bad rates #1');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates(":2");
+cmp_deeply(\@rates, [], '... bad rates #2');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("a:b");
+cmp_deeply(\@rates, [], '... bad rates #3');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2:2");
+cmp_deeply(\@rates, [ ['2', '2'] ], '... one line of good rates');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2.0:2.0");
+cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with decimal points');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2.0:2.0\n");
+cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with newline');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2.0:2.0\r\n");
+cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with cr/newline');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("2.0 : 2.0\r\n");
+cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with cr/newline and spaces');
+@rates = WebGUI::Shop::ShipDriver::USPS::_parseInsuranceRates("  2.0 : 2.0  \r\n");
+cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with cr/newline and more spaces');
+
 #----------------------------------------------------------------------------
 # Cleanup
 END {

From 359882cc212c76540a5be0d4bce4bfa606a3d60b Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 10 Oct 2009 16:49:36 -0700
Subject: [PATCH 10/11] Add cleanup for shopping carts and shipping drivers.

Check for leaky carts and drivers.
When cleaning up a cart, also clean up its address book.
---
 t/lib/WebGUI/Test.pm | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/t/lib/WebGUI/Test.pm b/t/lib/WebGUI/Test.pm
index 649f73356..2a0472bb1 100644
--- a/t/lib/WebGUI/Test.pm
+++ b/t/lib/WebGUI/Test.pm
@@ -100,14 +100,16 @@ sub import {
     if ($ENV{WEBGUI_TEST_DEBUG}) {
         ##Offset Sessions, and Scratch by 1 because 1 will exist at the start
         my @checkCount = (
-            Sessions  => 'userSession',
-            Scratch   => 'userSessionScratch',
-            Users     => 'users',
-            Groups    => 'groups',
-            mailQ     => 'mailQueue',
-            Tags      => 'assetVersionTag',
-            Assets    => 'assetData',
-            Workflows => 'Workflow',
+            Sessions        => 'userSession',
+            Scratch         => 'userSessionScratch',
+            Users           => 'users',
+            Groups          => 'groups',
+            mailQ           => 'mailQueue',
+            Tags            => 'assetVersionTag',
+            Assets          => 'assetData',
+            Workflows       => 'Workflow',
+            Carts           => 'cart',
+            'Ship Drivers'  => 'shipper',
         );
         my %initCounts;
         for ( my $i = 0; $i < @checkCount; $i += 2) {
@@ -763,6 +765,8 @@ were passed in.  Currently able to destroy:
     WebGUI::User
     WebGUI::VersionTag
     WebGUI::Workflow
+    WebGUI::Shop::Cart
+    WebGUI::Shop::ShipDriver
 
 Example call:
 
@@ -831,14 +835,20 @@ Example call:
     );
 
     my %cleanup = (
-        'WebGUI::User'       => 'delete',
-        'WebGUI::Group'      => 'delete',
-        'WebGUI::Storage'    => 'delete',
-        'WebGUI::Shop::Cart' => 'delete',
-        'WebGUI::Asset'      => 'purge',
-        'WebGUI::VersionTag' => 'rollback',
-        'WebGUI::Workflow'   => 'delete',
-        'WebGUI::Session'    => sub {
+        'WebGUI::User'             => 'delete',
+        'WebGUI::Group'            => 'delete',
+        'WebGUI::Storage'          => 'delete',
+        'WebGUI::Asset'            => 'purge',
+        'WebGUI::VersionTag'       => 'rollback',
+        'WebGUI::Workflow'         => 'delete',
+        'WebGUI::Shop::ShipDriver' => 'delete',
+        'WebGUI::Shop::Cart'       => sub {
+            my $cart        = shift;
+            my $addressBook = $cart->getAddressBook();
+            $addressBook->delete if $addressBook;  ##Should we call cleanupGuard instead???
+            $cart->delete;
+        },
+        'WebGUI::Session'          => sub {
             my $session = shift;
             $session->var->end;
             $session->close;

From 993f6d0cc2006ad1b633bc66fcb339ce4c54d8a1 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Sat, 10 Oct 2009 16:51:57 -0700
Subject: [PATCH 11/11] Use the new Shop cleanup for carts and shipping
 drivers.

---
 t/Shop/ShipDriver/USPS.t | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/t/Shop/ShipDriver/USPS.t b/t/Shop/ShipDriver/USPS.t
index 65bd7149e..9a73f28dc 100644
--- a/t/Shop/ShipDriver/USPS.t
+++ b/t/Shop/ShipDriver/USPS.t
@@ -42,7 +42,7 @@ $session->user({user => $user});
 # put your tests here
 
 
-my ($driver, $cart);
+my ($driver2, $cart);
 my $insuranceTable =  <<EOTABLE;
 5:1.00
 10:2.00
@@ -149,10 +149,11 @@ my $options = {
                 enabled => 1,
               };
 
-$driver = WebGUI::Shop::ShipDriver::USPS->create($session, $options);
+$driver2 = WebGUI::Shop::ShipDriver::USPS->create($session, $options);
+addToCleanup($driver2);
 
-isa_ok($driver, 'WebGUI::Shop::ShipDriver::USPS');
-isa_ok($driver, 'WebGUI::Shop::ShipDriver');
+isa_ok($driver2, 'WebGUI::Shop::ShipDriver::USPS');
+isa_ok($driver2, 'WebGUI::Shop::ShipDriver');
 
 #######################################################################
 #
@@ -168,13 +169,13 @@ is (WebGUI::Shop::ShipDriver::USPS->getName($session), 'U.S. Postal Service', 'g
 #
 #######################################################################
 
-my $driverId = $driver->getId;
-$driver->delete;
+my $driverId = $driver2->getId;
+$driver2->delete;
 
 my $count = $session->db->quickScalar('select count(*) from shipper where shipperId=?',[$driverId]);
 is($count, 0, 'delete deleted the object');
 
-undef $driver;
+undef $driver2;
 
 #######################################################################
 #
@@ -182,11 +183,12 @@ undef $driver;
 #
 #######################################################################
 
-$driver = WebGUI::Shop::ShipDriver::USPS->create($session, {
+my $driver = WebGUI::Shop::ShipDriver::USPS->create($session, {
     label    => 'Shipping from Shawshank',
     enabled  => 1,
     shipType => 'PARCEL',
 });
+addToCleanup($driver);
 
 eval { $driver->calculate() };
 $e = Exception::Class->caught();
@@ -215,6 +217,7 @@ cmp_deeply(
 );
 
 $cart = WebGUI::Shop::Cart->newBySession($session);
+addToCleanup($cart);
 my $addressBook = $cart->getAddressBook;
 my $workAddress = $addressBook->addAddress({
     label => 'work',
@@ -850,16 +853,6 @@ cmp_deeply(\@rates, [ ['2.0', '2.0'] ], '... one line of good rates with cr/newl
 
 #----------------------------------------------------------------------------
 # Cleanup
-END {
-    if (defined $driver && $driver->isa('WebGUI::Shop::ShipDriver')) {
-        $driver->delete;
-    }
-    if (defined $cart && $cart->isa('WebGUI::Shop::Cart')) {
-        my $addressBook = $cart->getAddressBook();
-        $addressBook->delete if $addressBook;
-        $cart->delete;
-    }
-}
 
 sub calculateInsurance {
     my $driver = shift;