From 8525e4b7ee2928f25dcfcf6c442d23d4789fe4ec Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Mon, 4 Apr 2011 14:45:53 -0700 Subject: [PATCH] Don't refund transactionItems unless the transaction is successful, or if the item is already canceled. Also, no not call onCancelRecurring for normal Sku's unless they're really recurring. Fixes bug #12089. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Asset.pm | 15 ++++ lib/WebGUI/Asset/Sku.pm | 4 +- lib/WebGUI/Shop/Transaction.pm | 2 +- lib/WebGUI/Shop/TransactionItem.pm | 9 +- t/Asset/Sku.t | 11 ++- t/Shop/TransactionItem.t | 136 +++++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 t/Shop/TransactionItem.t diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 7bb3ab8fb..7ae8e5f54 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -6,6 +6,7 @@ - fixed #12086: Shop Billing Address Unpopulated - Snippets can now select a template parser (instead of being restricted to the configured default) - fixed #12081: addrees not in addressbook after user change in session + - fixed #12089: Cannot refund item in transaction if the sku no longer exists. 7.10.12 - fixed #12072: Product, related and accessory assets diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm index ad585de59..d3274617c 100644 --- a/lib/WebGUI/Asset.pm +++ b/lib/WebGUI/Asset.pm @@ -2588,6 +2588,21 @@ sub purgeCache { } +#------------------------------------------------------------------- + +=head2 refused ( ) + +Returns an error message to the user, wrapped in the user's style. This is most useful for +handling UI errors. Privilege errors should be still be sent to $session->privilege. + +=cut + +sub refused { + my ($self) = @_; + return $self->{_session}; +} + + #------------------------------------------------------------------- =head2 session ( ) diff --git a/lib/WebGUI/Asset/Sku.pm b/lib/WebGUI/Asset/Sku.pm index be4b02efb..7b0cc9995 100644 --- a/lib/WebGUI/Asset/Sku.pm +++ b/lib/WebGUI/Asset/Sku.pm @@ -564,7 +564,9 @@ The WebGUI::Shop::TransactionItem being refunded. sub onRefund { my ($self, $item) = @_; - $self->onCancelRecurring($item); + if ($self->isRecurring) { + $self->onCancelRecurring($item); + } return undef; } diff --git a/lib/WebGUI/Shop/Transaction.pm b/lib/WebGUI/Shop/Transaction.pm index ba35875b3..b37f253b3 100644 --- a/lib/WebGUI/Shop/Transaction.pm +++ b/lib/WebGUI/Shop/Transaction.pm @@ -1020,8 +1020,8 @@ Refunds a specific item from a transaction and then issues shop credit. sub www_refundItem { my ($class, $session) = @_; return $session->privilege->insufficient unless (WebGUI::Shop::Admin->new($session)->canManage); - my $self = $class->new($session, $session->form->get("transactionId")); my $form = $session->form; + my $self = $class->new($session, $form->get("transactionId")); my $item = eval { $self->getItem($form->get("itemId")) }; if (WebGUI::Error->caught()) { $session->errorHandler->error("Can't get item ".$form->get("itemId")); diff --git a/lib/WebGUI/Shop/TransactionItem.pm b/lib/WebGUI/Shop/TransactionItem.pm index ec6150cc4..ab238a767 100644 --- a/lib/WebGUI/Shop/TransactionItem.pm +++ b/lib/WebGUI/Shop/TransactionItem.pm @@ -145,15 +145,20 @@ sub getSku { =head2 issueCredit ( ) -Returns the money from this item to the user in the form of in-store credit. +Returns the money from this item to the user in the form of in-store credit. Items marked +cancelled cannot be refunded. =cut sub issueCredit { my $self = shift; + return if $self->get('orderStatus') eq 'Cancelled'; + return unless $self->transaction->isSuccessful; my $credit = WebGUI::Shop::Credit->new($self->transaction->session, $self->transaction->get('userId')); $credit->adjust(($self->get('price') * $self->get('quantity')), "Issued credit on sku ".$self->get('assetId')." for transaction item ".$self->getId." on transaction ".$self->transaction->getId); - $self->getSku->onRefund($self); + if (my $sku = eval {$self->getSku}) { + $sku->onRefund($self); + } $self->update({orderStatus=>'Cancelled'}); } diff --git a/t/Asset/Sku.t b/t/Asset/Sku.t index 10420e541..898191611 100644 --- a/t/Asset/Sku.t +++ b/t/Asset/Sku.t @@ -30,7 +30,7 @@ my $session = WebGUI::Test->session; #---------------------------------------------------------------------------- # Tests -plan tests => 21; # Increment this number for each test you create +plan tests => 22; # Increment this number for each test you create #---------------------------------------------------------------------------- # put your tests here @@ -84,4 +84,13 @@ $item->cart->delete; my $loadSku = WebGUI::Asset::Sku->newBySku($session, $sku->get("sku")); is($loadSku->getId, $sku->getId, "newBySku() works."); +{ + ##Check to see that calling onRefund does not call onCancelRecurring for a default, non-recurring sku. + use Monkey::Patch qw/:all/; + my $messageAdded = 0; + my $monkey = patch_object $sku => 'onCancelRecurring' => sub { $messageAdded = 1; }; + $sku->onRefund; + ok !$messageAdded, 'onRefund did not call onCancelRecurring'; +} + 1; diff --git a/t/Shop/TransactionItem.t b/t/Shop/TransactionItem.t new file mode 100644 index 000000000..71bbe663f --- /dev/null +++ b/t/Shop/TransactionItem.t @@ -0,0 +1,136 @@ +# vim:syntax=perl +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2008 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 Test::Exception; +use Test::MockObject::Extends; +use WebGUI::Test; # Must use this before any other WebGUI modules +use WebGUI::Session; +use WebGUI::Shop::Cart; +use WebGUI::Shop::Ship; +use WebGUI::Shop::Transaction; +use WebGUI::Shop::PayDriver::ITransact; +use JSON; +use HTML::Form; + +#---------------------------------------------------------------------------- +# Init +my $session = WebGUI::Test->session; +$session->user({userId => 3}); + + +#---------------------------------------------------------------------------- +# Tests + +plan tests => 8; + +#---------------------------------------------------------------------------- +# figure out if the test can actually run + + +my $e; +my $ship = WebGUI::Shop::Ship->new($session); +my $cart = WebGUI::Shop::Cart->newBySession($session); +WebGUI::Test->addToCleanup($cart); +my $shipper = $ship->getShipper('defaultfreeshipping000'); +my $address = $cart->getAddressBook->addAddress( { + label => 'red', + firstName => 'Ellis Boyd', lastName => 'Redding', + address1 => 'cell block #5', + city => 'Shawshank', state => 'MN', + code => '55555', country => 'United States of America', + phoneNumber => '555.555.5555', email => 'red@shawshank.gov', +} ); +$cart->update({ + billingAddressId => $address->getId, + shippingAddressId => $address->getId, + shipperId => $shipper->getId, +}); + +my $versionTag = WebGUI::VersionTag->getWorking($session); + +my $home = WebGUI::Asset->getDefault($session); + +my $rockHammer = $home->addChild({ + className => 'WebGUI::Asset::Sku::Product', + isShippingRequired => 0, 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 $foreignHammer = $rockHammer->setCollateral('variantsJSON', 'variantId', 'new', + { + shortdesc => '錘', price => 7.00, + varSku => 'foreigh-hammer', weight => 1.0, + quantity => 9999, + } +); + + +$versionTag->commit; +$cart->update({gatewayId => 'gzUxkEZJxREF9JpylOg2zw',}); ##Cash checkout +my $transaction = WebGUI::Shop::Transaction->create($session, { + cart => $cart, + isRecurring => $cart->requiresRecurringPayment, +}); +##Block sending emails and inbox messages on this transation +$transaction = Test::MockObject::Extends->new($transaction); +$transaction->set_true('sendNotifications'); + +WebGUI::Test->addToCleanup($versionTag, $transaction); + +my $credit = WebGUI::Shop::Credit->new($session, '3'); +WebGUI::Test->addToCleanup(sub { $credit->purge }); + +my $hammer2 = $rockHammer->addToCart($rockHammer->getCollateral('variantsJSON', 'variantId', $foreignHammer)); +my $transactionItem = $transaction->addItem({ item => $hammer2 }); + +$transaction->update({isSuccessful => 1}); + +$transactionItem->update({orderStatus => 'Cancelled'}); +$transactionItem->issueCredit; +is $credit->getSum, '0.00', 'issueCredit fails if the item is Cancelled'; + +$transactionItem->update({orderStatus => 'Shipped'}); +$transactionItem->issueCredit; +is $credit->getSum, '7.00', '... succeeds if the item is not Cancelled'; +is $transactionItem->get('orderStatus'), 'Cancelled', '... item status updated to Cancelled'; + +$transaction->update({isSuccessful => 0}); +$transactionItem->update({orderStatus => 'Not Shipped'}); +$transactionItem->issueCredit; +is $credit->getSum, '7.00', 'issueCredit is unchanged when the transaction is not successful'; +is $transactionItem->get('orderStatus'), 'Not Shipped', '... item status unchanged'; + +$transaction->update({isSuccessful => 1}); + +$transactionItem->update({orderStatus => 'Shipped'}); +$rockHammer->purge; +lives_ok { $transactionItem->issueCredit } 'issueCredit does not die if the asset does not exist'; +is $credit->getSum, '14.00', '... credit is still issued'; +is $transactionItem->get('orderStatus'), 'Cancelled', '... item status still updated'; + +#vim:ft=perl