Feature/421 43 refund to store credit

CHANGELOG.md

@@ -1,3 +1,14 @@
+### 0.4.0 (2017-03-14)
+
+* Add code to create new database tables dedicated to storing AvaTax data
+* Add code to migrate existing data from AvaTax columns on sales_invoice and sales_creditmemo tables to new tables
+* Refactor code to store AvaTax data in new tables instead of attaching to entities
+* Previous versions of this extension added two fields to the native Magento invoice and credit memo tables. When this extension
+  changed the values of these two fields, it would save the invoice/credit memo. This caused multiple issues (see [#24](https://github.com/classyllama/ClassyLlama_AvaTax/issues/24), [#29](https://github.com/classyllama/ClassyLlama_AvaTax/issues/29), [#36](https://github.com/classyllama/ClassyLlama_AvaTax/issues/36), [#40](https://github.com/classyllama/ClassyLlama_AvaTax/issues/40), and [#47](https://github.com/classyllama/ClassyLlama_AvaTax/issues/47)). 
+  Rather than continuing to try and fix the underlying Magento issues that were triggered by saving these objects, we have moved
+  the fields to separate database tables (avatax_sales_creditmemo and avatax_sales_invoice). This release includes that refactor and 
+  fixes issue [#47](https://github.com/classyllama/ClassyLlama_AvaTax/issues/47). Specific changes:
+
 ### 0.3.5 (2017-03-17)
 
 * Fix issue where gift card purchases are taxed [#53](https://github.com/classyllama/ClassyLlama_AvaTax/issues/53)

Framework/AppInterface.php

@@ -20,5 +20,5 @@ interface AppInterface
     /**
      * If this is updated it must also be updated in composer.json
      */
-    const APP_VERSION = '0.3.5';
+    const APP_VERSION = '0.4.0';
 }

Model/CreditMemo.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * @category    ClassyLlama
+ * @copyright   Copyright (c) 2017 Classy Llama Studios, LLC
+ */
+
+namespace ClassyLlama\AvaTax\Model;
+
+/**
+ * CreditMemo
+ *
+ * @method int getParentId() getParentId()
+ * @method string getIsUnbalanced() getIsUnbalanced()
+ * @method float getBaseAvataxTaxAmount() getBaseAvataxTaxAmount()
+ * @method CreditMemo setParentId() setParentId(int $parentId)
+ * @method CreditMemo setIsUnbalanced() setIsUnbalanced(string $isUnbalanced)
+ * @method CreditMemo setBaseAvataxTaxAmount() setBaseAvataxTaxAmount(float $baseAvataxTaxAmount)
+ */
+class CreditMemo
+    extends \Magento\Framework\Model\AbstractModel
+    implements \Magento\Framework\DataObject\IdentityInterface
+{
+    const CACHE_TAG = 'classyllama_avatax_creditMemo';
+
+    protected function _construct()
+    {
+        $this->_init('ClassyLlama\AvaTax\Model\ResourceModel\CreditMemo');
+    }
+
+    /**
+     * @return array
+     */
+    public function getIdentities()
+    {
+        return [self::CACHE_TAG . '_' . $this->getId()];
+    }
+}

Model/Invoice.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * @category    ClassyLlama
+ * @copyright   Copyright (c) 2017 Classy Llama Studios, LLC
+ */
+
+namespace ClassyLlama\AvaTax\Model;
+
+/**
+ * Invoice
+ *
+ * @method int getParentId() getParentId()
+ * @method string getIsUnbalanced() getIsUnbalanced()
+ * @method float getBaseAvataxTaxAmount() getBaseAvataxTaxAmount()
+ * @method Invoice setParentId() setParentId(int $parentId)
+ * @method Invoice setIsUnbalanced() setIsUnbalanced(string $isUnbalanced)
+ * @method Invoice setBaseAvataxTaxAmount() setBaseAvataxTaxAmount(float $baseAvataxTaxAmount)
+ */
+class Invoice
+    extends \Magento\Framework\Model\AbstractModel
+    implements \Magento\Framework\DataObject\IdentityInterface
+{
+    const CACHE_TAG = 'classyllama_avatax_invoice';
+
+    protected function _construct()
+    {
+        $this->_init('ClassyLlama\AvaTax\Model\ResourceModel\Invoice');
+    }
+
+    /**
+     * @return array
+     */
+    public function getIdentities()
+    {
+        return [self::CACHE_TAG . '_' . $this->getId()];
+    }
+}

Model/Queue/Processing.php

@@ -20,6 +20,10 @@
 use ClassyLlama\AvaTax\Model\Queue;
 use ClassyLlama\AvaTax\Framework\Interaction\Tax\Get;
 use ClassyLlama\AvaTax\Api\Data\GetTaxResponseInterface;
+use ClassyLlama\AvaTax\Model\Invoice;
+use ClassyLlama\AvaTax\Model\InvoiceFactory;
+use ClassyLlama\AvaTax\Model\CreditMemo;
+use ClassyLlama\AvaTax\Model\CreditMemoFactory;
 use Magento\Framework\Stdlib\DateTime\DateTime;
 use Magento\Sales\Api\InvoiceRepositoryInterface;
 use Magento\Sales\Api\CreditmemoRepositoryInterface;
@@ -32,6 +36,8 @@
 use Magento\Sales\Api\Data\CreditmemoExtensionFactory;
 use Magento\Eav\Model\Config as EavConfig;
 use Magento\Framework\Exception\NoSuchEntityException;
+use ClassyLlama\AvaTax\Model\ResourceModel\CreditMemo as CreditMemoResourceModel;
+use ClassyLlama\AvaTax\Model\ResourceModel\Invoice as InvoiceResourceModel;
 
 /**
  * Queue Processing
@@ -64,9 +70,9 @@ class Processing
     protected $invoiceRepository;
 
     /**
-     * @var InvoiceRepositoryInterface
+     * @var CreditmemoRepositoryInterface
      */
-    protected $creditmemoRepository;
+    protected $creditMemoRepository;
 
     /**
      * @var OrderRepositoryInterface
@@ -91,27 +97,39 @@ class Processing
     /**
      * @var CreditmemoExtensionFactory
      */
-    protected $creditmemoExtensionFactory;
+    protected $creditMemoExtensionFactory;
 
     /**
      * @var EavConfig
      */
     protected $eavConfig;
 
+    /**
+     * @var InvoiceFactory
+     */
+    protected $avataxInvoiceFactory;
+
+    /**
+     * @var CreditMemoFactory
+     */
+    protected $avataxCreditMemoFactory;
+
     /**
      * Processing constructor.
      * @param AvaTaxLogger $avaTaxLogger
      * @param Config $avaTaxConfig
      * @param Get $interactionGetTax
      * @param DateTime $dateTime
      * @param InvoiceRepositoryInterface $invoiceRepository
-     * @param CreditmemoRepositoryInterface $creditmemoRepository
+     * @param CreditMemoRepositoryInterface $creditmemoRepository
      * @param OrderRepositoryInterface $orderRepository
      * @param OrderManagementInterface $orderManagement
      * @param OrderStatusHistoryInterfaceFactory $orderStatusHistoryFactory
      * @param InvoiceExtensionFactory $invoiceExtensionFactory
      * @param CreditmemoExtensionFactory $creditmemoExtensionFactory
      * @param EavConfig $eavConfig
+     * @param InvoiceFactory $avataxInvoiceFactory
+     * @param CreditMemoFactory $avataxCreditMemoFactory
      */
     public function __construct(
         AvaTaxLogger $avaTaxLogger,
@@ -125,7 +143,9 @@ public function __construct(
         OrderStatusHistoryInterfaceFactory $orderStatusHistoryFactory,
         InvoiceExtensionFactory $invoiceExtensionFactory,
         CreditmemoExtensionFactory $creditmemoExtensionFactory,
-        EavConfig $eavConfig
+        EavConfig $eavConfig,
+        InvoiceFactory $avataxInvoiceFactory,
+        CreditMemoFactory $avataxCreditMemoFactory
     ) {
         $this->avaTaxLogger = $avaTaxLogger;
         $this->avaTaxConfig = $avaTaxConfig;
@@ -139,6 +159,8 @@ public function __construct(
         $this->invoiceExtensionFactory = $invoiceExtensionFactory;
         $this->creditmemoExtensionFactory = $creditmemoExtensionFactory;
         $this->eavConfig = $eavConfig;
+        $this->avataxInvoiceFactory = $avataxInvoiceFactory;
+        $this->avataxCreditMemoFactory = $avataxCreditMemoFactory;
     }
 
     /**
@@ -160,8 +182,8 @@ public function execute(Queue $queue)
         // Process entity with AvaTax
         $processSalesResponse = $this->processWithAvaTax($queue, $entity);
 
-        // update invoice with additional fields
-        $this->updateAdditionalEntityAttributes($entity, $processSalesResponse);
+        // Create AvaTax record
+        $this->saveAvaTaxRecord($entity, $processSalesResponse);
 
         // Update the queue record status
         // and add comment to order
@@ -373,119 +395,102 @@ protected function processWithAvaTax(Queue $queue, $entity)
     /**
      * @param \Magento\Sales\Api\Data\InvoiceInterface|\Magento\Sales\Api\Data\CreditmemoInterface $entity
      * @param \ClassyLlama\AvaTax\Api\Data\GetTaxResponseInterface $processSalesResponse
+     * @throws \Exception
      */
-    protected function updateAdditionalEntityAttributes(
+    protected function saveAvaTaxRecord(
         $entity,
         GetTaxResponseInterface $processSalesResponse
-    ) {
-        $entityExtension = $entity->getExtensionAttributes();
-        if ($entityExtension == null) {
-            $entityExtension = $this->getEntityExtensionInterface($entity);
-        }
+    )
+    {
+        // Get the associated AvataxEntity record (related to extension attributes) for this entity type
+        $avaTaxRecord = $this->getAvataxEntity($entity);
 
-        // check to see if the AvataxIsUnbalanced is already set on this entity
-        $avataxIsUnbalancedToSave = false;
-        if ($entityExtension->getAvataxIsUnbalanced() === null) {
-            $entityExtension->setAvataxIsUnbalanced($processSalesResponse->getIsUnbalanced());
-            $avataxIsUnbalancedToSave = true;
-        } else {
-            // check to see if any existing value is different from the new value
-            if ($processSalesResponse->getIsUnbalanced() <> $entityExtension->getAvataxIsUnbalanced()) {
-                // Log the warning
-                $this->avaTaxLogger->warning(
-                    __('When processing an entity in the queue there was an existing AvataxIsUnbalanced and ' .
-                    'the new value was different than the old one. The old value was overwritten.'),
-                    [ /* context */
-                        'old_is_unbalanced' => $entityExtension->getAvataxIsUnbalanced(),
-                        'new_is_unbalanced' => $processSalesResponse->getIsUnbalanced(),
-                    ]
-                );
-                $entityExtension->setAvataxIsUnbalanced($processSalesResponse->getIsUnbalanced());
+        if ($avaTaxRecord->getParentId()) {
+            // Record exists, compare existing values to new
+
+            // Check to see if isUnbalanced is already set on this entity
+            $avataxIsUnbalancedToSave = false;
+            if ($avaTaxRecord->getIsUnbalanced() == null) {
+                $avaTaxRecord->setIsUnbalanced($processSalesResponse->getIsUnbalanced());
                 $avataxIsUnbalancedToSave = true;
+            } else {
+                // check to see if any existing value is different from the new value
+                if ($processSalesResponse->getIsUnbalanced() <> $avaTaxRecord->getIsUnbalanced()) {
+                    // Log the warning
+                    $this->avaTaxLogger->warning(
+                        __('When processing an entity in the queue there was an existing AvataxIsUnbalanced and ' .
+                            'the new value was different than the old one. The old value was overwritten.'),
+                        [ /* context */
+                            'old_is_unbalanced' => $avaTaxRecord->getIsUnbalanced(),
+                            'new_is_unbalanced' => $processSalesResponse->getIsUnbalanced(),
+                        ]
+                    );
+                    $avaTaxRecord->setIsUnbalanced($processSalesResponse->getIsUnbalanced());
+                    $avataxIsUnbalancedToSave = true;
+                }
             }
-        }
 
-        // check to see if the BaseAvataxTaxAmount is already set on this entity
-        $baseAvataxTaxAmountToSave = false;
-        if ($entityExtension->getBaseAvataxTaxAmount() === null) {
-            $entityExtension->setBaseAvataxTaxAmount($processSalesResponse->getBaseAvataxTaxAmount());
-            $baseAvataxTaxAmountToSave = true;
-        } else {
-            // check to see if any existing value is different from the new value
-            if ($processSalesResponse->getBaseAvataxTaxAmount() <> $entityExtension->getBaseAvataxTaxAmount()) {
-                // Log the warning
-                $this->avaTaxLogger->warning(
-                    __('When processing an entity in the queue there was an existing BaseAvataxTaxAmount and ' .
-                    'the new value was different than the old one. The old value was overwritten.'),
-                    [ /* context */
-                        'old_base_avatax_tax_amount' => $entityExtension->getBaseAvataxTaxAmount(),
-                        'new_base_avatax_tax_amount' => $processSalesResponse->getBaseAvataxTaxAmount(),
-                    ]
-                );
-                $entityExtension->setBaseAvataxTaxAmount($processSalesResponse->getBaseAvataxTaxAmount());
+            // Check to see if the BaseAvataxTaxAmount is already set on this entity
+            $baseAvataxTaxAmountToSave = false;
+            if ($avaTaxRecord->getBaseAvataxTaxAmount() == null) {
+                $avaTaxRecord->setBaseAvataxTaxAmount($processSalesResponse->getBaseAvataxTaxAmount());
                 $baseAvataxTaxAmountToSave = true;
+            } else {
+                // Check to see if any existing value is different from the new value
+                if ($processSalesResponse->getBaseAvataxTaxAmount() <> $avaTaxRecord->getBaseAvataxTaxAmount()) {
+                    // Log the warning
+                    $this->avaTaxLogger->warning(
+                        __('When processing an entity in the queue there was an existing BaseAvataxTaxAmount and ' .
+                            'the new value was different than the old one. The old value was overwritten.'),
+                        [ /* context */
+                            'old_base_avatax_tax_amount' => $avaTaxRecord->getBaseAvataxTaxAmount(),
+                            'new_base_avatax_tax_amount' => $processSalesResponse->getBaseAvataxTaxAmount(),
+                        ]
+                    );
+                    $avaTaxRecord->setBaseAvataxTaxAmount($processSalesResponse->getBaseAvataxTaxAmount());
+                    $baseAvataxTaxAmountToSave = true;
+                }
             }
+        } else {
+            // No entry exists for entity ID, add data to entry and set store flags to true
+            $avataxIsUnbalancedToSave = true;
+            $baseAvataxTaxAmountToSave = true;
+            $avaTaxRecord->setParentId($entity->getId());
+            $avaTaxRecord->setIsUnbalanced($processSalesResponse->getIsUnbalanced());
+            $avaTaxRecord->setBaseAvataxTaxAmount($processSalesResponse->getBaseAvataxTaxAmount());
         }
 
-        // save the ExtensionAttributes on the entity object
         if ($avataxIsUnbalancedToSave || $baseAvataxTaxAmountToSave) {
-            $entity->setExtensionAttributes($entityExtension);
-
-            // get the repository for this entity type
-            $entityRepository = $this->getEntityRepository($entity);
-
-            // save the entity object using the repository
-            $entityRepository->save($entity);
+            // Save the AvaTax entry
+            $avaTaxRecord->save();
         }
     }
 
     /**
      * @param \Magento\Sales\Api\Data\InvoiceInterface|\Magento\Sales\Api\Data\CreditmemoInterface $entity
-     * @return \Magento\Sales\Api\Data\InvoiceExtension|\Magento\Sales\Api\Data\CreditmemoExtension
+     * @return CreditMemo|Invoice
      * @throws \Exception
      */
-    protected function getEntityExtensionInterface($entity)
+    protected function getAvataxEntity($entity)
     {
         if ($entity instanceof InvoiceInterface) {
-            return $this->invoiceExtensionFactory->create();
-        } elseif ($entity instanceof CreditmemoInterface) {
-            return $this->creditmemoExtensionFactory->create();
-        } else {
-            $message = __('Did not receive a valid entity instance to determine the extension to return');
-            throw new \Exception($message);
-        }
-    }
+            /** @var Invoice $avaTaxRecord */
+            $avaTaxRecord = $this->avataxInvoiceFactory->create();
 
-    /**
-     * @param \Magento\Sales\Api\Data\InvoiceInterface|\Magento\Sales\Api\Data\CreditmemoInterface $entity
-     * @return \Magento\Eav\Model\Entity\Type
-     * @throws \Exception
-     */
-    protected function getEntityType($entity)
-    {
-        if ($entity instanceof InvoiceInterface) {
-            return $this->eavConfig->getEntityType(Queue::ENTITY_TYPE_CODE_INVOICE);
-        } elseif ($entity instanceof CreditmemoInterface) {
-            return $this->eavConfig->getEntityType(Queue::ENTITY_TYPE_CODE_CREDITMEMO);
-        } else {
-            $message = __('Did not receive a valid entity instance to determine the entity type to return');
-            throw new \Exception($message);
-        }
-    }
+            // Load existing AvaTax entry for this entity, if exists
+            $avaTaxRecord->load($entity->getId(), InvoiceResourceModel::PARENT_ID_FIELD_NAME);
 
-    /**
-     * @param \Magento\Sales\Api\Data\InvoiceInterface|\Magento\Sales\Api\Data\CreditmemoInterface $entity
-     * @return \Magento\Sales\Api\InvoiceRepositoryInterface|\Magento\Sales\Api\CreditmemoRepositoryInterface
-     * @throws \Exception
-     */
-    protected function getEntityRepository($entity)
-    {
-        if ($entity instanceof InvoiceInterface) {
-            return $this->invoiceRepository;
+            return $avaTaxRecord;
         } elseif ($entity instanceof CreditmemoInterface) {
-            return $this->creditmemoRepository;
+            /** @var CreditMemo $avaTaxRecord */
+            $avaTaxRecord = $this->avataxCreditMemoFactory->create();
+
+            // Load existing AvaTax entry for this entity, if exists
+            $avaTaxRecord->load($entity->getId(), CreditMemoResourceModel::PARENT_ID_FIELD_NAME);
+
+            return $avaTaxRecord;
         } else {
-            $message = __('Did not receive a valid entity instance to determine the repository type to return');
+            $message = __('Did not receive a valid entity instance to determine the factory type to return');
             throw new \Exception($message);
         }
     }