Change how the shipping driver throws errors. Fixes bug #11202.

plainblack · Nov 4, 2009 · fc6acc7 · fc6acc7
1 parent 0e70378
commit fc6acc7
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 50 deletions.
diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
@@ -1,6 +1,7 @@
 7.8.4
  - Fixed a compatibility problem between WRE and new Spectre code.
  - fixed #11198: Typo in i18n
+ - fixed #11202: USPS driver does not log authentication errors
 
 7.8.3
  - Rewrote Spectre's workflow queues to prevent it from "forgetting" about some workflows.

diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm
@@ -6,6 +6,7 @@ use WebGUI::Exception;
 use XML::Simple;
 use LWP;
 use Tie::IxHash;
+use Data::Dumper;
 
 =head1 NAME
 
@@ -142,12 +143,12 @@ sub calculate {
     my $response = $self->_doXmlRequest($xml);
     ##Error handling
     if (! $response->is_success) {
-        WebGUI::Error::RemoteShippingRate->throw(error => 'Problem connecting to USPS Web Tools: '. $response->status_line);
+        WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Problem connecting to USPS Web Tools: '. $response->status_line);
     }
     my $returnedXML = $response->content;
-    my $xmlData     = XMLin($returnedXML, ForceArray => [qw/Package/]);
+    my $xmlData     = XMLin($returnedXML, KeepRoot => 1, ForceArray => [qw/Package/]);
     if (exists $xmlData->{Error}) {
-        WebGUI::Error::RemoteShippingRate->throw(error => 'Problem with USPS Web Tools XML: '. $xmlData->{Description});
+        WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Problem with USPS Web Tools XML: '. $xmlData->{Error}->{Description});
     }
     ##Summarize costs from returned data
     $cost = $self->_calculateFromXML($xmlData, @shippableUnits);
@@ -167,14 +168,16 @@ Processed XML data from an XML rate request, processed in perl data structure.
 have this structure:
 
     {
-        Package => [
-            {
-                ID => 0,
-                Postage => {
-                    Rate => some_number
-                }
-            },
-        ]
+        RateV3Response => {
+            Package => [
+                {
+                    ID => 0,
+                    Postage => {
+                        Rate => some_number
+                    }
+                },
+            ]
+        }
     }
 
 =head3 @shippableUnits
@@ -186,12 +189,12 @@ The set of shippable units, which are required to do quantity lookups.
 sub _calculateFromXML {
     my ($self, $xmlData, @shippableUnits) = @_;
     my $cost = 0;
-    foreach my $package (@{ $xmlData->{Package} }) {
+    foreach my $package (@{ $xmlData->{RateV3Response}->{Package} }) {
         my $id   = $package->{ID};
         my $rate = $package->{Postage}->{Rate};
         ##Error check for invalid index
         if ($id < 0 || $id > $#shippableUnits) {
-            WebGUI::Error::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id");
+            WebGUI::Error::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id");
         }
         my $unit = $shippableUnits[$id];
         if ($unit->[0]->getSku->shipsSeparately) {

diff --git 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 => 64;
+plan tests => 65;
 use_ok('WebGUI::Shop::ShipDriver::USPS')
     or die 'Unable to load module WebGUI::Shop::ShipDriver::USPS';
 
@@ -377,15 +377,18 @@ SKIP: {
 
 }
 
-my $cost = $driver->_calculateFromXML({
-    Package => [
-        {
-            ID => 0,
-            Postage => {
-                Rate => 5.25,
-            },
+my $cost = $driver->_calculateFromXML(
+    {
+        RateV3Response => {
+            Package => [
+                {
+                    ID => 0,
+                    Postage => {
+                        Rate => 5.25,
+                    },
+                },
+            ],
         },
-    ],
     },
     @shippableUnits
 );
@@ -471,21 +474,24 @@ SKIP: {
 
 }
 
-$cost = $driver->_calculateFromXML({
-    Package => [
-        {
-            ID => 0,
-            Postage => {
-                Rate => 7.00,
-            },
-        },
-        {
-            ID => 1,
-            Postage => {
-                Rate => 5.25,
-            },
+$cost = $driver->_calculateFromXML(
+    {
+        RateV3Response => {
+            Package => [
+                {
+                    ID => 0,
+                    Postage => {
+                        Rate => 7.00,
+                    },
+                },
+                {
+                    ID => 1,
+                    Postage => {
+                        Rate => 5.25,
+                    },
+                },
+            ],
         },
-    ],
     },
     @shippableUnits
 );
@@ -497,21 +503,24 @@ $bibleItem->setQuantity(2);
 
 is(calculateInsurance($driver), 8, '_calculateInsurance: two items in cart with quantity=2, calculates insurance');
 
-$cost = $driver->_calculateFromXML({
-    Package => [
-        {
-            ID => 0,
-            Postage => {
-                Rate => 7.00,
-            },
-        },
-        {
-            ID => 1,
-            Postage => {
-                Rate => 5.25,
-            },
+$cost = $driver->_calculateFromXML(
+    {
+        RateV3Response => {
+            Package => [
+                {
+                    ID => 0,
+                    Postage => {
+                        Rate => 7.00,
+                    },
+                },
+                {
+                    ID => 1,
+                    Postage => {
+                        Rate => 5.25,
+                    },
+                },
+            ],
         },
-    ],
     },
     @shippableUnits
 );
@@ -811,6 +820,24 @@ SKIP: {
 
 }
 
+#######################################################################
+#
+# Check for throwing an exception
+#
+#######################################################################
+
+my $userId  = $driver->get('userId');
+$properties = $driver->get();
+$properties->{userId} = '__NEVER_GOING_TO_HAPPEN__';
+$driver->update($properties);
+
+$cost = eval { $driver->calculate($cart); };
+$e = Exception::Class->caught();
+isa_ok($e, 'WebGUI::Error::Shop::RemoteShippingRate', 'calculate throws an exception when a bad userId is used');
+
+$properties->{userId} = $userId;
+$driver->update($properties);
+
 #######################################################################
 #
 # _calculateInsurance edge case