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

@@ -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

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 ( )

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;
 }
 

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"));

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'});
 }
 

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;

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