From 15514cb97a54b1f17f4e6ef86f860f52281a3003 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Thu, 5 Nov 2009 12:50:00 -0800 Subject: [PATCH 01/20] Begin international USPS shipping driver. --- .../Shop/ShipDriver/USPSInternational.pm | 358 ++++++++++++++++++ .../English/ShipDriver_USPSInternational.pm | 15 + 2 files changed, 373 insertions(+) create mode 100644 lib/WebGUI/Shop/ShipDriver/USPSInternational.pm create mode 100644 lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm diff --git a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm new file mode 100644 index 000000000..a927e3d44 --- /dev/null +++ b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm @@ -0,0 +1,358 @@ +package WebGUI::Shop::ShipDriver::USPSInternational; + +use strict; +use base qw/WebGUI::Shop::ShipDriver/; +use WebGUI::Exception; +use XML::Simple; +use LWP; +use Tie::IxHash; +use Data::Dumper; + +=head1 NAME + +Package WebGUI::Shop::ShipDriver::USPSInternational + +=head1 DESCRIPTION + +Shipping driver for the United States Postal Service, international shipping services. + +=head1 SYNOPSIS + +=head1 METHODS + +See the master class, WebGUI::Shop::ShipDriver for information about +base methods. These methods are customized in this class: + +=cut + +#------------------------------------------------------------------- + +=head2 buildXML ( $cart, @packages ) + +Returns XML for submitting to the US Postal Service servers + +=head3 $cart + +A WebGUI::Shop::Cart object. This allows us access to the user's +address book + +=head3 @packages + +An array of array references. Each array element is 1 set of items. The +quantity of items will vary in each set. If the quantity of an item +is more than 1, then we will check for shipping 1 item, and multiple the +result by the quantity, rather than doing several identical checks. + +=cut + +sub buildXML { + my ($self, $cart, @packages) = @_; + tie my %xmlHash, 'Tie::IxHash'; + %xmlHash = ( RateV3Request => {}, ); + my $xmlTop = $xmlHash{RateV3Request}; + $xmlTop->{USERID} = $self->get('userId'); + $xmlTop->{Package} = []; + ##Do a request for each package. + my $packageIndex; + my $shipType = $self->get('shipType'); + my $service = $shipType eq 'PRIORITY VARIABLE' ? 'PRIORITY' + : $shipType; + PACKAGE: for(my $packageIndex = 0; $packageIndex < scalar @packages; $packageIndex++) { + my $package = $packages[$packageIndex]; + next PACKAGE unless scalar @{ $package }; + tie my %packageData, 'Tie::IxHash'; + my $weight = 0; + foreach my $item (@{ $package }) { + my $sku = $item->getSku; + my $itemWeight = $sku->getWeight(); + ##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'); + } + $weight += $itemWeight; + } + my $pounds = int($weight); + my $ounces = sprintf '%3.1f', (16 * ($weight - $pounds)); + if ($pounds == 0 && $ounces eq '0.0' ) { + $ounces = 0.1; + } + my $destination = $package->[0]->getShippingAddress; + my $destZipCode = $destination->get('code'); + $packageData{ID} = $packageIndex; + $packageData{Service} = [ $service ]; + $packageData{ZipOrigination} = [ $self->get('sourceZip') ]; + $packageData{ZipDestination} = [ $destZipCode ]; + $packageData{Pounds} = [ $pounds ]; + $packageData{Ounces} = [ $ounces ]; + if ($shipType eq 'PRIORITY') { + $packageData{Container} = [ 'FLAT RATE BOX' ]; + } + elsif ($shipType eq 'PRIORITY VARIABLE') { + #$packageData{Container} = [ 'VARIABLE' ]; + } + $packageData{Size} = [ 'REGULAR' ]; + $packageData{Machinable} = [ 'true' ]; + push @{ $xmlTop->{Package} }, \%packageData; + } + my $xml = XMLout(\%xmlHash, + KeepRoot => 1, + NoSort => 1, + NoIndent => 1, + KeyAttr => { + Package => 'ID', + }, + SuppressEmpty => 0, + ); + return $xml; +} + + +#------------------------------------------------------------------- + +=head2 calculate ( $cart ) + +Returns a shipping price. + +=head3 $cart + +A WebGUI::Shop::Cart object. The contents of the cart are analyzed to calculate +the shipping costs. If no items in the cart require shipping, then no shipping +costs are assessed. + +=cut + +sub calculate { + my ($self, $cart) = @_; + if (! $self->get('sourceZip')) { + WebGUI::Error::InvalidParam->throw(error => q{Driver configured without a source zipcode.}); + } + if (! $self->get('userId')) { + WebGUI::Error::InvalidParam->throw(error => q{Driver configured without a USPS userId.}); + } + if ($cart->getShippingAddress->get('country') ne 'United States') { + WebGUI::Error::InvalidParam->throw(error => q{Driver only handles domestic shipping}); + } + my $cost = 0; + ##Sort the items into shippable bundles. + my @shippableUnits = $self->_getShippableUnits($cart); + my $packageCount = scalar @shippableUnits; + if ($packageCount > 25) { + WebGUI::Error::InvalidParam->throw(error => q{Cannot do USPS lookups for more than 25 items.}); + } + my $anyShippable = $packageCount > 0 ? 1 : 0; + return $cost unless $anyShippable; + #$cost = scalar @shippableUnits * $self->get('flatFee'); + ##Build XML ($cart, @shippableUnits) + my $xml = $self->buildXML($cart, @shippableUnits); + ##Do request ($xml) + my $response = $self->_doXmlRequest($xml); + ##Error handling + if (! $response->is_success) { + WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Problem connecting to USPS Web Tools: '. $response->status_line); + } + my $returnedXML = $response->content; + my $xmlData = XMLin($returnedXML, KeepRoot => 1, ForceArray => [qw/Package/]); + if (exists $xmlData->{Error}) { + 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); + return $cost; +} + +#------------------------------------------------------------------- + +=head2 _calculateFromXML ( $xmlData, @shippableUnits ) + +Takes data from the USPS and returns the calculated shipping price. + +=head3 $xmlData + +Processed XML data from an XML rate request, processed in perl data structure. The data is expected to +have this structure: + + { + RateV3Response => { + Package => [ + { + ID => 0, + Postage => { + Rate => some_number + } + }, + ] + } + } + +=head3 @shippableUnits + +The set of shippable units, which are required to do quantity lookups. + +=cut + +sub _calculateFromXML { + my ($self, $xmlData, @shippableUnits) = @_; + my $cost = 0; + 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::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id"); + } + if (exists $package->{Error}) { + WebGUI::Error::Shop::RemoteShippingRate->throw(error => $package->{Description}); + } + my $unit = $shippableUnits[$id]; + if ($unit->[0]->getSku->shipsSeparately) { + ##This is a single item due to ships separately. Since in reality there will be + ## N things being shipped, multiply the rate by the quantity. + $cost += $rate * $unit->[0]->get('quantity'); + } + else { + ##This is a loose bundle of items, all shipped together + $cost += $rate; + } + } + return $cost; +} + +#------------------------------------------------------------------- + +=head2 definition ( $session ) + +This subroutine returns an arrayref of hashrefs, used to validate data put into +the object by the user, and to automatically generate the edit form to show +the user. + +=cut + +sub definition { + my $class = shift; + my $session = shift; + WebGUI::Error::InvalidParam->throw(error => q{Must provide a session variable}) + unless ref $session eq 'WebGUI::Session'; + my $definition = shift || []; + my $i18n = WebGUI::International->new($session, 'ShipDriver_USPS'); + my $i18n2 = WebGUI::International->new($session, 'ShipDriver_USPSInternational'); + tie my %shippingTypes, 'Tie::IxHash'; + ##Note, these keys are used by buildXML + $shippingTypes{'PRIORITY VARIABLE'} = $i18n->get('priority variable'); + $shippingTypes{'PRIORITY'} = $i18n->get('priority'); + $shippingTypes{'EXPRESS' } = $i18n->get('express'); + $shippingTypes{'PARCEL' } = $i18n->get('parcel post'); + tie my %fields, 'Tie::IxHash'; + %fields = ( + instructions => { + fieldType => 'readOnly', + label => $i18n->get('instructions'), + defaultValue => $i18n->get('usps instructions'), + noFormProcess => 1, + }, + userId => { + fieldType => 'text', + label => $i18n->get('userid'), + hoverHelp => $i18n->get('userid help'), + defaultValue => '', + }, + shipType => { + fieldType => 'selectBox', + label => $i18n->get('ship type'), + hoverHelp => $i18n->get('ship type help'), + options => \%shippingTypes, + defaultValue => 'PARCEL', + }, +##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. +# flatFee => { +# fieldType => 'float', +# label => $i18n->get('flatFee'), +# hoverHelp => $i18n->get('flatFee help'), +# defaultValue => 0, +# }, + ); + my %properties = ( + name => $i18n2->get('U.S. Postal Service, International'), + properties => \%fields, + ); + push @{ $definition }, \%properties; + return $class->SUPER::definition($session, $definition); +} + +#------------------------------------------------------------------- + +=head2 _doXmlRequest ( $xml ) + +Contact the USPS website and submit the XML for a shipping rate lookup. +Returns a LWP::UserAgent response object. + +=head3 $xml + +XML to send. It has some very high standards, including XML components in +the right order and sets of allowed tags. + +=cut + +sub _doXmlRequest { + my ($self, $xml) = @_; + my $userAgent = LWP::UserAgent->new; + $userAgent->env_proxy; + $userAgent->agent('WebGUI'); + my $url = 'http://production.shippingapis.com/ShippingAPI.dll?API=IntlRate&XML='; + $url .= $xml; + my $request = HTTP::Request->new(GET => $url); + my $response = $userAgent->request($request); + return $response; +} + +#------------------------------------------------------------------- + +=head2 _getShippableUnits ( $cart ) + +This is a private method. + +Sorts items into the cart by how they must be shipped, together, separate, +etc. Returns an array of array references of cart items grouped by +whether or not they ship separately, and then sorted by destination +zip code. + +If an item in the cart must be shipped separately, but has a quantity greater +than 1, then for the purposes of looking up shipping costs it is returned +as 1 bundle, since the total cost can now be calculated by multiplying the +quantity together with the cost for a single unit. + +For an empty cart (which shouldn't ever happen), it would return an empty array. + +=head3 $cart + +A WebGUI::Shop::Cart object. It provides access to the items in the cart +that must be sorted. + +=cut + +sub _getShippableUnits { + my ($self, $cart) = @_; + my @shippableUnits = (); + ##Loose units are sorted by zip code. + my %looseUnits = (); + ITEM: foreach my $item (@{$cart->getItems}) { + my $sku = $item->getSku; + next ITEM unless $sku->isShippingRequired; + if ($sku->shipsSeparately) { + push @shippableUnits, [ $item ]; + } + else { + my $zip = $item->getShippingAddress->get('code'); + if ($item->getShippingAddress->get('country') ne 'United States') { + WebGUI::Error::InvalidParam->throw(error => q{Driver only handles domestic shipping}); + } + push @{ $looseUnits{$zip} }, $item; + } + } + push @shippableUnits, values %looseUnits; + return @shippableUnits; +} + +1; diff --git a/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm b/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm new file mode 100644 index 000000000..5de6964b1 --- /dev/null +++ b/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm @@ -0,0 +1,15 @@ +package WebGUI::i18n::English::ShipDriver_USPS; + +use strict; + +our $I18N = { + + 'U.S. Postal Service, International' => { + message => q|U.S. Postal Service, International|, + lastUpdated => 1203569535, + context => q|Name of the shipping driver|, + } + +}; + +1; From c1f7788309e6d5fca41855052cd5e4bd2707ea2f Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Thu, 5 Nov 2009 15:33:17 -0800 Subject: [PATCH 02/20] Copy tests from USPS. Add insurance back to the International driver. Reverse the country check. --- .../Shop/ShipDriver/USPSInternational.pm | 14 +- t/Shop/ShipDriver/USPSInternational.t | 861 ++++++++++++++++++ 2 files changed, 871 insertions(+), 4 deletions(-) create mode 100644 t/Shop/ShipDriver/USPSInternational.t diff --git a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm index a927e3d44..d583dedab 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm @@ -130,8 +130,8 @@ sub calculate { if (! $self->get('userId')) { WebGUI::Error::InvalidParam->throw(error => q{Driver configured without a USPS userId.}); } - if ($cart->getShippingAddress->get('country') ne 'United States') { - WebGUI::Error::InvalidParam->throw(error => q{Driver only handles domestic shipping}); + if ($cart->getShippingAddress->get('country') eq 'United States') { + WebGUI::Error::InvalidParam->throw(error => q{Driver only handles international shipping}); } my $cost = 0; ##Sort the items into shippable bundles. @@ -263,6 +263,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. @@ -345,8 +351,8 @@ sub _getShippableUnits { } else { my $zip = $item->getShippingAddress->get('code'); - if ($item->getShippingAddress->get('country') ne 'United States') { - WebGUI::Error::InvalidParam->throw(error => q{Driver only handles domestic shipping}); + if ($item->getShippingAddress->get('country') eq 'United States') { + WebGUI::Error::InvalidParam->throw(error => q{Driver only handles international shipping}); } push @{ $looseUnits{$zip} }, $item; } diff --git a/t/Shop/ShipDriver/USPSInternational.t b/t/Shop/ShipDriver/USPSInternational.t new file mode 100644 index 000000000..55593d256 --- /dev/null +++ b/t/Shop/ShipDriver/USPSInternational.t @@ -0,0 +1,861 @@ +# vim:syntax=perl +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2009 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------ +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------ + +# Write a little about what this script tests. +# +# + +use FindBin; +use strict; +use lib "$FindBin::Bin/../../lib"; +use Test::More; +use Test::Deep; +use XML::Simple; +use Data::Dumper; + +use WebGUI::Test; # Must use this before any other WebGUI modules +use WebGUI::Session; +use WebGUI::Shop::ShipDriver::USPSInternational; + +plan tests => 53; + +#---------------------------------------------------------------------------- +# Init +my $session = WebGUI::Test->session; +my $user = WebGUI::User->create($session); +WebGUI::Test->usersToDelete($user); +$session->user({user => $user}); + +#---------------------------------------------------------------------------- +# Tests + +#---------------------------------------------------------------------------- +# put your tests here + + +my ($driver2, $cart); + +my $versionTag = WebGUI::VersionTag->getWorking($session); + +my $home = WebGUI::Asset->getDefault($session); + +my $rockHammer = $home->addChild({ + className => 'WebGUI::Asset::Sku::Product', + isShippingRequired => 1, title => 'Rock Hammers', + shipsSeparately => 0, +}); + +my $smallHammer = $rockHammer->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'Small rock hammer', price => 7.50, + varSku => 'small-hammer', weight => 1.5, + quantity => 9999, + } +); + +my $bigHammer = $rockHammer->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'Big rock hammer', price => 19.99, + varSku => 'big-hammer', weight => 12, + quantity => 9999, + } +); + +my $bible = $home->addChild({ + className => 'WebGUI::Asset::Sku::Product', + isShippingRequired => 1, title => 'Bibles, individuall wrapped and shipped', + shipsSeparately => 1, +}); + +my $kjvBible = $bible->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'King James Bible', price => 17.50, + varSku => 'kjv-bible', weight => 2.5, + quantity => 99999, + } +); + +my $nivBible = $bible->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'NIV Bible', price => 22.50, + varSku => 'niv-bible', weight => 2.0, + quantity => 999999, + } +); + +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; +addToCleanup($versionTag); + +####################################################################### +# +# definition +# +####################################################################### + +my $definition; +my $e; ##Exception variable, used throughout the file + +eval { $definition = WebGUI::Shop::ShipDriver::USPSInternational->definition(); }; +$e = Exception::Class->caught(); +isa_ok($e, 'WebGUI::Error::InvalidParam', 'definition takes an exception to not giving it a session variable'); +cmp_deeply( + $e, + methods( + error => 'Must provide a session variable', + ), + '... checking error message', +); + + +isa_ok( + $definition = WebGUI::Shop::ShipDriver::USPS->definition($session), + 'ARRAY' +); + + +####################################################################### +# +# create +# +####################################################################### + +my $options = { + label => 'USPS Driver', + enabled => 1, + }; + +$driver2 = WebGUI::Shop::ShipDriver::USPS->create($session, $options); +addToCleanup($driver2); + +isa_ok($driver2, 'WebGUI::Shop::ShipDriver::USPS'); +isa_ok($driver2, 'WebGUI::Shop::ShipDriver'); + +####################################################################### +# +# getName +# +####################################################################### + +is (WebGUI::Shop::ShipDriver::USPS->getName($session), 'U.S. Postal Service', 'getName returns the human readable name of this 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 $driver2; + +####################################################################### +# +# calculate, and private methods. +# +####################################################################### + +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(); +isa_ok($e, 'WebGUI::Error::InvalidParam', 'calculate throws an exception when no zipcode has been set'); +cmp_deeply( + $e, + methods( + error => 'Driver configured without a source zipcode.', + ), + '... checking error message', +); + +my $properties = $driver->get(); +$properties->{sourceZip} = '97123'; +$driver->update($properties); + +eval { $driver->calculate() }; +$e = Exception::Class->caught(); +isa_ok($e, 'WebGUI::Error::InvalidParam', 'calculate throws an exception when no userId'); +cmp_deeply( + $e, + methods( + error => 'Driver configured without a USPS userId.', + ), + '... checking error message', +); + +$cart = WebGUI::Shop::Cart->newBySession($session); +addToCleanup($cart); +my $addressBook = $cart->getAddressBook; +my $workAddress = $addressBook->addAddress({ + label => 'work', + organization => 'Plain Black Corporation', + address1 => '1360 Regent St. #145', + city => 'Madison', state => 'WI', code => '53715', + country => 'United States', +}); +my $wucAddress = $addressBook->addAddress({ + label => 'wuc', + organization => 'Madison Concourse Hotel', + address1 => '1 W Dayton St', + city => 'Madison', state => 'WI', code => '53703', + country => 'United States', +}); +$cart->update({shippingAddressId => $workAddress->getId}); + +cmp_deeply( + [$driver->_getShippableUnits($cart)], + [(), ], + '_getShippableUnits: empty cart' +); + +$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); +cmp_deeply( + [$driver->_getShippableUnits($cart)], + [[ ignore() ], ], + '_getShippableUnits: one loose item in the cart' +); + +$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); +cmp_deeply( + [$driver->_getShippableUnits($cart)], + [[ ignore(), ignore() ], ], + '_getShippableUnits: two loose items in the cart' +); + +$bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $kjvBible)); +cmp_bag( + [$driver->_getShippableUnits($cart)], + [[ ignore(), ignore() ], [ ignore(), ], ], + '_getShippableUnits: two loose items, and 1 ships separately item in the cart' +); + +my $bibleItem = $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $nivBible)); +$bibleItem->setQuantity(5); +cmp_bag( + [$driver->_getShippableUnits($cart)], + [[ ignore(), ignore() ], [ ignore() ], [ ignore() ], ], + '_getShippableUnits: two loose items, and 2 ships separately item in the cart, regarless of quantity for the new item' +); + +my $rockHammer2 = $bible->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); +$rockHammer2->update({shippingAddressId => $wucAddress->getId}); +cmp_bag( + [$driver->_getShippableUnits($cart)], + [[ ignore(), ignore() ], [ ignore() ], [ ignore() ], [ ignore() ], ], + '_getShippableUnits: two loose items, and 2 ships separately item in the cart, and another loose item sorted by zipcode' +); + +$cart->empty; +$bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $nivBible)); +cmp_deeply( + [$driver->_getShippableUnits($cart)], + [ [ ignore() ], ], + '_getShippableUnits: only 1 ships separately item in the cart' +); +$cart->empty; + +my $userId = $session->config->get('testing/USPS_userId'); +my $hasRealUserId = 1; +##If there isn't a userId, set a fake one for XML testing. +if (! $userId) { + $hasRealUserId = 0; + $userId = "blahBlahBlah"; +} +$properties = $driver->get(); +$properties->{userId} = $userId; +$properties->{sourceZip} = '97123'; +$driver->update($properties); + +$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); +my @shippableUnits = $driver->_getShippableUnits($cart); + +$properties = $driver->get(); +$properties->{addInsurance} = 1; +$driver->update($properties); + +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/ 1, + ForceArray => ['Package'], +); +cmp_deeply( + $xmlData, + { + RateV3Request => { + USERID => $userId, + Package => [ + { + ID => 0, + ZipDestination => '53715', ZipOrigination => '97123', + Pounds => '1', Ounces => '8.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + ], + } + }, + 'buildXML: PARCEL service, 1 item in cart' +); + +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/],); + cmp_deeply( + $xmlData, + { + Package => [ + { + ID => 0, + ZipOrigination => ignore(), ZipDestination => ignore(), + Machinable => ignore(), Ounces => ignore(), + Pounds => ignore(), Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + ], + }, + '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' + ); + +} + +my $cost = $driver->_calculateFromXML( + { + RateV3Response => { + Package => [ + { + ID => 0, + Postage => { + Rate => 5.25, + }, + }, + ], + }, + }, + @shippableUnits +); + +is($cost, 5.25, '_calculateFromXML calculates shipping cost correctly for 1 item in the cart'); + +$bibleItem = $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $nivBible)); +@shippableUnits = $driver->_getShippableUnits($cart); + +is(calculateInsurance($driver), 7, '_calculateInsurance: two items in cart with quantity=1, calculates insurance'); + +$xml = $driver->buildXML($cart, @shippableUnits); +$xmlData = XMLin( $xml, + KeepRoot => 1, + ForceArray => ['Package'], +); + +cmp_deeply( + $xmlData, + { + RateV3Request => { + USERID => $userId, + Package => [ + { + ID => 0, + ZipDestination => '53715', ZipOrigination => '97123', + Pounds => '2', Ounces => '0.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + { + ID => 1, + ZipDestination => '53715', ZipOrigination => '97123', + Pounds => '1', Ounces => '8.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + ], + } + }, + 'Validate XML structure and content for 2 items in the cart' +); + +SKIP: { + + skip 'No userId for testing', 2 unless $hasRealUserId; + + my $response = $driver->_doXmlRequest($xml); + ok($response->is_success, '_doXmlRequest to USPS successful for 2 items in cart'); + my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],); + cmp_deeply( + $xmlData, + { + Package => [ + { + ID => 0, + ZipOrigination => ignore(), ZipDestination => ignore(), + Machinable => ignore(), Ounces => '0.0', + Pounds => 2, Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + { + ID => 1, + ZipOrigination => ignore(), ZipDestination => ignore(), + Machinable => ignore(), Ounces => '8.0', + Pounds => 1, Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + ], + }, + '... returned data from USPS in correct format for 2 items in cart. If this test fails, the driver may need to be updated' + ); + +} + +$cost = $driver->_calculateFromXML( + { + RateV3Response => { + Package => [ + { + ID => 0, + Postage => { + Rate => 7.00, + }, + }, + { + ID => 1, + Postage => { + Rate => 5.25, + }, + }, + ], + }, + }, + @shippableUnits +); + +is($cost, 12.25, '_calculateFromXML calculates shipping cost correctly for 2 items in the cart'); + +$bibleItem->setQuantity(2); +@shippableUnits = $driver->_getShippableUnits($cart); + +is(calculateInsurance($driver), 8, '_calculateInsurance: two items in cart with quantity=2, calculates insurance'); + +$cost = $driver->_calculateFromXML( + { + RateV3Response => { + Package => [ + { + ID => 0, + Postage => { + Rate => 7.00, + }, + }, + { + ID => 1, + Postage => { + Rate => 5.25, + }, + }, + ], + }, + }, + @shippableUnits +); +is($cost, 19.25, '_calculateFromXML calculates shipping cost correctly for 2 items in the cart, with quantity of 2'); + +$rockHammer2 = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); +$rockHammer2->update({shippingAddressId => $wucAddress->getId}); +@shippableUnits = $driver->_getShippableUnits($cart); +is(calculateInsurance($driver), 12, '_calculateInsurance: calculates insurance'); +$xml = $driver->buildXML($cart, @shippableUnits); + +$xmlData = XMLin( $xml, + KeepRoot => 1, + ForceArray => ['Package'], +); + +cmp_deeply( + $xmlData, + { + RateV3Request => { + USERID => $userId, + Package => [ + { + ID => 0, + ZipDestination => '53715', ZipOrigination => '97123', + Pounds => '2', Ounces => '0.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + { + ID => 1, + ZipDestination => '53715', ZipOrigination => '97123', + Pounds => '1', Ounces => '8.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + { + ID => 2, + ZipDestination => '53703', ZipOrigination => '97123', + Pounds => '12', Ounces => '0.0', + Size => 'REGULAR', Service => 'PARCEL', + Machinable => 'true', + }, + ], + } + }, + 'Validate XML structure and content for 3 items in the cart, 3 shippable items' +); + +SKIP: { + + skip 'No userId for testing', 2 unless $hasRealUserId; + + my $response = $driver->_doXmlRequest($xml); + ok($response->is_success, '_doXmlRequest to USPS successful for 3 items in cart'); + my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],); + cmp_deeply( + $xmlData, + { + Package => [ + { + ID => 0, + ZipOrigination => ignore(), ZipDestination => ignore(), + Machinable => ignore(), Ounces => '0.0', + Pounds => 2, Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + { + ID => 1, + ZipOrigination => ignore(), ZipDestination => ignore(), + Machinable => ignore(), Ounces => '8.0', + Pounds => 1, Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + { + ID => 2, + ZipOrigination => ignore(), ZipDestination => 53703, + Machinable => ignore(), Ounces => '0.0', + Pounds => 12, Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(20,20), ##A number around 20... + } + }, + ], + }, + '... returned data from USPS in correct format for 3 items in cart. If this test fails, the driver may need to be updated' + ); + +} + +####################################################################### +# +# Test Priority shipping setup +# +####################################################################### + +$cart->empty; +$properties = $driver->get(); +$properties->{shipType} = 'PRIORITY'; +$driver->update($properties); + +$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); +@shippableUnits = $driver->_getShippableUnits($cart); +$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.0', + Size => 'REGULAR', Service => 'PRIORITY', + Machinable => 'true', Container => 'FLAT RATE BOX', + }, + ], + } + }, + 'buildXML: PRIORITY service, 1 item in cart' +); +like($xml, qr/RateV3Request USERID.+?Package ID=.+?Service.+?ZipOrigination.+?ZipDestination.+?Pounds.+?Ounces.+?Container.+?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/],); + cmp_deeply( + $xmlData, + { + Package => [ + { + ID => 0, + ZipOrigination => ignore(), ZipDestination => ignore(), + Container => ignore(), Ounces => ignore(), ##Machinable missing, added Container + Pounds => ignore(), Size => ignore(), + Zone => ignore(), + Postage => { + CLASSID => ignore(), + MailService => ignore(), + Rate => num(10,10), ##A number around 10... + } + }, + ], + }, + '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' + ); + +} + +####################################################################### +# +# Test EXPRESS shipping setup +# +####################################################################### + +$properties = $driver->get(); +$properties->{shipType} = 'EXPRESS'; +$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.0', + Size => 'REGULAR', Service => 'EXPRESS', + Machinable => 'true', + }, + ], + } + }, + 'buildXML: EXPRESS service, 1 item in cart' +); +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/],); + 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(30,30), ##A number around 10... + } + }, + ], + }, + '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' + ); + +} + +####################################################################### +# +# Test PRIORITY VARIABLE shipping setup +# +####################################################################### + +$properties = $driver->get(); +$properties->{shipType} = 'PRIORITY VARIABLE'; +$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.0', + Size => 'REGULAR', Service => 'PRIORITY', + Machinable => 'true',# Container => 'VARIABLE', + }, + ], + } + }, + 'buildXML: PRIORITY, VARIABLE service, 1 item in cart' +); +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/],); + 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' + ); + +} + +####################################################################### +# +# 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); + +my $dutchAddress = $addressBook->addAddress({ + label => 'dutch', + address1 => 'Rotterdamseweg 183C', + city => 'Delft', code => '2629HD', + country => 'Netherlands', +}); + +$cart->update({shippingAddressId => $dutchAddress->getId}); +$cost = eval { $driver->calculate($cart); }; +$e = Exception::Class->caught(); +isa_ok($e, 'WebGUI::Error::InvalidParam', "calculate won't calculate for foreign countries"); + +$cart->update({shippingAddressId => $workAddress->getId}); + +# +#-2147219500 +#DomesticRatesV3;clsRateV3.ValidateWeight;RateEngineV3.ProcessRequest +#Please enter the package weight. +#1000440 + +####################################################################### +# +# _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'); + + From 6543ed502734a0652af22ed27519a11d64d78eaa Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 6 Nov 2009 19:43:12 -0800 Subject: [PATCH 03/20] All tests passing, able to talk to USPS and get rates for multiple package configurations. --- .../Shop/ShipDriver/USPSInternational.pm | 59 +- .../English/ShipDriver_USPSInternational.pm | 52 +- t/Shop/ShipDriver/USPSInternational.t | 547 +++++------------- 3 files changed, 228 insertions(+), 430 deletions(-) diff --git a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm index d583dedab..e614bddb7 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm @@ -48,15 +48,12 @@ result by the quantity, rather than doing several identical checks. sub buildXML { my ($self, $cart, @packages) = @_; tie my %xmlHash, 'Tie::IxHash'; - %xmlHash = ( RateV3Request => {}, ); - my $xmlTop = $xmlHash{RateV3Request}; + %xmlHash = ( IntlRateRequest => {}, ); + my $xmlTop = $xmlHash{IntlRateRequest}; $xmlTop->{USERID} = $self->get('userId'); $xmlTop->{Package} = []; ##Do a request for each package. my $packageIndex; - my $shipType = $self->get('shipType'); - my $service = $shipType eq 'PRIORITY VARIABLE' ? 'PRIORITY' - : $shipType; PACKAGE: for(my $packageIndex = 0; $packageIndex < scalar @packages; $packageIndex++) { my $package = $packages[$packageIndex]; next PACKAGE unless scalar @{ $package }; @@ -78,21 +75,13 @@ sub buildXML { $ounces = 0.1; } my $destination = $package->[0]->getShippingAddress; - my $destZipCode = $destination->get('code'); - $packageData{ID} = $packageIndex; - $packageData{Service} = [ $service ]; - $packageData{ZipOrigination} = [ $self->get('sourceZip') ]; - $packageData{ZipDestination} = [ $destZipCode ]; - $packageData{Pounds} = [ $pounds ]; - $packageData{Ounces} = [ $ounces ]; - if ($shipType eq 'PRIORITY') { - $packageData{Container} = [ 'FLAT RATE BOX' ]; - } - elsif ($shipType eq 'PRIORITY VARIABLE') { - #$packageData{Container} = [ 'VARIABLE' ]; - } - $packageData{Size} = [ 'REGULAR' ]; - $packageData{Machinable} = [ 'true' ]; + my $country = $destination->get('country'); + $packageData{ID} = $packageIndex; + $packageData{Pounds} = [ $pounds ]; + $packageData{Ounces} = [ $ounces ]; + $packageData{Machinable} = [ 'true' ]; + $packageData{MailType} = [ 'Package' ]; + $packageData{Country} = [ $country ]; push @{ $xmlTop->{Package} }, \%packageData; } my $xml = XMLout(\%xmlHash, @@ -124,9 +113,6 @@ costs are assessed. sub calculate { my ($self, $cart) = @_; - if (! $self->get('sourceZip')) { - WebGUI::Error::InvalidParam->throw(error => q{Driver configured without a source zipcode.}); - } if (! $self->get('userId')) { WebGUI::Error::InvalidParam->throw(error => q{Driver configured without a USPS userId.}); } @@ -173,7 +159,7 @@ Processed XML data from an XML rate request, processed in perl data structure. have this structure: { - RateV3Response => { + IntlRateResponse => { Package => [ { ID => 0, @@ -194,17 +180,24 @@ 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->{RateV3Response}->{Package} }) { + foreach my $package (@{ $xmlData->{IntlRateResponse}->{Package} }) { my $id = $package->{ID}; - my $rate = $package->{Postage}->{Rate}; ##Error check for invalid index if ($id < 0 || $id > $#shippableUnits) { WebGUI::Error::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id"); } if (exists $package->{Error}) { - WebGUI::Error::Shop::RemoteShippingRate->throw(error => $package->{Description}); + WebGUI::Error::Shop::RemoteShippingRate->throw(error => $package->{Error}->{Description}); } my $unit = $shippableUnits[$id]; + my $rate; + SERVICE: foreach my $service (@{ $package->{Service} }) { + next SERVICE unless $service->{ID} eq $self->get('shipType'); + $rate = $service->{Postage}; + } + if (!$rate) { + WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Selected shipping service not available'); + } if ($unit->[0]->getSku->shipsSeparately) { ##This is a single item due to ships separately. Since in reality there will be ## N things being shipped, multiply the rate by the quantity. @@ -238,10 +231,14 @@ sub definition { my $i18n2 = WebGUI::International->new($session, 'ShipDriver_USPSInternational'); tie my %shippingTypes, 'Tie::IxHash'; ##Note, these keys are used by buildXML - $shippingTypes{'PRIORITY VARIABLE'} = $i18n->get('priority variable'); - $shippingTypes{'PRIORITY'} = $i18n->get('priority'); - $shippingTypes{'EXPRESS' } = $i18n->get('express'); - $shippingTypes{'PARCEL' } = $i18n->get('parcel post'); + $shippingTypes{1} = $i18n2->get('express mail international'); + $shippingTypes{2} = $i18n2->get('priority mail international'); + $shippingTypes{6} = $i18n2->get('global express guaranteed rectangular'); + $shippingTypes{7} = $i18n2->get('global express guaranteed non-rectangular'); + $shippingTypes{9} = $i18n2->get('priority mail flat rate box'); + $shippingTypes{11} = $i18n2->get('priority mail large flat rate box'); + $shippingTypes{15} = $i18n2->get('first class mail international parcels'); + $shippingTypes{16} = $i18n2->get('priority mail small flat rate box'); tie my %fields, 'Tie::IxHash'; %fields = ( instructions => { diff --git a/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm b/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm index 5de6964b1..b628981f4 100644 --- a/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm +++ b/lib/WebGUI/i18n/English/ShipDriver_USPSInternational.pm @@ -1,4 +1,4 @@ -package WebGUI::i18n::English::ShipDriver_USPS; +package WebGUI::i18n::English::ShipDriver_USPSInternational; use strict; @@ -8,7 +8,55 @@ our $I18N = { message => q|U.S. Postal Service, International|, lastUpdated => 1203569535, context => q|Name of the shipping driver|, - } + }, + + 'express mail international' => { + message => q|Express Mail International|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'priority mail international' => { + message => q|Priority Mail International|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'global express guaranteed rectangular' => { + message => q|Global Express Guaranteed Non-Document Rectangular|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'global express guaranteed non-rectangular' => { + message => q|Global Express Guaranteed Non-Document Non-Rectangular|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'priority mail flat rate box' => { + message => q|Priority Mail Flat Rate Box|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'priority mail large flat rate box' => { + message => q|Priority Mail Large Flat Rate Box|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'priority mail small flat rate box' => { + message => q|Priority Mail Small Flat Rate Box|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, + + 'first class mail international parcels' => { + message => q|First Class Mail International Parcels|, + lastUpdated => 1203569535, + context => q|Name of a shipping option|, + }, }; diff --git a/t/Shop/ShipDriver/USPSInternational.t b/t/Shop/ShipDriver/USPSInternational.t index 55593d256..163c7aeee 100644 --- a/t/Shop/ShipDriver/USPSInternational.t +++ b/t/Shop/ShipDriver/USPSInternational.t @@ -25,7 +25,7 @@ use WebGUI::Test; # Must use this before any other WebGUI modules use WebGUI::Session; use WebGUI::Shop::ShipDriver::USPSInternational; -plan tests => 53; +plan tests => 34; #---------------------------------------------------------------------------- # Init @@ -124,7 +124,7 @@ cmp_deeply( isa_ok( - $definition = WebGUI::Shop::ShipDriver::USPS->definition($session), + $definition = WebGUI::Shop::ShipDriver::USPSInternational->definition($session), 'ARRAY' ); @@ -136,14 +136,14 @@ isa_ok( ####################################################################### my $options = { - label => 'USPS Driver', + label => 'Intl USPS Driver', enabled => 1, }; -$driver2 = WebGUI::Shop::ShipDriver::USPS->create($session, $options); +$driver2 = WebGUI::Shop::ShipDriver::USPSInternational->create($session, $options); addToCleanup($driver2); -isa_ok($driver2, 'WebGUI::Shop::ShipDriver::USPS'); +isa_ok($driver2, 'WebGUI::Shop::ShipDriver::USPSInternational'); isa_ok($driver2, 'WebGUI::Shop::ShipDriver'); ####################################################################### @@ -152,7 +152,7 @@ isa_ok($driver2, 'WebGUI::Shop::ShipDriver'); # ####################################################################### -is (WebGUI::Shop::ShipDriver::USPS->getName($session), 'U.S. Postal Service', 'getName returns the human readable name of this driver'); +is (WebGUI::Shop::ShipDriver::USPSInternational->getName($session), 'U.S. Postal Service, International', 'getName returns the human readable name of this driver'); ####################################################################### # @@ -174,28 +174,12 @@ undef $driver2; # ####################################################################### -my $driver = WebGUI::Shop::ShipDriver::USPS->create($session, { +my $driver = WebGUI::Shop::ShipDriver::USPSInternational->create($session, { label => 'Shipping from Shawshank', enabled => 1, - shipType => 'PARCEL', }); addToCleanup($driver); -eval { $driver->calculate() }; -$e = Exception::Class->caught(); -isa_ok($e, 'WebGUI::Error::InvalidParam', 'calculate throws an exception when no zipcode has been set'); -cmp_deeply( - $e, - methods( - error => 'Driver configured without a source zipcode.', - ), - '... checking error message', -); - -my $properties = $driver->get(); -$properties->{sourceZip} = '97123'; -$driver->update($properties); - eval { $driver->calculate() }; $e = Exception::Class->caught(); isa_ok($e, 'WebGUI::Error::InvalidParam', 'calculate throws an exception when no userId'); @@ -212,17 +196,15 @@ addToCleanup($cart); my $addressBook = $cart->getAddressBook; my $workAddress = $addressBook->addAddress({ label => 'work', - organization => 'Plain Black Corporation', - address1 => '1360 Regent St. #145', - city => 'Madison', state => 'WI', code => '53715', - country => 'United States', + organization => 'ProcoliX', + address1 => 'Rotterdamseweg 183C', + city => 'Delft', code => '2629HD', + country => 'Netherlands', }); -my $wucAddress = $addressBook->addAddress({ - label => 'wuc', - organization => 'Madison Concourse Hotel', - address1 => '1 W Dayton St', - city => 'Madison', state => 'WI', code => '53703', - country => 'United States', +my $sdhAddress = $addressBook->addAddress({ + label => 'other side of planet', + organization => 'SDH', + country => 'Australia', }); $cart->update({shippingAddressId => $workAddress->getId}); @@ -262,7 +244,7 @@ cmp_bag( ); my $rockHammer2 = $bible->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); -$rockHammer2->update({shippingAddressId => $wucAddress->getId}); +$rockHammer2->update({shippingAddressId => $sdhAddress->getId}); cmp_bag( [$driver->_getShippableUnits($cart)], [[ ignore(), ignore() ], [ ignore() ], [ ignore() ], [ ignore() ], ], @@ -285,31 +267,16 @@ if (! $userId) { $hasRealUserId = 0; $userId = "blahBlahBlah"; } -$properties = $driver->get(); -$properties->{userId} = $userId; -$properties->{sourceZip} = '97123'; +my $properties = $driver->get(); +$properties->{userId} = $userId; +$properties->{shipType} = '9'; $driver->update($properties); $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); my @shippableUnits = $driver->_getShippableUnits($cart); -$properties = $driver->get(); -$properties->{addInsurance} = 1; -$driver->update($properties); - -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/ { + IntlRateRequest => { USERID => $userId, Package => [ { ID => 0, - ZipDestination => '53715', ZipOrigination => '97123', - Pounds => '1', Ounces => '8.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '1', Ounces => '8.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', }, ], } }, - 'buildXML: PARCEL service, 1 item in cart' + 'buildXML: 1 item in cart' ); -like($xml, qr/RateV3Request USERID.+?Package ID=.+?Service.+?ZipOrigination.+?ZipDestination.+?Pounds.+?Ounces.+?Size.+?Machinable/, '... and tag order'); +like($xml, qr/IntlRateRequest USERID.+?Package ID=.+?Pounds.+?Ounces.+?Machinable.+?MailType.+?Country.+?/, '... and tag order'); SKIP: { @@ -350,15 +316,25 @@ SKIP: { Package => [ { ID => 0, - ZipOrigination => ignore(), ZipDestination => ignore(), - Machinable => ignore(), Ounces => ignore(), - Pounds => ignore(), Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } + AreasServed => ignore(), Prohibitions => ignore(), + ExpressMail => ignore(), CustomsForms => ignore(), + Observations => ignore(), Restrictions => ignore(), + Service => [ + { + ID => ignore(), + MaxWeight => ignore(), + MaxDimensions => ignore(), + MailType => 'Package', + Ounces => '8', + Pounds => '1', + Country => 'NETHERLANDS', + Machinable => 'true', + Postage => num(100,99), + SvcCommitments => ignore(), + SvcDescription => ignore(), + }, + (ignore())x12, + ], }, ], }, @@ -369,13 +345,22 @@ SKIP: { my $cost = $driver->_calculateFromXML( { - RateV3Response => { + IntlRateResponse => { Package => [ { ID => 0, - Postage => { - Rate => 5.25, - }, + Service => [ + { + ID => '9', + Postage => '5.25', + MaxWeight => '70' + }, + { + ID => '11', + Postage => '7.25', + MaxWeight => '70' + }, + ], }, ], }, @@ -388,8 +373,6 @@ 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), 7, '_calculateInsurance: two items in cart with quantity=1, calculates insurance'); - $xml = $driver->buildXML($cart, @shippableUnits); $xmlData = XMLin( $xml, KeepRoot => 1, @@ -399,22 +382,20 @@ $xmlData = XMLin( $xml, cmp_deeply( $xmlData, { - RateV3Request => { + IntlRateRequest => { USERID => $userId, Package => [ { ID => 0, - ZipDestination => '53715', ZipOrigination => '97123', - Pounds => '2', Ounces => '0.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '2', Ounces => '0.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', }, { ID => 1, - ZipDestination => '53715', ZipOrigination => '97123', - Pounds => '1', Ounces => '8.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '1', Ounces => '8.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', }, ], } @@ -428,57 +409,41 @@ SKIP: { my $response = $driver->_doXmlRequest($xml); ok($response->is_success, '_doXmlRequest to USPS successful for 2 items in cart'); - my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],); - cmp_deeply( - $xmlData, - { - Package => [ - { - ID => 0, - ZipOrigination => ignore(), ZipDestination => ignore(), - Machinable => ignore(), Ounces => '0.0', - Pounds => 2, Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } - }, - { - ID => 1, - ZipOrigination => ignore(), ZipDestination => ignore(), - Machinable => ignore(), Ounces => '8.0', - Pounds => 1, Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } - }, - ], - }, - '... returned data from USPS in correct format for 2 items in cart. If this test fails, the driver may need to be updated' - ); - } $cost = $driver->_calculateFromXML( { - RateV3Response => { + IntlRateResponse => { Package => [ { ID => 0, - Postage => { - Rate => 7.00, - }, + Service => [ + { + ID => '9', + Postage => '7.00', + MaxWeight => '70' + }, + { + ID => '11', + Postage => '9.00', + MaxWeight => '70' + }, + ], }, { ID => 1, - Postage => { - Rate => 5.25, - }, + Service => [ + { + ID => '9', + Postage => '5.25', + MaxWeight => '70' + }, + { + ID => '11', + Postage => '7.25', + MaxWeight => '70' + }, + ], }, ], }, @@ -491,23 +456,39 @@ is($cost, 12.25, '_calculateFromXML calculates shipping cost correctly for 2 ite $bibleItem->setQuantity(2); @shippableUnits = $driver->_getShippableUnits($cart); -is(calculateInsurance($driver), 8, '_calculateInsurance: two items in cart with quantity=2, calculates insurance'); - $cost = $driver->_calculateFromXML( { - RateV3Response => { + IntlRateResponse => { Package => [ { ID => 0, - Postage => { - Rate => 7.00, - }, + Service => [ + { + ID => '9', + Postage => '7.00', + MaxWeight => '70' + }, + { + ID => '11', + Postage => '9.00', + MaxWeight => '70' + }, + ], }, { ID => 1, - Postage => { - Rate => 5.25, - }, + Service => [ + { + ID => '9', + Postage => '5.25', + MaxWeight => '70' + }, + { + ID => '11', + Postage => '7.25', + MaxWeight => '70' + }, + ], }, ], }, @@ -517,9 +498,8 @@ $cost = $driver->_calculateFromXML( is($cost, 19.25, '_calculateFromXML calculates shipping cost correctly for 2 items in the cart, with quantity of 2'); $rockHammer2 = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); -$rockHammer2->update({shippingAddressId => $wucAddress->getId}); +$rockHammer2->update({shippingAddressId => $sdhAddress->getId}); @shippableUnits = $driver->_getShippableUnits($cart); -is(calculateInsurance($driver), 12, '_calculateInsurance: calculates insurance'); $xml = $driver->buildXML($cart, @shippableUnits); $xmlData = XMLin( $xml, @@ -530,29 +510,26 @@ $xmlData = XMLin( $xml, cmp_deeply( $xmlData, { - RateV3Request => { - USERID => $userId, + IntlRateRequest => { + USERID => $userId, Package => [ { ID => 0, - ZipDestination => '53715', ZipOrigination => '97123', - Pounds => '2', Ounces => '0.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '2', Ounces => '0.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', }, { ID => 1, - ZipDestination => '53715', ZipOrigination => '97123', - Pounds => '1', Ounces => '8.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '12', Ounces => '0.0', + Machinable => 'true', Country => 'Australia', + MailType => 'Package', }, { ID => 2, - ZipDestination => '53703', ZipOrigination => '97123', - Pounds => '12', Ounces => '0.0', - Size => 'REGULAR', Service => 'PARCEL', - Machinable => 'true', + Pounds => '1', Ounces => '8.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', }, ], } @@ -561,252 +538,44 @@ cmp_deeply( ); SKIP: { - skip 'No userId for testing', 2 unless $hasRealUserId; my $response = $driver->_doXmlRequest($xml); ok($response->is_success, '_doXmlRequest to USPS successful for 3 items in cart'); - my $xmlData = XMLin($response->content, ForceArray => [qw/Package/],); - cmp_deeply( - $xmlData, - { - Package => [ - { - ID => 0, - ZipOrigination => ignore(), ZipDestination => ignore(), - Machinable => ignore(), Ounces => '0.0', - Pounds => 2, Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } - }, - { - ID => 1, - ZipOrigination => ignore(), ZipDestination => ignore(), - Machinable => ignore(), Ounces => '8.0', - Pounds => 1, Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } - }, - { - ID => 2, - ZipOrigination => ignore(), ZipDestination => 53703, - Machinable => ignore(), Ounces => '0.0', - Pounds => 12, Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(20,20), ##A number around 20... - } - }, - ], - }, - '... returned data from USPS in correct format for 3 items in cart. If this test fails, the driver may need to be updated' - ); - } ####################################################################### # -# Test Priority shipping setup +# Check too heavy for my shipping type # ####################################################################### $cart->empty; $properties = $driver->get(); -$properties->{shipType} = 'PRIORITY'; +$properties->{shipType} = '9'; $driver->update($properties); -$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $smallHammer)); -@shippableUnits = $driver->_getShippableUnits($cart); -$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.0', - Size => 'REGULAR', Service => 'PRIORITY', - Machinable => 'true', Container => 'FLAT RATE BOX', - }, - ], - } - }, - 'buildXML: PRIORITY service, 1 item in cart' -); -like($xml, qr/RateV3Request USERID.+?Package ID=.+?Service.+?ZipOrigination.+?ZipDestination.+?Pounds.+?Ounces.+?Container.+?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/],); + my $heavyHammer = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); + $heavyHammer->setQuantity(2); + $cost = eval { $driver->calculate($cart); }; + $e = Exception::Class->caught(); + isa_ok($e, 'WebGUI::Error::Shop::RemoteShippingRate', "USPS returns error when package is too heavy for the selected service"); cmp_deeply( - $xmlData, - { - Package => [ - { - ID => 0, - ZipOrigination => ignore(), ZipDestination => ignore(), - Container => ignore(), Ounces => ignore(), ##Machinable missing, added Container - Pounds => ignore(), Size => ignore(), - Zone => ignore(), - Postage => { - CLASSID => ignore(), - MailService => ignore(), - Rate => num(10,10), ##A number around 10... - } - }, - ], - }, - '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' + $e, + methods( + error => 'Selected shipping service not available', + ), + '... checking error message', ); -} - -####################################################################### -# -# Test EXPRESS shipping setup -# -####################################################################### - -$properties = $driver->get(); -$properties->{shipType} = 'EXPRESS'; -$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.0', - Size => 'REGULAR', Service => 'EXPRESS', - Machinable => 'true', - }, - ], - } - }, - 'buildXML: EXPRESS service, 1 item in cart' -); -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/],); - 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(30,30), ##A number around 10... - } - }, - ], - }, - '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' - ); - -} - -####################################################################### -# -# Test PRIORITY VARIABLE shipping setup -# -####################################################################### - -$properties = $driver->get(); -$properties->{shipType} = 'PRIORITY VARIABLE'; -$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.0', - Size => 'REGULAR', Service => 'PRIORITY', - Machinable => 'true',# Container => 'VARIABLE', - }, - ], - } - }, - 'buildXML: PRIORITY, VARIABLE service, 1 item in cart' -); -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/],); - 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' - ); + $heavyHammer->setQuantity(20); + $cost = eval { $driver->calculate($cart); }; + $e = Exception::Class->caught(); + isa_ok($e, 'WebGUI::Error::Shop::RemoteShippingRate', "USPS returns error when package is too heavy for any service"); } @@ -818,7 +587,7 @@ SKIP: { my $userId = $driver->get('userId'); $properties = $driver->get(); -$properties->{userId} = '__NEVER_GOING_TO_HAPPEN__'; +$properties->{userId} = '_NO_NO_NO_NO'; $driver->update($properties); $cost = eval { $driver->calculate($cart); }; @@ -829,33 +598,17 @@ $properties->{userId} = $userId; $driver->update($properties); my $dutchAddress = $addressBook->addAddress({ - label => 'dutch', - address1 => 'Rotterdamseweg 183C', - city => 'Delft', code => '2629HD', - country => 'Netherlands', + label => 'american', + organization => 'Plain Black Corporation', + address1 => '1360 Regent St. #145', + city => 'Madison', state => 'WI', code => '53715', + country => 'United States', }); $cart->update({shippingAddressId => $dutchAddress->getId}); $cost = eval { $driver->calculate($cart); }; $e = Exception::Class->caught(); -isa_ok($e, 'WebGUI::Error::InvalidParam', "calculate won't calculate for foreign countries"); +isa_ok($e, 'WebGUI::Error::InvalidParam', "calculate won't calculate for domestic countries"); $cart->update({shippingAddressId => $workAddress->getId}); -# -#-2147219500 -#DomesticRatesV3;clsRateV3.ValidateWeight;RateEngineV3.ProcessRequest -#Please enter the package weight. -#1000440 - -####################################################################### -# -# _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'); - - From acda103fed972d4415b7a6995e1fdd6f246ca175 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 6 Nov 2009 19:49:20 -0800 Subject: [PATCH 04/20] Add USPS International driver to configuration files. --- docs/upgrades/upgrade_7.8.4-7.8.5.pl | 11 +++++++++++ etc/WebGUI.conf.original | 1 + 2 files changed, 12 insertions(+) diff --git a/docs/upgrades/upgrade_7.8.4-7.8.5.pl b/docs/upgrades/upgrade_7.8.4-7.8.5.pl index 1a027df9e..0931252e2 100644 --- a/docs/upgrades/upgrade_7.8.4-7.8.5.pl +++ b/docs/upgrades/upgrade_7.8.4-7.8.5.pl @@ -30,6 +30,7 @@ my $quiet; # this line required my $session = start(); # this line required fixPackageFlagOnOlder( $session ); +addUSPSInternationalShippingDriver( $session ); # upgrade functions go here @@ -45,6 +46,16 @@ finish($session); # this line required # print "DONE!\n" unless $quiet; #} +#---------------------------------------------------------------------------- +# Describe what our function does +sub addUSPSInternationalShippingDriver { + my $session = shift; + print "\tAdd the USPS International shipping driver... " unless $quiet; + # and here's our code + $session->config->addToArray('shippingDrivers', 'WebGUI::Shop::ShipDriver::USPSInternational'); + print "DONE!\n" unless $quiet; +} + sub fixPackageFlagOnOlder { my $session = shift; print "\tFixing isPackage flag on folders and isDefault on templates from 7.6.35 to 7.7.17 upgrade. If default templates have been deleted from your site, you may see warnings about not being able to find assets. You may safely ignore those warnings. This entire process may take a while.. " unless $quiet; diff --git a/etc/WebGUI.conf.original b/etc/WebGUI.conf.original index 229ea13bb..46fbddd80 100644 --- a/etc/WebGUI.conf.original +++ b/etc/WebGUI.conf.original @@ -191,6 +191,7 @@ "shippingDrivers" : [ "WebGUI::Shop::ShipDriver::FlatRate", "WebGUI::Shop::ShipDriver::USPS", + "WebGUI::Shop::ShipDriver::USPSInternational", "WebGUI::Shop::ShipDriver::UPS" ], From 0b483521e859b04aab7ad9b3720fbbefc6c67e5a Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 6 Nov 2009 20:15:09 -0800 Subject: [PATCH 05/20] Update tests for new driver. --- t/Shop/Ship.t | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/Shop/Ship.t b/t/Shop/Ship.t index a75073f33..0efead294 100644 --- a/t/Shop/Ship.t +++ b/t/Shop/Ship.t @@ -93,9 +93,10 @@ cmp_bag( [ 'WebGUI::Shop::ShipDriver::FlatRate', 'WebGUI::Shop::ShipDriver::USPS', + 'WebGUI::Shop::ShipDriver::USPSInternational', 'WebGUI::Shop::ShipDriver::UPS', ], - 'getDrivers: WebGUI ships with 3 default shipping drivers', + 'getDrivers: All default shipping drivers present', ); ####################################################################### From 9206a0b80fbb5b7396a29a2644e04f3c9e6a91d0 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 6 Nov 2009 21:21:20 -0800 Subject: [PATCH 06/20] Add insurance option. --- .../Shop/ShipDriver/USPSInternational.pm | 15 +++++ t/Shop/ShipDriver/USPSInternational.t | 59 ++++++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm index e614bddb7..1a37d8b05 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm @@ -59,21 +59,26 @@ sub buildXML { next PACKAGE unless scalar @{ $package }; tie my %packageData, 'Tie::IxHash'; my $weight = 0; + my $value = 0; foreach my $item (@{ $package }) { my $sku = $item->getSku; my $itemWeight = $sku->getWeight(); + my $itemValue = $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'); + $itemValue *= $item->get('quantity'); } $weight += $itemWeight; + $value += $itemValue; } my $pounds = int($weight); my $ounces = sprintf '%3.1f', (16 * ($weight - $pounds)); if ($pounds == 0 && $ounces eq '0.0' ) { $ounces = 0.1; } + $value = sprintf '%.2f', $value; my $destination = $package->[0]->getShippingAddress; my $country = $destination->get('country'); $packageData{ID} = $packageIndex; @@ -81,6 +86,9 @@ sub buildXML { $packageData{Ounces} = [ $ounces ]; $packageData{Machinable} = [ 'true' ]; $packageData{MailType} = [ 'Package' ]; + if ($self->get('addInsurance')) { + $packageData{ValueOfContents} = [ $value ]; + } $packageData{Country} = [ $country ]; push @{ $xmlTop->{Package} }, \%packageData; } @@ -138,6 +146,7 @@ sub calculate { WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Problem connecting to USPS Web Tools: '. $response->status_line); } my $returnedXML = $response->content; + #warn $returnedXML; my $xmlData = XMLin($returnedXML, KeepRoot => 1, ForceArray => [qw/Package/]); if (exists $xmlData->{Error}) { WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Problem with USPS Web Tools XML: '. $xmlData->{Error}->{Description}); @@ -194,6 +203,12 @@ sub _calculateFromXML { SERVICE: foreach my $service (@{ $package->{Service} }) { next SERVICE unless $service->{ID} eq $self->get('shipType'); $rate = $service->{Postage}; + if ($self->get('addInsurance')) { + if (exists $service->{InsComment}) { + WebGUI::Error::Shop::RemoteShippingRate->throw(error => "No insurance because of: ".$service->{InsComment}); + } + $rate += $service->{Insurance}; + } } if (!$rate) { WebGUI::Error::Shop::RemoteShippingRate->throw(error => 'Selected shipping service not available'); diff --git a/t/Shop/ShipDriver/USPSInternational.t b/t/Shop/ShipDriver/USPSInternational.t index 163c7aeee..8d24e5f17 100644 --- a/t/Shop/ShipDriver/USPSInternational.t +++ b/t/Shop/ShipDriver/USPSInternational.t @@ -25,7 +25,7 @@ use WebGUI::Test; # Must use this before any other WebGUI modules use WebGUI::Session; use WebGUI::Shop::ShipDriver::USPSInternational; -plan tests => 34; +plan tests => 37; #---------------------------------------------------------------------------- # Init @@ -340,7 +340,6 @@ SKIP: { }, '... returned data from USPS in correct format. If this test fails, the driver may need to be updated' ); - } my $cost = $driver->_calculateFromXML( @@ -579,6 +578,62 @@ SKIP: { } +####################################################################### +# +# Insurance +# +####################################################################### + +SKIP: { + + skip 'No userId for testing', 3 unless $hasRealUserId; + + + $cart->empty; + $properties = $driver->get(); + $properties->{shipType} = '9'; + $driver->update($properties); + $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); + + my $noInsuranceCost = $driver->calculate($cart); + + $properties->{addInsurance} = 1; + $driver->update($properties); + + @shippableUnits = $driver->_getShippableUnits($cart); + my $xml = $driver->buildXML($cart, @shippableUnits); + my $xmlData = XMLin($xml, + KeepRoot => 1, + ForceArray => ['Package'], + ); + + cmp_deeply( + $xmlData, + { + IntlRateRequest => { + USERID => $userId, + Package => [ + { + ID => 0, + Pounds => '12', Ounces => '0.0', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', ValueOfContents => '19.99', + }, + ], + } + }, + 'buildXML: 1 item in cart' + ); + like($xml, qr/IntlRateRequest USERID.+?Package ID=.+?Pounds.+?Ounces.+?Machinable.+?MailType.+?ValueOfContents.+?Country.+?/, '... and tag order'); + + my $insuredCost = $driver->calculate($cart); + cmp_ok $noInsuranceCost, '<', $insuredCost, 'insured cost is higher than uninsured cost'; + + $properties->{addInsurance} = 0; + $driver->update($properties); + +} + ####################################################################### # # Check for throwing an exception From bfc05ddc7a0d401eb1652ed78ed977684bb1f755 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 6 Nov 2009 21:23:22 -0800 Subject: [PATCH 07/20] Fix a bug in error reporting based on returned XML data. --- lib/WebGUI/Shop/ShipDriver/USPS.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm index 8ddc2bc01..8c66d8e35 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPS.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm @@ -203,7 +203,7 @@ sub _calculateFromXML { WebGUI::Error::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id"); } if (exists $package->{Error}) { - WebGUI::Error::Shop::RemoteShippingRate->throw(error => $package->{Description}); + WebGUI::Error::Shop::RemoteShippingRate->throw(error => $package->{Error}->{Description}); } my $unit = $shippableUnits[$id]; if ($unit->[0]->getSku->shipsSeparately) { From e647c013b9981bfbc84a238b014bc386a2f78d73 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Sat, 7 Nov 2009 12:56:43 -0800 Subject: [PATCH 08/20] Add a few more tests to hit coverage. Check for non-numeric package IDs. --- .../Shop/ShipDriver/USPSInternational.pm | 2 +- t/Shop/ShipDriver/USPSInternational.t | 96 ++++++++++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm index 1a37d8b05..4d52983c3 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPSInternational.pm @@ -192,7 +192,7 @@ sub _calculateFromXML { foreach my $package (@{ $xmlData->{IntlRateResponse}->{Package} }) { my $id = $package->{ID}; ##Error check for invalid index - if ($id < 0 || $id > $#shippableUnits) { + if ($id < 0 || $id > $#shippableUnits || $id !~ /^\d+$/) { WebGUI::Error::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id"); } if (exists $package->{Error}) { diff --git a/t/Shop/ShipDriver/USPSInternational.t b/t/Shop/ShipDriver/USPSInternational.t index 8d24e5f17..2b9d2b2a8 100644 --- a/t/Shop/ShipDriver/USPSInternational.t +++ b/t/Shop/ShipDriver/USPSInternational.t @@ -25,7 +25,7 @@ use WebGUI::Test; # Must use this before any other WebGUI modules use WebGUI::Session; use WebGUI::Shop::ShipDriver::USPSInternational; -plan tests => 37; +plan tests => 40; #---------------------------------------------------------------------------- # Init @@ -99,6 +99,14 @@ my $gospels = $bible->setCollateral('variantsJSON', 'variantId', 'new', } ); +my $singlePage = $bible->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => 'Single page from bible', + price => 0.01, varSku => 'page', + weight => 0.0001, quantity => 999999, + } +); + $versionTag->commit; addToCleanup($versionTag); @@ -545,19 +553,55 @@ SKIP: { ####################################################################### # -# Check too heavy for my shipping type +# Check for minimum weight allowed # ####################################################################### $cart->empty; $properties = $driver->get(); -$properties->{shipType} = '9'; +$properties->{shipType} = '9'; +$properties->{addInsurance} = 0; $driver->update($properties); +my $page1 = $bible->addToCart($bible->getCollateral('variantsJSON', 'variantId', $singlePage)); +@shippableUnits = $driver->_getShippableUnits($cart); +$xml = $driver->buildXML($cart, @shippableUnits); +$xmlData = XMLin($xml, + KeepRoot => 1, + ForceArray => ['Package'], +); +cmp_deeply( + $xmlData, + { + IntlRateRequest => { + USERID => $userId, + Package => [ + { + ID => 0, + Pounds => '0', Ounces => '0.1', + Machinable => 'true', Country => 'Netherlands', + MailType => 'Package', + }, + ], + } + }, + 'buildXML: minimum weight' +); + +####################################################################### +# +# Check too heavy for my shipping type +# +####################################################################### SKIP: { skip 'No userId for testing', 2 unless $hasRealUserId; + $cart->empty; + $properties = $driver->get(); + $properties->{shipType} = '9'; + $driver->update($properties); + my $heavyHammer = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); $heavyHammer->setQuantity(2); $cost = eval { $driver->calculate($cart); }; @@ -634,13 +678,57 @@ SKIP: { } +####################################################################### +# +# _calculateFromXML +# +####################################################################### + +$cart->empty; +$properties = $driver->get(); +$properties->{shipType} = '9'; +$properties->{addInsurance} = 1; +$driver->update($properties); +$rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $bigHammer)); +@shippableUnits = $driver->_getShippableUnits($cart); + +$cost = eval { $driver->_calculateFromXML( + { + IntlRateResponse => { + Package => [ + { + ID => 11, + Service => [ + { + ID => '9', + Postage => '5.25', + MaxWeight => '70' + }, + ], + }, + ], + }, + }, + @shippableUnits +); }; + +$e = Exception::Class->caught(); +isa_ok($e, 'WebGUI::Error::Shop::RemoteShippingRate', '_calculateFromXML throws an exception for illegal package ids'); +cmp_deeply( + $e, + methods( + error => 'Illegal package index returned by USPS: 11', + ), + '... checking error message', +); + ####################################################################### # # Check for throwing an exception # ####################################################################### -my $userId = $driver->get('userId'); +$userId = $driver->get('userId'); $properties = $driver->get(); $properties->{userId} = '_NO_NO_NO_NO'; $driver->update($properties); From 0a3836bde391e05e1e265da4652630569f481082 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Sat, 7 Nov 2009 12:58:49 -0800 Subject: [PATCH 09/20] Add non-numeric package check to USPS driver. --- lib/WebGUI/Shop/ShipDriver/USPS.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WebGUI/Shop/ShipDriver/USPS.pm b/lib/WebGUI/Shop/ShipDriver/USPS.pm index 8c66d8e35..d4c32b91f 100644 --- a/lib/WebGUI/Shop/ShipDriver/USPS.pm +++ b/lib/WebGUI/Shop/ShipDriver/USPS.pm @@ -199,7 +199,7 @@ sub _calculateFromXML { my $id = $package->{ID}; my $rate = $package->{Postage}->{Rate}; ##Error check for invalid index - if ($id < 0 || $id > $#shippableUnits) { + if ($id < 0 || $id > $#shippableUnits || $id !~ /^\d+$/) { WebGUI::Error::Shop::RemoteShippingRate->throw(error => "Illegal package index returned by USPS: $id"); } if (exists $package->{Error}) { From 356116592001f125bf6eb793953f1148cb2eaa5c Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Thu, 12 Nov 2009 11:40:23 -0600 Subject: [PATCH 10/20] added a bunch of nice people --- docs/credits.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/credits.txt b/docs/credits.txt index 266cf0530..b70847404 100644 --- a/docs/credits.txt +++ b/docs/credits.txt @@ -14,6 +14,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Leendert Bottelberghs / United Knowledge Richard Caelius / 100 World Irving Carrion + N. Hao Ching / Plain Black Richard Clark Doug Collinge Misja Op de Coul / E-Wise @@ -25,6 +26,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Frank Dillon / Plain Black Arne Dokken Patrick Donelan / SDH Consulting + Paul Driver / Plain Black Junying Du / Brunswick Ed Van Duinen / UNC Greg Fast / Brunswick @@ -32,6 +34,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Andy Grundman Chris Jackson Roy Johnson / Plain Black + Bart Jol / ProcoliX Koen de Jonge / ProcoliX Martin Kamerbeek / Oqapi Yung Han Khoe @@ -47,6 +50,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Kaleb Murphy / Plain Black Chris Nehren / Plain Black Ernesto Hernández-Novich / itverx C.A. + Stephen Opal / Plain Black Tavis Parker / Plain Black Daniel Quinlan Jukka Raimovaara / Axxion Oy @@ -56,11 +60,13 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Tera Runde / Plain Black Steve Simms Ben Simpson + Andy Smith / SDH Consulting Alan Smithee Steve Swanson / Plain Black Jeff Szpak / Plain Black Sean Tu / WDI Vladimir Vitkovsky / WebGUI Worldwide + Rogier Voogt / United Knowledge Jamie Vrbsky / Plain Black Arjan Widlak / United Knowledge Madsen Wikholm From 9b4ac74731f86534693cd0e13927302116f082a6 Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Thu, 12 Nov 2009 12:02:32 -0600 Subject: [PATCH 11/20] added some more wonderful people --- docs/credits.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/credits.txt b/docs/credits.txt index b70847404..768ee8ae0 100644 --- a/docs/credits.txt +++ b/docs/credits.txt @@ -21,6 +21,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Flavio Curti John Dagitz / Plain Black Joeri de Bruin / Oqapi + David Delikat Michele Dell'Aquila / CSU Jeff Depons / Adaptive Dynamics Frank Dillon / Plain Black From c9d1cba82053a580da9fe8b22fd9146fcf15794f Mon Sep 17 00:00:00 2001 From: Doug Bell Date: Thu, 12 Nov 2009 14:05:04 -0600 Subject: [PATCH 12/20] even more awesome people --- docs/credits.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/credits.txt b/docs/credits.txt index 768ee8ae0..1f248664a 100644 --- a/docs/credits.txt +++ b/docs/credits.txt @@ -33,6 +33,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Greg Fast / Brunswick Chris Gebhardt / OpenServe Andy Grundman + Tessa Harmon / Knowmad Technologies Chris Jackson Roy Johnson / Plain Black Bart Jol / ProcoliX @@ -61,10 +62,11 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Tera Runde / Plain Black Steve Simms Ben Simpson - Andy Smith / SDH Consulting + Andrew Smith / SDH Consulting Alan Smithee Steve Swanson / Plain Black Jeff Szpak / Plain Black + Henry Tang / Long Term Results B.V. Sean Tu / WDI Vladimir Vitkovsky / WebGUI Worldwide Rogier Voogt / United Knowledge @@ -76,7 +78,7 @@ Contributing Developers..............Meg O'Keefe Andrea / Plain Black Zhou Xiaopeng / WebGUI Worldwide Gerald Young Tabitha Zipperer / Plain Black - Henry Tang / Long Term Results B.V. + Rory Zweistra / Oqapi The following are people/companies who didn't directly contribute to WebGUI, but whose work has made WebGUI possible: From 97ce5090594b3796fc932a6a5075690999fcf8d4 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Thu, 12 Nov 2009 13:56:50 -0800 Subject: [PATCH 13/20] Adding new Style templates and CSS from TWG. The new css/wg-css replaces /webgui.css. It is removed in this upgrade. --- docs/changelog/7.x.x.txt | 1 + docs/gotcha.txt | 19 ++++++++++++++++++ docs/upgrades/packages-7.8.5/css.wgpkg | Bin 0 -> 1476 bytes .../packages-7.8.5/root_import_style.wgpkg | Bin 0 -> 4007 bytes docs/upgrades/packages-7.8.5/style_01.wgpkg | Bin 0 -> 1753 bytes docs/upgrades/packages-7.8.5/style_02.wgpkg | Bin 0 -> 2030 bytes docs/upgrades/packages-7.8.5/style_03.wgpkg | Bin 0 -> 1768 bytes docs/upgrades/upgrade_7.8.4-7.8.5.pl | 14 +++++++++++++ 8 files changed, 34 insertions(+) create mode 100644 docs/upgrades/packages-7.8.5/css.wgpkg create mode 100644 docs/upgrades/packages-7.8.5/root_import_style.wgpkg create mode 100644 docs/upgrades/packages-7.8.5/style_01.wgpkg create mode 100644 docs/upgrades/packages-7.8.5/style_02.wgpkg create mode 100644 docs/upgrades/packages-7.8.5/style_03.wgpkg diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 9668abf09..7ee41a343 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -11,6 +11,7 @@ - fixed #11215: Los Angeles cannot be default timezone - fixed #11220: Map asset badly broken - fixed #11222: testEnvironment.pl Missing Dependencies + - fixed #11226: New stylesheet (wg-base.css), new style templates (from the TWG) 7.8.4 - Fixed a compatibility problem between WRE and new Spectre code. diff --git a/docs/gotcha.txt b/docs/gotcha.txt index c294a19a0..24271457b 100644 --- a/docs/gotcha.txt +++ b/docs/gotcha.txt @@ -15,6 +15,25 @@ save you many hours of grief. the Visitor account. Previously, based on how the user was created, they would get default values from different places. +* The following style templates have been cleaned up by the TWG: + - WebGUI 6 Blank Style, Style 01; Style 02, Style 03: + - no structural changes + - Fail safe: + - added new CSS that is more robust and validates (in external file: style.css); + - it was also necessary to update the css to work with the new navigation templates + - changed the markup and the order of the home/login/user/admin controls at the bottom + - All of the above templates: + - added a link tag to wg-base.css + - added conditional comments at the top and bottom of the body tag to be able to target + IE versions easily with css + +* Added wg-base.css, which is linked to in each style template. This stylesheet is for css that + is used in more than one tempalte, like pagination inline icons etc. Inline styles that are + removed from templates, will be replaced with styles in wg-base.css (and example is RFE 11182). + Elements that are styled in wg-base.css have a classname that starts with "wg-". + + wg-base.css replaces webgui.css, which will be removed from the site. + 7.8.4 -------------------------------------------------------------------- * A bug introduced in 7.8.1 could cause the Shop sale notification diff --git a/docs/upgrades/packages-7.8.5/css.wgpkg b/docs/upgrades/packages-7.8.5/css.wgpkg new file mode 100644 index 0000000000000000000000000000000000000000..e2bbbbc0f5f8f011b89d27e4152d623cda79d653 GIT binary patch literal 1476 zcmV;#1v~m5iwFP!00000|Ls^=QyV!D_H%xPYMyqtf|;>BzVhU-Avi1q);QjSElQpd zw#ZnTmE;Q-|9xAUOE!dS19|a0P{XLz$JgD`*DjY{{&8Km;rpeM>v|2}E0wB^$&!y!G*7Ro8E9%oD&q(*&!~!wxZ^oSUav)1st=2p3q`_?*~X?(lrIN@U|4y-)C!&1 zzT3y0Sv~%wzR?h{`bui$zA}1Ij`^H>%uXfh?532Ly*_ zcqC77@D+A}wKYHFlJQv5+YYX6Hk!}HIV+U&IU}MkiRc=oPPJH?ipXpTq%1ySDTzvD zPcCXxK5BZiUjMLmxR?$uY9ITZLYB%C!4+4_I0@Q{iOK^ShJ+ad7Tgkm{wzWw%tEKP zS7;jvK8?HKhmfjdSEX2+QAr`)dpH=AGddxBn#fe^u4PoJWkk-%1d^>t7Q=f5|C_TS z|5La2P&Ouls=kQ!1)rQ?L0F>Nk_m|n1dSEtCZ4s#1W-Wo7*7%e24wiL-)*-$8pU>d zz=u~PfR}w9>6r7fiYOy#?K=5623HNd!)aE?nS$CIV>Zfb&`O}L-6I~-OPW-S<5m2o z3z$?b6)CJ=C`k&hQD|3rL#&m+gb!&%dd~n;ex>m-SKv^9GqfBFNK;%rdYyn(tyxPt z(y1wL2glIIGrM&rgiVX?U2~So-?;9Wq(j$IV}oZZs?}<=T7IKqnH5Br7H7X^jhjH| zH=9=VL7hWaX8E$Jc`8ZLuUYpRGnRVwa4|NlToD$CWvoszQ?@2NN-Mf|cs;|%WHujs z92{O8$u7ZlZ(d|ow^%2N$9PB-rJP6IP zc?(vrS@$gTV$*DSpPJ&xd(^yGIfw;iN;~$*y7a)3z7uZV+`QO5v{n3L5V(C}CSSfM z3j~e#@xPj1t#0tYYOM_B?^#B-6>$IEwPZexe|Z(xx?rC`!0vSIDMZYW zEMNn^MP~12VimX5vV_)|m_=zGWMVQc1wF>?{<4d@45=}d$jl5ds1b#he2N0h&?QW1 zut?H1c*P0?%;@ySQ|}X z#*31r8@;YI1QEd^QDI)&L$1bVw-M1HQl|S5!GNsLPX!b|uQ`&)7*{4(09rqC7QfybZU`<@I(91(ffV(@8O~7snknWkP zg3TWktRqF`6W1SKDB-`dzButnhiBJdw#l<^t>8~^~;{lozP literal 0 HcmV?d00001 diff --git a/docs/upgrades/packages-7.8.5/root_import_style.wgpkg b/docs/upgrades/packages-7.8.5/root_import_style.wgpkg new file mode 100644 index 0000000000000000000000000000000000000000..7a324f859ca7369207259eec44cd0ad1c7d568e4 GIT binary patch literal 4007 zcmV;Y4_NRYiwFP!00000|Lt6RciOlT&)@qgy!7ln={*H(%p(wP&P{0AX15R8(8qSO z$uU@fT4Qr<6GC(N?sqhjk!;K>P1480IZeTqMxz-?^PAB~xt+WJ=(=7hm3DS?-Kdm| zogK5nKicf0?-)j*Qqhfasa)F84ESQ~Xi(+uKcgVDd|+)%+Px9Id3(@F_^?j0&&}!F zb4}#dkbf)H+^4{EZfh8$#f1K0EL3*1dy(Q@=|6X?BD^zV*T29d%mkt5 zTRoD|YT93aKf*^t-y0G?q$FTZu^209(_rXWlNJpfES@nkSf0IetROtT8hU<6>_e>P zjvQz9y5kcI-W*LB!Hg;E!3+#JvEiug(4bFj`O+oh_tdq$G1k)M3vw0u)-z(+Evpwa zt3g9UPKI0rN)Ku9jM_GFlRXch z$ydHje0*_u+K|+GzBd}4*vB>v@ug9a?=ERTLC+1V(0SOfsqruv<6%P z?@9aV+ml-DkV&ytd+)VBkq&%$;yJ9(GbuaNC1TKK!-{+r&SpYl5DdIFb;v0j85@c-_)Lc{y%1RH zSdqLkiYRe-ozgE{T&AgT!}>bH;hwREFoiGGsWc$&NKV`YyEJ(2u{p%2XG}e+0C7}R ztCecCR4FJt`GgHG&wU(CWg$krT9&ecu@3Vz(wVYyi~_=Ea5SHjLq1$RAS;T#h}-ce z!>}1Or7#*=Ju#|J$K}T#%P*eVSI=7?+Kr=L_f`94Pjbtq*`YJ+TWu18CLD(^Tna53 zoK6B@Ey*nn_%dj<=NCy7R3zripUlTf*N=0`m(ee{!2B5GXFg zf8~;4Ch(tGESg*VcOQd-M)+oR_v4eczG&F9~njeP!b>#_FX zS?k4f&B*E6Y3S2Vn9m=-Jj+0#ei#mG`TTf1&W+8S=lAlhH~A|@$Y2%4*KB~ba&~B+ zWeym8u47wQ1IG>ilSaj;R;#@7tOCGyta=vK8N|G}w0x}(4|8CqgV`Gfu@26eHQznU zAmBR7Jlxl^8i$JYJU)wx11|im^?lL>CeHJ_ogm1Md)c-XkQ{t}xDS2wi32YY0fDzc zLU_agI6Mo%Gsr_Rc#>a25UbvVTs_aahm`AW&z|sazh<+4(XMuKtnD4tZF;Fu8+f5) z&vTs#2&?|K$ z5rt44J4~cGQ3xf)2|$sN!-Z-w-Wov0_Z+5W^gTe>Fhr=!TndFQRCDOT`Lq3c9{!I= ziKo`?`G5BxroMXa^{CtOdOe3E-=9Ba9$E`V*=}p(yBdm0d*wdDmhhKaQAmOHF+XyY z9@SyUXfP%;knICSDw-G}tOC4RR6{CaQ3cij8Exaugo<{0M##tk}$k&ah9+ zer;s4R7-{5Rr`bw=B(1jwg_#ISep3)%@<^l4O@)m^i_f^e|)*LUXsi4`={^!xOfqq z5UXs&!k^zDgiOf z7Z~hztxNVIH*|aG<_n*m4nRkFc*vdl7$HDxe%Kj>+H#8U7AZnZ0J@VBsc`FvD(1ro zP$6J1V@}p+>Usp zSw6KKSt5W9p&F!43`2Z2v}_w>IeRiabNdG8U^9YcBnZ_lUOVFO_(`##GY;WZ4rT#$ zakJ_MP3#GlS?nciIke~2v<`Ur1Of<1y$gW)vD`nrK@&5vVBg1t_WEH>Yr~GB(!}Yg z%~-erdENsih(1t;9`ps36lL4^5zV4Q_5#8kywQ~69sEG?vk-abYK}w}Q{^)`HV(zu zU@XZtga%R0HtE2gKigh}h0k+|%Fr{Ix5VF&rMQTr@w$vB*eetqDLvbS3>$c!;~cud zIAV8|KvJX#G7^LuY(6gRb`d{ZPNKr=5eb+GV?(9`(=esmqUHIq2ei7@|58Ya8C?+5 z1G|=}p0Z#-cj`PLfvr^{+~}g;qCu?Bvsh?xj42g4v|GCmS;z}yF!O34%E|()>>0W& zxU`AjsFI4nk~aDLJLU%5Me#2(?L%Q{0`JQ7ppU56i~PA!19;t(Jn5h5#hGS z(itG444dUOJ{mKc<%CdM*22}hp0`ABDB_N>u0)Sx9uyM*M#0wV$-g?@aI&w#s%B*2 zPf=?!jh*hF}KJp=@mt@+rX3&-*v*k)Ik(+ zQnrb1iZX8rW2$5}Mwn^%av{2mu;uj}bcj$T$7>G+pj7Rn#F89JN-K;;I6{#4W7;>a z*R0C`Q zcvA_zQVI!RCxy=Fsv?#cf}=TwoMv4oK^$65QH0~Sb=s6t5rAz$qYSEX@gOK!qnZR} zF|j7;$GZ=s1n?ZfuoUhE0ON%sycAIf)RjtG0H#RW5@@O2VGwI>Y{VzdGy+v>Ab1I& zN`k5H7f3PA7KWgCm={WD5muX@k^uMCgHH98;bPAmn_rH~WCXNr&yUoe&Y_OHAm4xH&Q>1*h^_mi+poKHsO~u>k*W229NYlcCD{29@SIVo>cy&2Rb=tqkTey;ZbAP?$6o$P@@R@ms!Fc z9RIqjsBF2E=-ZkJWy{&S2QaT-aMgjrkn;LR9m{%Qn^Cs-9)r6$>bSs6sgclc7ErxC zqZEt7s>`+v+Q#A$i6weHW> zvoM$cQQXFV@66>@3G@7qJG$Rz)$L|z#?XHKNEbh{3uVyXWB0$LdwZjXYZ}cjG;mrU zonClRseY<-V!KJ}UHYgbkuN*&w@yG8E(7eqF}uTHcG&{Eg;T*4OE1Pabt&B163X6! z-B<-7l}jyDydw7XSB;GYmt_9-PA1l6x;y8RDD~H!Kzm#rQ0^h7m3dRI)6-B7!T10UJ8`f9ExJ1D3{az|!Cs zbFv;N7t36;2P~_$wpYlNe#;vu)+n5E3TqDO!cj>0$*C4}PE}Aa z$UxIm2;|u1L!1m;MH|M>Bmu3ceIy)Z$ipc}c!f;F?&49H@!6R`g_}xc2{#el((p<% zcsYJTs%F3w$Tp+4_5kw7m1Z%u2@+cHS|iTB+EFYSk>y?IOpB9Opf;p!dm&uWn;_0Q zr%Vn$29*0q6s2QQ^?Lf2x=!jjlCjkBlE6Qua&kiRmP-^;MR3x#Qt}{BZJm zd~)=pIrwt%=1-?x`19)Zt>#y+TG!{3uGxLN1-N!dS)qO`46Wj+?f0z&wi4J%;75_b N{{g4FoT&h0008P`%|!qJ literal 0 HcmV?d00001 diff --git a/docs/upgrades/packages-7.8.5/style_01.wgpkg b/docs/upgrades/packages-7.8.5/style_01.wgpkg new file mode 100644 index 0000000000000000000000000000000000000000..a23322b09c86217e16b0bbdcb93f165d911cb963 GIT binary patch literal 1753 zcmV;~1}6C*iwFP!00000|Ls_9bJI8w&S!pw@y#(|hQxMWo8apVO*z< zM;u=w&XF^j;(#+<>uR6BypZdRhZK2OeH76!!juy%1tp9?@)6?)%P6EAyRYO@F!Ozh zB9R?wPZ!`6a*3};v$0Ru1iRT{fEOQ0;D!sSWo8R(6@(EZ+jj7hQ*?xp zJ47BEAmuY7!ITA4qHG0YP<#_=3W5qY}e z4@H)+0S$S`S5drgWm|+D5!c1R=5{?A&Z541;1VvEtXgJzokF=_Kixm=4gVY*XcIp5 zwZZw`alfaH^on78toICKf4HxGIvTz`)~u?jopDMWZWsqABOQb$oJU>5SS%LRMZFqQ z<WpmIBBUipgz-Nv*2O-I2aKqNBi;ww6;rV1E>$Se;HMR!$WlK~iUyNOPnx(T}&vxf?pR0q7GR33eCkr zH441vFJV~(T?kJB(}0KY$S43iJ34(o?41v_e(zMo<{DIhmZ*7%wXjik7&8`LWsDfE z!o8G@y_MwwwT^k_6o=w7Ky1sW3{a|BLddl4jL2oK3=w9zWqTaDtN89|rSh3vY5fCj zXV-SgTqCZC65a^|e+5aipMJ)HORm0DDw_)JoI?9{h1QpxM(Y+0pJU-yv)WYkd|n~A z*(`8v#4ldCQxfbUT84l_c|wh)Tbd#QRgz?Ub3wEXOqFF-K-OJu4J;j%FC4J4wlm_O zVSusJ06dW%$d%L!CyZ8m|$ShCR0yi)Pb73I3zwkr+zsBkE#FVmi@AY(4*DPQ`1N^T+na$uV%Qi^*C6akH% zt5jDq$TRYHtX!`*6p9xi3T#!PUK8eSSCG2xpc8f+3Jx6e?i>pyY#(2tnZK1+HT2SE zr_=6qn(dlGR4JYl1|isAllOH3%uW_%Ai6-n*Ev@LIb%4c=4+xZ%_#6}5-$3|VMwQO zxD?$03XVm{iQ^=XYi1D#4qZiDFvd;DHBHX`6X*|4mhVKAZAsL!0U>_I^w^f=TTG1Y zUqgvim^+i@|A7W?LM4s+rwFheQ{R~Osw6Q#rNzidnVJ~eDJ{$%%LIsNmJ-0E z(WZ(j#oMNJiFDkYQypO}qioVz5m8g8&+*nVtA|h6ZE$G*^*g&}y>}`6bDyo!9 zQ7U#wWi45N6}pt2fE-1-8}LS?W7_k>WJmf{;QkL&95p&g2^SL^&Hn%SjFyBpj#*3< z;Oq#Rkab%4pPsz#I9s&#|7yK`?JkdppT@mC@9K1X;$@PAovq-N$aJ$I!GMQd?!nqys8(TD%MU{^a{Tzmj`|e~iFm1Rf*sAQAW%TNYkl02}}S#??%& literal 0 HcmV?d00001 diff --git a/docs/upgrades/packages-7.8.5/style_02.wgpkg b/docs/upgrades/packages-7.8.5/style_02.wgpkg new file mode 100644 index 0000000000000000000000000000000000000000..cf0e8adb935aef4fc369f2a546293f70bf4b1441 GIT binary patch literal 2030 zcmVySJ_b z+G1q3vLsOSYZv?O_wY~>Dcfn%tSPV|F6!7kJP)21&(EsvJ^agYoOZLhx92!+yXo%j z)!XW2I#Y&IJk@vk@95dZJu#fOT2^&2_wfyMmq2Pndl zptCq2f?g3J$YeUBp-4(r*ZT1BXQU@0;>4%sQp}^6a=|EWL@`oQKoW7hiX$$l_X0`b zJP1&VBu_X6HQwP`iYnDhIRUCrp!`xi9|tU%Qcn{@x_r+>1%eAIlXV!y3EQ@%z`Vs4EknvL6Ut9fq-pD%2WUtl)&ae8YT6!>BvhmfiJ%LL zTd=y+o3WQA=R6XTSjXzz#<5s(!aR?Lo5%HeG>=ccV~+_WxrVqbgh=Q;IeOb4{OA1G znu=LqonIcFo%XF^sbbsj>wVik8XQ@FoD5!{S#H&_E(B*2VcW-Vh9%fE6(a81_Hwzb zF6-5Z`}W|Sy^?}l$(VkuB*<0ugf}c54EZpW|9hx!H4DPzmtB(FPN$;;q)fdjL{G7c zo`5X8S`co@#j1dq<`gCZdN7*oTNP_`4yjlca^=~6&&ChgzJP_FElvYa&L&X^=b3VP z4c|t`GF6iDHCaePH4gn}`#_fNmz!rG(}qP@vQb!eH}wpW7bkDu4f>Y@>$Lw?`sxPR zpa5m>5|pvn9O_vpy#oL#XRXR`aBAQH3YHul$G;FS;aL;tkFqiXdM)}< zFrUd3U66qJA*_lh&T2`({@mL#E|eQ2lZg2bw9!c!>O)op@TDq;1u?kuY799pE2+3| z<6EvehPoKYQagNggZ_L6(0kL`!A5z)3N{C@1gtLDw(%|33H2${y{Iqm4Kg*WxFr+^ zQYieB-A9Wz)}TG;?xNk#Wt8sj|9E`0^p)c>Rb~W`T(B0XBZe%ss4Tful6Ea8Udh%)0s(v|(?X%JW*_uyacTSp4K_$Y3Eci*$8ei@P)l;31+9n%IbEUr~5O)eHK<^t#rv;MA-sH zPpyo>m-1E&y{-k)Y1I*cjH} zuH2lw6b%o7p|ZxvRHGMif`Y+lwJu)=bOn>>Ts3N=zB3i7b!U!$%$w~< z{cH0kh@dlE$oU&4A~S!^%EQvI^mF=lekiXEWB!JZjmEH~Q)(#GL@+*667Gbo~tePEx%}M?r&G zIg*f$V};QNd+QhGFgaG;wg+al+qS^`w|k4Nr$t-N{}>MqidZ57*a1`=;8V-BF7E-23}0BfP8?<(I4VN&6}^w!Eh%kOeLu+C6}kMpU&0- z;>*|o*O^YhEA}QtXPBjt+{Pl!!u5l*`QilymT(fi$yE`Rs7ESzn!msJ$-lbo7RBOqzdz=+i15M zwWib1n_z4P*pk}Zt~l`c1Hh+&EbGQxna`xD(efGEUqAfyz>o4C#XoxB(F2bj_$m+l MALVQC-vA^40NmO9N&o-= literal 0 HcmV?d00001 diff --git a/docs/upgrades/packages-7.8.5/style_03.wgpkg b/docs/upgrades/packages-7.8.5/style_03.wgpkg new file mode 100644 index 0000000000000000000000000000000000000000..3791ffe8c399ef5e8c68e0e13ce910481dd27803 GIT binary patch literal 1768 zcmVrYNGLZ=r;Nm|kJPOwCJ~AmMuJhO;hF-MvZF;rVurkZK?iZ|_`1&iGiBg3~wtoLe~-rYG3*P`G|vKPJ8PmSN22^YXk}Cb4hy zE{p|NWO2cCS)xL!>@lye@Acz;=m=YjNf6TCw`sAO&1NE;3%~^l(LQq+WZ;*X8*5|O zEHk*aoPt?^_6DO}t)vaQkcLIZcV6x~hFD~L0S<4p7zJP)k4Ok-8OP`v-UdfJ8))&` zopC}r3jLS6K<1(6%oEUQz$R=NLRmJK97W~j$@xXEebv)C?Q#nat_~H9jwuKH+PvbEMTj zry&J9LU9%;=}6AdXMf-cFLw>9qBt;$swX9!9uE!(3DRev&=3sSojw1ALJwbmE|sz! zgWtb+afZZOCM)UN&Yt7pnT9=nacGBx!4k5`S<~Z4fyRnJ<9-1+y=tO7^YlD4ng2MwhE~Y`@m6IQ7;$leLEEf4Tj|qx%0)%mm)Z-PQ&^I|Qus?>J zN>%f#PYUQD225w@!($LkT_4^V#T z$;E@qw^$ctyyhEtH@5Z$Cq*cHUh43Y$m%qu=YIKXlynZU11v)l!@aaLGg|P5;sy*h6C^2$)iK;XXBek# zCG~s7E7eDJJ=o`?1SZ5{ncDv{A|!yxJk5(ql$6W_o;wFnGD;vN5&9m3slE^MSD{0Y z^S;Zgt(>yuVN_jG^M#S9O=6gzJY=f}$}@9F7$cKyY*{bJS+5CUEBUS38j)xjZ^_;l zoB$bjSp01XDF}HYFMri1(6;U3ry&L`aubqyfIJ_S!@#&Z1ES%7%K*SqIMjv#;Er3; zK=n_q<%*vqYho)MM#TD#cE`KdeLT<-?_+^ByPd_5P1)e{USx_yAvheP7&EAv1A znH-(IB`|Psf(Lc0V(*yMdbPGARir5EJw^%MY)F!5RN_3B9Q$c1_QRCpGItmx=dz>} zy}{8LVPJLSGE6pL9WQFpz;Dsa&omVnS9c@8-7^&%3&?qi|4yt{t94r%j@%gFt=_%o$IRWY`uo=@sbnQ-qjeAu?FKjlOjb_tsR1{k1W!96eeXijD&YMA46Um9Zrr>MZGQkOud7N2`GZh0 zAP^2r6HuyxI5}fe@Fq97L^2}tNtbMiYUi_NZ9fBPaz6uRKMKXtcAnc4slYv02U*Tm zSe1xtTgPaEcODr(rT!;FN&Mc4kYoTAR3tZ7(U7c_6%4cyvJK256>h~klAPlkxuv3- zO(b(yy#EK-vKf6R0Rm7J90bTt3V+c6( KR#>newByDynamicClass($session, 'PcRRPhh-0KfvLLNIPdxJTw'); + if ($snippet) { + $snippet->purge; + } + # and here's our code + print "DONE!\n" unless $quiet; +} + #---------------------------------------------------------------------------- # Describe what our function does sub configEMSActivities { From 7c94f6f8baf2d1b67b4a9df282d82d09c79523ce Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Thu, 12 Nov 2009 22:21:34 -0800 Subject: [PATCH 14/20] Fix pagination issues in the List LDAPLinks screen. --- lib/WebGUI/Operation/LDAPLink.pm | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/WebGUI/Operation/LDAPLink.pm b/lib/WebGUI/Operation/LDAPLink.pm index d303a06d5..cac6aad64 100644 --- a/lib/WebGUI/Operation/LDAPLink.pm +++ b/lib/WebGUI/Operation/LDAPLink.pm @@ -361,16 +361,16 @@ links. Each LDAP link is tested and the status of that test is returned. sub www_listLDAPLinks { my $session = shift; return $session->privilege->adminOnly() unless canView($session); - my ($output, $p, $sth, $data, @row, $i); my $i18n = WebGUI::International->new($session,"AuthLDAP"); my $returnUrl = ""; if ($session->form->process("returnUrl")) { $returnUrl = ";returnUrl=".$session->url->escape($session->form->process("returnUrl")); } - $sth = $session->db->read("select * from ldapLink order by ldapLinkName"); - $row[$i] = ' '.$i18n->get("LDAPLink_1076").''.$i18n->get("LDAPLink_1077").''; + my $sth = $session->db->read("select * from ldapLink order by ldapLinkName"); + my $i = 0; + my @row = (); $i++; - while ($data = $sth->hashRef) { + while (my $data = $sth->hashRef) { $row[$i] = '' .$session->icon->delete('op=deleteLDAPLink;llid='.$data->{ldapLinkId},$session->url->page(),$i18n->get("LDAPLink_988")) .$session->icon->edit('op=editLDAPLink;llid='.$data->{ldapLinkId}.$returnUrl) @@ -391,9 +391,14 @@ sub www_listLDAPLinks { $i++; } $sth->finish; - $p = WebGUI::Paginator->new($session,$session->url->page('op=listLDAPLinks')); + my $p = WebGUI::Paginator->new($session,$session->url->page('op=listLDAPLinks')); $p->setDataByArrayRef(\@row); - $output .= ''; + my $output = qq{
\n}; + $output .= q{\n}; $output .= $p->getPage; $output .= '
 } + . $i18n->get("LDAPLink_1076") + . q{} + . $i18n->get("LDAPLink_1077") + . qq{
'; $output .= $p->getBarTraditional; From bc21f904da64f37edbb904b296d90677e29a5a5b Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 09:49:36 -0800 Subject: [PATCH 15/20] Fix status reporting for successful connects but bad binds. Fixes bug #11216 --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/LDAPLink.pm | 24 +++++++--- lib/WebGUI/Operation/LDAPLink.pm | 3 +- t/LDAPLink.t | 82 ++++++++++++++++++++++++++++++++ t/lib/WebGUI/Test.pm | 27 +++++++++++ 5 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 t/LDAPLink.t diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 7ee41a343..2c03a7006 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -12,6 +12,7 @@ - fixed #11220: Map asset badly broken - fixed #11222: testEnvironment.pl Missing Dependencies - fixed #11226: New stylesheet (wg-base.css), new style templates (from the TWG) + - fixed #11216: LDAP Connections status incorrect 7.8.4 - Fixed a compatibility problem between WRE and new Spectre code. diff --git a/lib/WebGUI/LDAPLink.pm b/lib/WebGUI/LDAPLink.pm index daa358ff1..49a97f2a9 100644 --- a/lib/WebGUI/LDAPLink.pm +++ b/lib/WebGUI/LDAPLink.pm @@ -141,6 +141,19 @@ sub get { #------------------------------------------------------------------- +=head2 getErrorCode ( ) + +Returns the numerical error code generated by the bind() method. + +=cut + +sub getErrorCode { + my $self = shift; + return $self->{_error}; +} + +#------------------------------------------------------------------- + =head2 getErrorMessage ( [ldapErrorCode] ) Returns the error string representing the error code generated by Net::LDAP. If no code is passed in, the most recent error stored by the class is returned @@ -153,7 +166,7 @@ A valid ldap error code. sub getErrorMessage { my $self = shift; - my $errorCode = shift || $self->{_error}; + my $errorCode = shift || $self->getErrorMessage; return "" unless $errorCode; my $i18nCode = "LDAPLink_".$errorCode; my $i18n = WebGUI::International->new($self->session,"AuthLDAP"); @@ -242,12 +255,11 @@ The ldapLinkId of the ldapLink you're creating an object reference for. =cut sub new { - my ($ldapLinkId, $ldapLink); - my $class = shift; - my $session = shift; - $ldapLinkId = shift; + my $class = shift; + my $session = shift; + my $ldapLinkId = shift; return undef unless $ldapLinkId; - $ldapLink = $session->db->quickHashRef("select * from ldapLink where ldapLinkId=?",[$ldapLinkId]); + my $ldapLink = $session->db->quickHashRef("select * from ldapLink where ldapLinkId=?",[$ldapLinkId]); bless {_session=>$session, _ldapLinkId=>$ldapLinkId, _ldapLink=>$ldapLink }, $class; } diff --git a/lib/WebGUI/Operation/LDAPLink.pm b/lib/WebGUI/Operation/LDAPLink.pm index cac6aad64..5d7e8690a 100644 --- a/lib/WebGUI/Operation/LDAPLink.pm +++ b/lib/WebGUI/Operation/LDAPLink.pm @@ -380,10 +380,11 @@ sub www_listLDAPLinks { my $ldapLink = WebGUI::LDAPLink->new($session,$data->{ldapLinkId}); my $status = $i18n->get("LDAPLink_1078"); - if ($ldapLink->bind) { + if ($ldapLink->bind && $ldapLink->getErrorCode == 0) { $status = $i18n->get("LDAPLink_1079"); } else { $session->errorHandler->warn($ldapLink->getErrorMessage()); + $status .= ": ".$ldapLink->getErrorMessage(); } $ldapLink->unbind; $row[$i] .= ''.$status.''; diff --git a/t/LDAPLink.t b/t/LDAPLink.t new file mode 100644 index 000000000..f068a35eb --- /dev/null +++ b/t/LDAPLink.t @@ -0,0 +1,82 @@ +# vim:syntax=perl +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2009 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------ +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------ + +# Test Auth::LDAP to make sure it works with both ldap and ldaps +# +# + +use FindBin; +use strict; +use lib "$FindBin::Bin/lib"; +use Test::More; +use Test::Deep; +use Data::Dumper; +use WebGUI::Test; # Must use this before any other WebGUI modules +use WebGUI::Session; +use WebGUI::LDAPLink; + +#---------------------------------------------------------------------------- +# Init +my $session = WebGUI::Test->session; + +#---------------------------------------------------------------------------- +# Tests + +plan tests => 8; # Increment this number for each test you create + + +########################################################################### +# +# new +# +########################################################################### + +{ + my $ldap = WebGUI::LDAPLink->new($session, "new"); + addToCleanup($ldap); + isa_ok($ldap, 'WebGUI::LDAPLink'); + is $ldap->{_ldapLinkId}, "new", '... created with correct linkId'; +} + +########################################################################### +# +# successful bind +# +########################################################################### + +{ + my $ldapProps = WebGUI::Test->getSmokeLDAPProps(); + $session->db->setRow('ldapLink', 'ldapLinkId', $ldapProps, $ldapProps->{ldapLinkId}); + my $ldap = WebGUI::LDAPLink->new($session, $ldapProps->{ldapLinkId}); + addToCleanup($ldap); + cmp_deeply $ldap->get(), superhashof($ldapProps), 'all db properties retrieved'; + my $connection = $ldap->bind(); + isa_ok $connection, 'Net::LDAP', 'returned by bind'; + is $ldap->{'_error'}, undef, 'no errors from binding' +} + +########################################################################### +# +# failed bind +# +########################################################################### + +{ + my $ldapProps = WebGUI::Test->getSmokeLDAPProps(); + $ldapProps->{identifier} = 'hadley'; + $session->db->setRow('ldapLink', 'ldapLinkId', $ldapProps, $ldapProps->{ldapLinkId}); + my $ldap = WebGUI::LDAPLink->new($session, $ldapProps->{ldapLinkId}); + addToCleanup($ldap); + my $connection = $ldap->bind(); + isa_ok $connection, 'Net::LDAP', 'returned by bind'; + is $ldap->{_error}, 104, 'auth error due to bad identifier'; + is $ldap->getErrorCode, 104, 'getErrorCode returns the stored error code'; +} diff --git a/t/lib/WebGUI/Test.pm b/t/lib/WebGUI/Test.pm index 05b4e5154..afe889baa 100644 --- a/t/lib/WebGUI/Test.pm +++ b/t/lib/WebGUI/Test.pm @@ -113,6 +113,7 @@ sub import { 'Transaction Items' => 'transactionItem', 'Ship Drivers' => 'shipper', 'Database Links' => 'databaseLink', + 'LDAP Links' => 'ldapLink', ); my %initCounts; for ( my $i = 0; $i < @checkCount; $i += 2) { @@ -497,6 +498,27 @@ sub webguiBirthday { #---------------------------------------------------------------------------- +=head2 getSmokeLDAPProps ( ) + +Returns a hashref of properties for connecting to smoke's LDAP server. + +=cut + +sub getSmokeLDAPProps { + my $ldapProps = { + ldapLinkName => "Test LDAP Link", + ldapUrl => "ldaps://smoke.plainblack.com/ou=Convicts,o=shawshank", # Always test ldaps + connectDn => "cn=Samuel Norton,ou=Warden,o=shawshank", + identifier => "gooey", + ldapUserRDN => "dn", + ldapIdentity => "cn", + ldapLinkId => sprintf( '%022s', "testlink" ), + }; + return $ldapProps; +} + +#---------------------------------------------------------------------------- + =head2 prepareMailServer ( ) Prepare a Net::SMTP::Server to use for testing mail. @@ -772,6 +794,7 @@ were passed in. Currently able to destroy: WebGUI::Shop::ShipDriver WebGUI::Shop::Transaction WebGUI::DatabaseLink + WebGUI::LDAPLink Example call: @@ -865,6 +888,10 @@ Example call: $session->var->end; $session->close; }, + 'WebGUI::LDAPLink' => sub { + my $link = shift; + $link->session->db->write("delete from ldapLink where ldapLinkId=?", [$link->{ldapLinkId}]); + }, ); sub cleanupGuard { From 6c6270346eabfc5226013c4400b273a015191f01 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 10:46:20 -0800 Subject: [PATCH 16/20] Add a test to check what the login method returns in the normal case, with no login messages. --- t/Auth.t | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/Auth.t b/t/Auth.t index 281fb010a..571011797 100644 --- a/t/Auth.t +++ b/t/Auth.t @@ -33,7 +33,7 @@ my ($request, $oldRequest, $output); #---------------------------------------------------------------------------- # Tests -plan tests => 2; # Increment this number for each test you create +plan tests => 3; # Increment this number for each test you create #---------------------------------------------------------------------------- # Test createAccountSave and returnUrl together @@ -71,12 +71,14 @@ $session->{_request} = $request; $auth = WebGUI::Auth->new( $session, $AUTH_METHOD, 3 ); my $username = $session->id->generate; push @cleanupUsernames, $username; +$session->setting->set('showMessageOnLogin', 0); $output = $auth->login; is( $session->http->getRedirectLocation, 'REDIRECT_LOGIN_URL', "returnUrl field is used to set redirect after login", ); +is $output, undef, 'login returns undef when showMessageOnLogin is false'; # Session Cleanup $session->{_request} = $oldRequest; From b88a7bc190b18af02f3df1f5b1313195f56c396c Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 11:19:25 -0800 Subject: [PATCH 17/20] Escape single quotes sent to the ProgressBar. Fixes but #11229. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/AssetBranch.pm | 4 +++- lib/WebGUI/ProgressBar.pm | 3 ++- lib/WebGUI/i18n/English/Asset.pm | 12 ++++++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 2c03a7006..9e7c894ac 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -13,6 +13,7 @@ - fixed #11222: testEnvironment.pl Missing Dependencies - fixed #11226: New stylesheet (wg-base.css), new style templates (from the TWG) - fixed #11216: LDAP Connections status incorrect + - fixed #11229: ProgressBar throws errors on some messages. 7.8.4 - Fixed a compatibility problem between WRE and new Spectre code. diff --git a/lib/WebGUI/AssetBranch.pm b/lib/WebGUI/AssetBranch.pm index 6dd5dd746..faca1854c 100644 --- a/lib/WebGUI/AssetBranch.pm +++ b/lib/WebGUI/AssetBranch.pm @@ -317,6 +317,8 @@ sub www_editBranchSave { my %data; my $pb = WebGUI::ProgressBar->new($session); my $i18n = WebGUI::International->new($session, 'Asset'); + $pb->start($i18n->get('edit branch'), $session->url->extras('adminConsole/assets.gif')); + $pb->update($i18n->get('Processing form data')); $data{isHidden} = $form->yesNo("isHidden") if ($form->yesNo("change_isHidden")); $data{newWindow} = $form->yesNo("newWindow") if ($form->yesNo("change_newWindow")); $data{encryptPage} = $form->yesNo("encryptPage") if ($form->yesNo("change_encryptPage")); @@ -353,7 +355,6 @@ sub www_editBranchSave { $urlBase = $form->text("baseUrl"); $endOfUrl = $form->selectBox("endOfUrl"); } - $pb->start($i18n->get('edit branch'), $session->url->extras('adminConsole/assets.gif')); my $descendants = $self->getLineage(["self","descendants"],{returnObjects=>1}); DESCENDANT: foreach my $descendant (@{$descendants}) { if ( !$descendant->canEdit ) { @@ -401,6 +402,7 @@ sub www_editBranchSave { } } } + $pb->update(sprintf $i18n->get('Attempting to commit changes')); if (WebGUI::VersionTag->autoCommitWorkingIfEnabled($self->session, { allowComments => 1, returnUrl => $self->getUrl, diff --git a/lib/WebGUI/ProgressBar.pm b/lib/WebGUI/ProgressBar.pm index 6fb9990eb..5be25932a 100644 --- a/lib/WebGUI/ProgressBar.pm +++ b/lib/WebGUI/ProgressBar.pm @@ -147,7 +147,8 @@ A message to be displayed in the status bar. sub update { my $self = shift; - my $message = shift; ##JS string escaping? + my $message = shift; + $message =~ s/'/\\'/g; ##Encode single quotes for JSON; $self->session->log->preventDebugOutput; $self->{_counter} += 1; diff --git a/lib/WebGUI/i18n/English/Asset.pm b/lib/WebGUI/i18n/English/Asset.pm index 72817513f..708f23973 100644 --- a/lib/WebGUI/i18n/English/Asset.pm +++ b/lib/WebGUI/i18n/English/Asset.pm @@ -343,12 +343,24 @@ our $I18N = { context => q|To skip, to move over, to not process| }, + 'Processing form data' => { + message => q|Processing form data|, + lastUpdated => 1245343280, + context => q|To edit or change| + }, + 'editing %s' => { message => q|editing %s|, lastUpdated => 1245343280, context => q|To edit or change| }, + 'Attempting to commit changes' => { + message => q|Attempting to commit changes|, + lastUpdated => 1245343280, + context => q||, + }, + 'this asset only' => { message => q|This Asset Only|, lastUpdated => 0, From c4e63dfef0449155a72d22125827c093035557fb Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 16:17:18 -0800 Subject: [PATCH 18/20] Auto update user's DN if it changes on the LDAP server. Fixes bug #11217 --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Auth.pm | 4 +- lib/WebGUI/Auth/LDAP.pm | 43 ++++++++++++++-------- lib/WebGUI/LDAPLink.pm | 5 ++- t/Auth/LDAP.t | 79 ++++++++++++++++++++++++++++++++++++++-- 5 files changed, 110 insertions(+), 22 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 9e7c894ac..56035044f 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -14,6 +14,7 @@ - fixed #11226: New stylesheet (wg-base.css), new style templates (from the TWG) - fixed #11216: LDAP Connections status incorrect - fixed #11229: ProgressBar throws errors on some messages. + - fixed #11217: LDAP authentication fails if user DN changes 7.8.4 - Fixed a compatibility problem between WRE and new Spectre code. diff --git a/lib/WebGUI/Auth.pm b/lib/WebGUI/Auth.pm index 363771176..222a59083 100644 --- a/lib/WebGUI/Auth.pm +++ b/lib/WebGUI/Auth.pm @@ -701,8 +701,8 @@ Returns a hash reference with the user's authentication information. This metho =cut sub getParams { - my $self = shift; - my $userId = $_[0] || $self->userId; + my $self = shift; + my $userId = $_[0] || $self->userId; my $authMethod = $_[1] || $self->authMethod; return $self->session->db->buildHashRef("select fieldName, fieldData from authentication where userId=".$self->session->db->quote($userId)." and authMethod=".$self->session->db->quote($authMethod)); } diff --git a/lib/WebGUI/Auth/LDAP.pm b/lib/WebGUI/Auth/LDAP.pm index c80d5a0f9..c60d2bf84 100644 --- a/lib/WebGUI/Auth/LDAP.pm +++ b/lib/WebGUI/Auth/LDAP.pm @@ -40,9 +40,11 @@ i.e., it does not validate their username or ensure their account is active. =cut sub _isValidLDAPUser { - my $self = shift; + my $self = shift; + my $session = $self->session; + my $form = $session->form; my ($error, $ldap, $search, $auth, $connectDN); - my $i18n = WebGUI::International->new($self->session); + my $i18n = WebGUI::International->new($session); my $connection = $self->getLDAPConnection; return 0 unless $connection; @@ -53,8 +55,8 @@ sub _isValidLDAPUser { $self->error('
  • '.$i18n->get(2,'AuthLDAP').'
    • '); return 0; } - my $username = $self->session->form->get("authLDAP_ldapId") || $self->session->form->get("username"); - my $password = $self->session->form->get("authLDAP_identifier") || $self->session->form->get("identifier"); + my $username = $form->get("authLDAP_ldapId") || $form->get("username"); + my $password = $form->get("authLDAP_identifier") || $form->get("identifier"); my $uri = URI->new($connection->{ldapUrl}) or $error = '
  • '.$i18n->get(2,'AuthLDAP').'
    • '; @@ -102,27 +104,27 @@ sub _isValidLDAPUser { # Invalid login credentials, directory did not authenticate the user if ($auth->code == 48 || $auth->code == 49) { $error .= '
  • '.$i18n->get(68).'
    • '; - $self->session->errorHandler->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process('authLDAP_ldapId')); + $session->log->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process('authLDAP_ldapId')); } elsif ($auth->code > 0) { # Some other LDAP error occured $error .= '
  • LDAP error "'.$self->ldapStatusCode($auth->code).'" occured. '.$i18n->get(69).'
    • '; - $self->session->errorHandler->error("LDAP error: ".$self->ldapStatusCode($auth->code)); + $session->log->error("LDAP error: ".$self->ldapStatusCode($auth->code)); } $ldap->unbind; } else { # Could not find the user in the directory to build a DN $error .= '
  • '.$i18n->get(68).'
    • '; - $self->session->errorHandler->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process("authLDAP_ldapId")); + $session->log->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process("authLDAP_ldapId")); } } else { # Unable to bind with proxy user credentials or anonymously for our search $error = '
  • '.$i18n->get(2,'AuthLDAP').'
    • '; - $self->session->errorHandler->error("Couldn't bind to LDAP server: ".$connection->{ldapUrl}); + $session->log->error("Couldn't bind to LDAP server: ".$connection->{ldapUrl}); } } else { # Could not create our LDAP object $error = '
  • '.$i18n->get(2,'AuthLDAP').'
    • '; - $self->session->errorHandler->error("Couldn't create LDAP object: ".$connection->{ldapUrl}); + $session->log->error("Couldn't create LDAP object: ".$connection->{ldapUrl}); } $self->error($error); @@ -176,21 +178,32 @@ sub authenticate { # Try to bind using the users dn and password $auth = $ldap->bind(dn=>$userData->{connectDN}, password=>$identifier); + + # Failure to bind could have resulted from change to in DN on LDAP server. + # Test for new DN and update user account as needed + if ($auth->code > 0 && $self->_isValidLDAPUser()) { + # Update user profile and log change + # _isValidLDAPUser will set _connectDN to new correct value + $auth = $ldap->bind(dn=>$self->{_connectDN}, password=>$identifier); + my $message = "DN has been changed for user ".$_[0]." from \"".$userData->{connectDN}."\" to \"".$self->{_connectDN}."\""; + $self->saveParams($self->user->userId, $self->authMethod, { connectDN => $self->{_connectDN} }); + $self->session->errorHandler->warn($message); + } # Authentication failed - if ($auth->code == 48 || $auth->code == 49){ + if ($auth->code == 48 || $auth->code == 49 || $auth->code == 32){ $error .= $self->SUPER::authenticationError; } elsif ($auth->code > 0) { # Some other LDAP error happened $error .= '
  • LDAP error "'.$self->ldapStatusCode($auth->code).'" occured.'.$i18n->get(69).'
    • '; - $self->session->errorHandler->error("LDAP error: ".$self->ldapStatusCode($auth->code)); + $self->session->log->error("LDAP error: ".$self->ldapStatusCode($auth->code)); } $ldap->unbind; } else { $error .= '
  • '.$i18n->get(13,'AuthLDAP').'
    • '; - $self->session->errorHandler->error("Could not process this LDAP URL: ".$userData->{ldapUrl}); + $self->session->log->error("Could not process this LDAP URL: ".$userData->{ldapUrl}); } if($error ne ""){ @@ -645,8 +658,8 @@ Process the login form. Create a new account if auto registration is enabled. sub login { my $self = shift; my $i18n = WebGUI::International->new($self->session); - my $username = $self->session->form->process("username"); - my $identifier = $self->session->form->process("identifier"); + my $username = $self->session->form->process("username"); + my $identifier = $self->session->form->process("identifier"); my $autoRegistration = $self->session->setting->get("automaticLDAPRegistration"); my $hasAuthenticated = 0; @@ -684,7 +697,7 @@ sub login { } return $self->SUPER::login() if $hasAuthenticated; #Standard login routine for login - $self->session->errorHandler->security("login to account ".$self->session->form->process("username")." with invalid information."); + $self->session->log->security("login to account ".$self->session->form->process("username")." with invalid information."); return $self->displayLogin("

    ".$i18n->get(70)."

    ".$self->error); } diff --git a/lib/WebGUI/LDAPLink.pm b/lib/WebGUI/LDAPLink.pm index 49a97f2a9..2d6008f04 100644 --- a/lib/WebGUI/LDAPLink.pm +++ b/lib/WebGUI/LDAPLink.pm @@ -49,8 +49,9 @@ These subroutines are available from this package: =head2 bind ( ) -Authenticates against the ldap server with the parameters stored in the class, returning a valid ldap connection, or 0 if a connection -cannot be established +Authenticates against the ldap server with the parameters stored in the +class, returning a valid ldap connection, or 0 if a connection cannot +be established =cut diff --git a/t/Auth/LDAP.t b/t/Auth/LDAP.t index 35971413c..24def1b75 100644 --- a/t/Auth/LDAP.t +++ b/t/Auth/LDAP.t @@ -19,6 +19,7 @@ use lib "$FindBin::Bin/../lib"; use Test::More; use WebGUI::Test; # Must use this before any other WebGUI modules use WebGUI::Session; +use Test::Deep; use Scope::Guard; #---------------------------------------------------------------------------- @@ -36,7 +37,8 @@ my $ldapProps = { ldapLinkId => sprintf( '%022s', "testlink" ), }; $session->db->setRow("ldapLink","ldapLinkId",$ldapProps, $ldapProps->{ldapLinkId}); -my $ldap = WebGUI::LDAPLink->new( $session, $ldapProps->{ldapLinkId} ); +my $ldapLink = WebGUI::LDAPLink->new( $session, $ldapProps->{ldapLinkId} ); +my $ldap = $ldapLink->bind; $session->setting->set('ldapConnection', $ldapProps->{ldapLinkId} ); # Cleanup @@ -50,7 +52,7 @@ my @cleanup = ( #---------------------------------------------------------------------------- # Tests -plan tests => 3; # Increment this number for each test you create +plan tests => 8; # Increment this number for each test you create #---------------------------------------------------------------------------- # Test Login of existing user @@ -110,5 +112,76 @@ is( $session->user->get('username'), 'Bogs Diamond', 'Bogs was created' ) or diag( $auth->error ); WebGUI::Test->addToCleanup( $session->user ); -$session->user({ userId => 1 }); # Restore Visitor +$session->setting->set('automaticLDAPRegistration', 0); +$session->user({ userId => 1 }); # Restore Visitor + +#---------------------------------------------------------------------------- +# Test DN reset from LDAP + +$session->setting->set('automaticLDAPRegistration', 1); +my $result = $ldap->add( 'cn=Brooks Hatley,ou=Convicts,o=shawshank', + attr => [ + cn => 'Brooks Hatley', + givenName => 'Brooks', + sn => 'Hatley', + ou => 'Convicts', + o => 'shawshank', + objectClass => [ qw( top inetOrgPerson ) ], + userPassword => 'BrooksHatley', + ] +); + +$session->request->setup_body({ + username => 'Brooks Hatley', + identifier => 'BrooksHatley', +}); +$auth = WebGUI::Auth::LDAP->new( $session, 'LDAP' ); +$out = $auth->login; +is $session->user->get('username'), 'Brooks Hatley', 'Brooks was created'; +cmp_deeply( + $auth->getParams, + { + connectDN => 'cn=Brooks Hatley,ou=Convicts,o=shawshank', + ldapConnection => '00000000000000testlink', + ldapUrl => 'ldaps://smoke.plainblack.com/ou=Convicts,o=shawshank', + }, + 'authentication information set after creating account' +); +WebGUI::Test->addToCleanup( $session->user, ); +$out = $auth->logout; +is $session->user->get('username'), 'Visitor', 'Brooks was logged out'; + +$ldap->moddn( 'cn=Brooks Hatley,ou=Convicts,o=shawshank', + newrdn => 'cn=Brooks Hatlen', +); + +$ldap->modify( 'cn=Brooks Hatlen,ou=Convicts,o=shawshank', + replace => { + sn => 'Hatlen', + userPassword => 'BrooksHatlen', + }, +); + +$session->request->setup_body({ + username => 'Brooks Hatley', + identifier => 'BrooksHatlen', +}); + +$auth = WebGUI::Auth::LDAP->new( $session, 'LDAP' ); +$out = $auth->login; +is $session->user->get('username'), 'Brooks Hatley', 'Brooks was logged in after name change'; +cmp_deeply( + $auth->getParams, + { + connectDN => 'cn=Brooks Hatlen,ou=Convicts,o=shawshank', + ldapConnection => '00000000000000testlink', + ldapUrl => 'ldaps://smoke.plainblack.com/ou=Convicts,o=shawshank', + }, + 'authentication information updated after name change' +); + + +$ldap->delete( 'cn=Brooks Hatlen,ou=Convicts,o=shawshank' ); +$ldap->delete( 'cn=Brooks Hatley,ou=Convicts,o=shawshank' ); + $session->setting->set('automaticLDAPRegistration', 0); From c4ab124380836a60fd9b06c94a9f81db3137f52e Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 16:30:51 -0800 Subject: [PATCH 19/20] i18n the Add Photo save button --- lib/WebGUI/Asset/File/GalleryFile/Photo.pm | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/WebGUI/Asset/File/GalleryFile/Photo.pm b/lib/WebGUI/Asset/File/GalleryFile/Photo.pm index d09cfb6fd..9547b4784 100644 --- a/lib/WebGUI/Asset/File/GalleryFile/Photo.pm +++ b/lib/WebGUI/Asset/File/GalleryFile/Photo.pm @@ -478,10 +478,12 @@ This page is only available to those who can edit this Photo. sub www_edit { my $self = shift; my $session = $self->session; - my $form = $self->session->form; + my $form = $session->form; - return $self->session->privilege->insufficient unless $self->canEdit; - return $self->session->privilege->locked unless $self->canEditIfLocked; + return $session->privilege->insufficient unless $self->canEdit; + return $session->privilege->locked unless $self->canEditIfLocked; + + my $i18n = WebGUI::International->new($session, 'WebGUI'); # Prepare the template variables # Cannot get all template vars since they require a storage location, doesn't work for @@ -539,7 +541,7 @@ sub www_edit { $var->{ form_submit } = WebGUI::Form::submit( $session, { name => "submit", - value => "Save", + value => $i18n->get('save'), }); $var->{ form_title } From 1f49737ae7a5f2e8818eb0046ecf27e58b50a548 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 13 Nov 2009 17:16:56 -0800 Subject: [PATCH 20/20] Allow other users to save photos into another user's album. Fixes bug #11228. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Asset/File/GalleryFile.pm | 13 +++++++++++++ lib/WebGUI/Asset/Wobject/GalleryAlbum.pm | 5 +---- t/Asset/Wobject/GalleryAlbum/permission.t | 8 +------- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 56035044f..47f8f1994 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -15,6 +15,7 @@ - fixed #11216: LDAP Connections status incorrect - fixed #11229: ProgressBar throws errors on some messages. - fixed #11217: LDAP authentication fails if user DN changes + - fixed #11228: Gallery image upload to other users folder permission denied 7.8.4 - Fixed a compatibility problem between WRE and new Spectre code. diff --git a/lib/WebGUI/Asset/File/GalleryFile.pm b/lib/WebGUI/Asset/File/GalleryFile.pm index 210bccb25..c640dcf45 100644 --- a/lib/WebGUI/Asset/File/GalleryFile.pm +++ b/lib/WebGUI/Asset/File/GalleryFile.pm @@ -738,6 +738,19 @@ sub update { } +#---------------------------------------------------------------------------- + +=head2 validParent ( ) + +Override validParent to only allow GalleryAlbums to hold GalleryFiles. + +=cut + +sub validParent { + my ($class, $session) = @_; + return $session->asset->isa('WebGUI::Asset::Wobject::GalleryAlbum'); +} + #---------------------------------------------------------------------------- =head2 view ( ) diff --git a/lib/WebGUI/Asset/Wobject/GalleryAlbum.pm b/lib/WebGUI/Asset/Wobject/GalleryAlbum.pm index d339d2cba..79937eb06 100644 --- a/lib/WebGUI/Asset/Wobject/GalleryAlbum.pm +++ b/lib/WebGUI/Asset/Wobject/GalleryAlbum.pm @@ -306,10 +306,7 @@ sub canEdit { my $form = $self->session->form; # Handle adding a photo - if ( $form->get("func") eq "add" ) { - return $self->canAddFile; - } - elsif ( $form->get("func") eq "editSave" && $form->get("className") eq __PACKAGE__ ) { + if ( $form->get("func") eq "add" || $form->get("func") eq "editSave" ) { return $self->canAddFile; } else { diff --git a/t/Asset/Wobject/GalleryAlbum/permission.t b/t/Asset/Wobject/GalleryAlbum/permission.t index 2ae6a5750..86a0dcd98 100644 --- a/t/Asset/Wobject/GalleryAlbum/permission.t +++ b/t/Asset/Wobject/GalleryAlbum/permission.t @@ -32,6 +32,7 @@ WebGUI::Test->usersToDelete($user{'2'}); my $versionTag = WebGUI::VersionTag->getWorking($session); $versionTag->set({name=>"Album Test"}); +addToCleanup($versionTag); my $gallery = $node->addChild({ className => "WebGUI::Asset::Wobject::Gallery", @@ -104,10 +105,3 @@ $maker->prepare({ fail => [ 1, ], }); $maker->run; - -#---------------------------------------------------------------------------- -# Cleanup -END { - $versionTag->rollback(); -} -