NTR - Clean up code

shopware5 · Mar 19, 2019 · 189c85b · 189c85b
1 parent 1ce7bfe
commit 189c85b
Show file tree

Hide file tree

Showing 16 changed files with 179 additions and 138 deletions.
diff --git a/Controllers/Frontend/PaypalUnified.php b/Controllers/Frontend/PaypalUnified.php
@@ -83,7 +83,6 @@ public function indexAction()
     public function gatewayAction()
     {
         $orderData = $this->get('session')->get('sOrderVariables');
-        $userData = $orderData['sUserData'];
 
         if ($orderData === null) {
             $this->handleError(ErrorCodes::NO_ORDER_TO_PROCESS);
@@ -97,6 +96,8 @@ public function gatewayAction()
             return;
         }
 
+        $userData = $orderData['sUserData'];
+
         try {
             //Query all information
             $basketData = $orderData['sBasket'];
@@ -147,6 +148,8 @@ public function gatewayAction()
         $addressPatch = new PaymentAddressPatch($addressService->getShippingAddress($userData));
         $payerInfoPatch = new PayerInfoPatch($addressService->getPayerInfo($userData));
 
+        $useInContext = (bool) $this->Request()->getParam('useInContext', false);
+
         try {
             $this->paymentResource->patch($responseStruct->getId(), [
                 $addressPatch,
@@ -157,7 +160,7 @@ public function gatewayAction()
              * The field addressValidation gets checked via JavaScript to ensure the redirect to the right error page,
              * if the user uses the In-Context mode.
              */
-            if ($this->Request()->getParam('useInContext')) {
+            if ($useInContext) {
                 $this->Front()->Plugins()->Json()->setRenderer();
 
                 $this->View()->assign('addressValidation', false);
@@ -170,7 +173,7 @@ public function gatewayAction()
             return;
         }
 
-        if ($this->Request()->getParam('useInContext')) {
+        if ($useInContext) {
             $this->Front()->Plugins()->Json()->setRenderer();
 
             $this->View()->assign('paymentId', $responseStruct->getId());
@@ -259,21 +262,21 @@ public function returnAction()
 
             /** @var OrderDataService $orderDataService */
             $orderDataService = $this->get('paypal_unified.order_data_service');
-            /** @var RelatedResource $responseSale */
-            $responseSale = $response->getTransactions()->getRelatedResources()->getResources()[0];
+            /** @var RelatedResource $relatedResource */
+            $relatedResource = $response->getTransactions()->getRelatedResources()->getResources()[0];
 
             //Use TXN-ID instead of the PaymentId
-            $saleId = $responseSale->getId();
-            if (!$orderDataService->applyTransactionId($orderNumber, $saleId)) {
+            $relatedResourceId = $relatedResource->getId();
+            if (!$orderDataService->applyTransactionId($orderNumber, $relatedResourceId)) {
                 $this->handleError(ErrorCodes::NO_ORDER_TO_PROCESS);
 
                 return;
             }
 
             // apply the payment status if its completed by PayPal
-            $paymentState = $responseSale->getState();
+            $paymentState = $relatedResource->getState();
             if ($paymentState === PaymentStatus::PAYMENT_COMPLETED) {
-                $this->savePaymentStatus($saleId, $paymentId, PaymentStatus::PAYMENT_STATUS_APPROVED);
+                $this->savePaymentStatus($relatedResourceId, $paymentId, PaymentStatus::PAYMENT_STATUS_APPROVED);
                 $orderDataService->setClearedDate($orderNumber);
             }
 

diff --git a/Controllers/Frontend/PaypalUnifiedWebhook.php b/Controllers/Frontend/PaypalUnifiedWebhook.php
@@ -46,11 +46,13 @@ public function preDispatch()
         $this->webhookService = $this->get('paypal_unified.webhook_service');
         $this->logger = $this->get('paypal_unified.logger_service');
 
+        $modelManager = $this->get('models');
+
         $this->webhookService->registerWebhooks([
-            new SaleComplete($this->logger, $this->get('models')),
-            new SaleDenied($this->logger, $this->get('models')),
-            new SaleRefunded($this->logger, $this->get('models')),
-            new AuthorizationVoided($this->logger, $this->get('models')),
+            new SaleComplete($this->logger, $modelManager),
+            new SaleDenied($this->logger, $modelManager),
+            new SaleRefunded($this->logger, $modelManager),
+            new AuthorizationVoided($this->logger, $modelManager),
         ]);
     }
 

diff --git a/PayPalBundle/Structs/Payment.php b/PayPalBundle/Structs/Payment.php
@@ -44,7 +44,7 @@ class Payment
     private $links;
 
     /**
-     * @var PaymentInstruction
+     * @var PaymentInstruction|null
      */
     private $paymentInstruction;
 
@@ -235,7 +235,7 @@ public function setUpdateTime($updateTime)
     }
 
     /**
-     * @return PaymentInstruction
+     * @return PaymentInstruction|null
      */
     public function getPaymentInstruction()
     {
@@ -300,7 +300,9 @@ public static function fromArray(array $data = [])
             $result->setPaymentInstruction(PaymentInstruction::fromArray($data['payment_instruction']));
         }
 
-        $result->setPayer(Payer::fromArray($data['payer']));
+        if (array_key_exists('payer', $data)) {
+            $result->setPayer(Payer::fromArray($data['payer']));
+        }
 
         $links = [];
         foreach ($data['links'] as $link) {
@@ -310,7 +312,7 @@ public static function fromArray(array $data = [])
 
         $result->setRedirectUrls(RedirectUrls::fromArray($data['redirect_urls']));
 
-        if ($data['credit_financing_offered']) {
+        if (array_key_exists('credit_financing_offered', $data)) {
             $result->setCreditFinancingOffered(Credit::fromArray($data['credit_financing_offered']));
         }
 

diff --git a/PayPalBundle/Structs/Payment/Transactions/ItemList/ShippingAddress.php b/PayPalBundle/Structs/Payment/Transactions/ItemList/ShippingAddress.php
@@ -45,7 +45,7 @@ public function toArray()
     }
 
     /**
-     * {@inheritdoc}
+     * @return ShippingAddress
      */
     public static function fromArray(array $data = null)
     {

diff --git a/Resources/views/backend/paypal_unified_settings/view/tabs/general.js b/Resources/views/backend/paypal_unified_settings/view/tabs/general.js
@@ -234,7 +234,7 @@ Ext.define('Shopware.apps.PaypalUnifiedSettings.view.tabs.General', {
                     inputValue: true,
                     uncheckedValue: false,
                     fieldLabel: '{s name="fieldset/behaviour/advertiseReturns"}Free returns{/s}',
-                    boxLabel: '{s name="fieldset/behaviour/advertiseReturns/boxLabel"}Enablte to advertise free returns via PayPal. More information <a href="https://www.paypal.com/de/webapps/mpp/returns-on-paypal" title="PayPal returns" target="_blank">here</a>{/s}',
+                    boxLabel: '{s name="fieldset/behaviour/advertiseReturns/boxLabel"}Enable to advertise free returns via PayPal. More information <a href="https://www.paypal.com/de/webapps/mpp/returns-on-paypal" title="PayPal returns" target="_blank">here</a>{/s}',
                     helpText: '{s name="fieldset/behaviour/advertiseReturns/helpText"}If you already offer free returns, the use of the program is excluded.{/s}'
                 }
             ]

diff --git a/Subscriber/Installments.php b/Subscriber/Installments.php
@@ -113,13 +113,13 @@ public function onPostDispatchDetail(ActionEventArgs $args)
             return;
         }
 
-        /** @var GeneralSettingsModel $generalSettings */
+        /** @var GeneralSettingsModel|null $generalSettings */
         $generalSettings = $this->settingsService->getSettings();
         if (!$generalSettings || !$generalSettings->getActive()) {
             return;
         }
 
-        /** @var InstallmentsSettingsModel $installmentsSettings */
+        /** @var InstallmentsSettingsModel|null $installmentsSettings */
         $installmentsSettings = $this->settingsService->getSettings(null, SettingsTable::INSTALLMENTS);
         if (!$installmentsSettings || !$installmentsSettings->getActive()) {
             return;
@@ -162,13 +162,13 @@ public function onPostDispatchCheckout(ActionEventArgs $args)
             return;
         }
 
-        /** @var GeneralSettingsModel $generalSettings */
+        /** @var GeneralSettingsModel|null $generalSettings */
         $generalSettings = $this->settingsService->getSettings();
         if (!$generalSettings || !$generalSettings->getActive()) {
             return;
         }
 
-        /** @var InstallmentsSettingsModel $installmentsSettings */
+        /** @var InstallmentsSettingsModel|null $installmentsSettings */
         $installmentsSettings = $this->settingsService->getSettings(null, SettingsTable::INSTALLMENTS);
         if (!$installmentsSettings || !$installmentsSettings->getActive()) {
             return;

diff --git a/Tests/Functional/Components/ExpressCheckout/CustomerServiceTest.php b/Tests/Functional/Components/ExpressCheckout/CustomerServiceTest.php
@@ -78,7 +78,7 @@ public function test_createNewCustomer()
         static::assertEquals('Shopware', $user['firstname']);
         static::assertEquals('PHPUnit', $user['lastname']);
 
-        static::assertNotNull(Shopware()->Container()->get('session')->offsetGet('sUserId'));
+        static::assertNotNull(Shopware()->Container()->get('session')->get('sUserId'));
     }
 
     private function getUserByMail($mail = '[email protected]')

diff --git a/Tests/Functional/Subscriber/Documents/InstallmentsSubscriberTest.php b/Tests/Functional/Subscriber/Documents/InstallmentsSubscriberTest.php
@@ -9,11 +9,12 @@
 namespace SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents;
 
 use Doctrine\DBAL\Connection;
-use Enlight_Class;
-use Shopware_Models_Document_Order;
 use SwagPaymentPayPalUnified\Subscriber\Documents\Installments;
 use SwagPaymentPayPalUnified\Tests\Functional\DatabaseTestCaseTrait;
 use SwagPaymentPayPalUnified\Tests\Functional\PayPalUnifiedPaymentIdTrait;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithCorrectPaymentId;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithoutSubject;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithWrongPaymentId;
 
 class InstallmentsSubscriberTest extends \PHPUnit_Framework_TestCase
 {
@@ -107,108 +108,3 @@ private function updateOrderPaymentId($orderId, $paymentId)
         ]);
     }
 }
-
-class HookArgsWithoutSubject extends \Enlight_Hook_HookArgs
-{
-    /**
-     * @param bool $isShopware55
-     */
-    public function __construct($isShopware55 = false)
-    {
-        if ($isShopware55) {
-            parent::__construct(new \stdClass(), '');
-        }
-    }
-
-    public function getSubject()
-    {
-        return null;
-    }
-}
-
-class HookArgsWithWrongPaymentId extends \Enlight_Hook_HookArgs
-{
-    /**
-     * @param bool $isShopware55
-     */
-    public function __construct($isShopware55 = false)
-    {
-        if ($isShopware55) {
-            parent::__construct(new \stdClass(), '');
-        }
-    }
-
-    /**
-     * @return Enlight_Class
-     */
-    public function getSubject()
-    {
-        $subject = Enlight_Class::Instance(\Shopware_Components_Document::class);
-
-        $subject->_order = new Shopware_Models_Document_Order(15);
-
-        return $subject;
-    }
-}
-
-class HookArgsWithCorrectPaymentId extends \Enlight_Hook_HookArgs
-{
-    /**
-     * @var \Smarty_Data
-     */
-    private $_view;
-
-    /**
-     * @var \Enlight_Template_Manager
-     */
-    private $_template;
-
-    /**
-     * @param bool $isShopware55
-     */
-    public function __construct($isShopware55 = false)
-    {
-        if ($isShopware55) {
-            parent::__construct(new \stdClass(), '');
-        }
-    }
-
-    /**
-     * @return Enlight_Class
-     */
-    public function getSubject()
-    {
-        $subject = Enlight_Class::Instance(\Shopware_Components_Document::class);
-
-        $subject->_order = new Shopware_Models_Document_Order(15);
-        $view = new \Smarty_Data();
-        $view->assign('Order', [
-            '_payment' => [
-                'description' => 'PayPal',
-            ],
-        ]);
-        $subject->_view = $view;
-        $subject->_template = new \Enlight_Template_Manager();
-
-        $this->_view = $subject->_view;
-        $this->_template = $subject->_template;
-
-        return $subject;
-    }
-
-    /**
-     * @return \Smarty_Data
-     */
-    public function getView()
-    {
-        return $this->_view;
-    }
-
-    /**
-     * @return \Enlight_Template_Manager
-     */
-    public function getTemplate()
-    {
-        return $this->_template;
-    }
-}
diff --git a/Tests/Functional/Subscriber/Documents/InvoiceSubscriberTest.php b/Tests/Functional/Subscriber/Documents/InvoiceSubscriberTest.php
@@ -14,6 +14,9 @@
 use SwagPaymentPayPalUnified\Subscriber\Documents\Invoice;
 use SwagPaymentPayPalUnified\Tests\Functional\DatabaseTestCaseTrait;
 use SwagPaymentPayPalUnified\Tests\Functional\PayPalUnifiedPaymentIdTrait;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithCorrectPaymentId;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithoutSubject;
+use SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock\HookArgsWithWrongPaymentId;
 
 class InvoiceSubscriberTest extends \PHPUnit_Framework_TestCase
 {

diff --git a/Tests/Functional/Subscriber/Documents/Mock/HookArgsWithCorrectPaymentId.php b/Tests/Functional/Subscriber/Documents/Mock/HookArgsWithCorrectPaymentId.php
@@ -0,0 +1,71 @@
+<?php
+/**
+ * (c) shopware AG <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace SwagPaymentPayPalUnified\Tests\Functional\Subscriber\Documents\Mock;
+
+class HookArgsWithCorrectPaymentId extends \Enlight_Hook_HookArgs
+{
+    /**
+     * @var \Smarty_Data
+     */
+    private $_view;
+
+    /**
+     * @var \Enlight_Template_Manager
+     */
+    private $_template;
+
+    /**
+     * @param bool $isShopware55
+     */
+    public function __construct($isShopware55 = false)
+    {
+        if ($isShopware55) {
+            parent::__construct(new \stdClass(), '');
+        }
+    }
+
+    /**
+     * @return \Enlight_Class
+     */
+    public function getSubject()
+    {
+        $subject = \Enlight_Class::Instance(\Shopware_Components_Document::class);
+
+        $subject->_order = new \Shopware_Models_Document_Order(15);
+        $view = new \Smarty_Data();
+        $view->assign('Order', [
+            '_payment' => [
+                'description' => 'PayPal',
+            ],
+        ]);
+        $subject->_view = $view;
+        $subject->_template = new \Enlight_Template_Manager();
+
+        $this->_view = $subject->_view;
+        $this->_template = $subject->_template;
+
+        return $subject;
+    }
+
+    /**
+     * @return \Smarty_Data
+     */
+    public function getView()
+    {
+        return $this->_view;
+    }
+
+    /**
+     * @return \Enlight_Template_Manager
+     */
+    public function getTemplate()
+    {
+        return $this->_template;
+    }
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -45,7 +45,7 @@ public function toArray() @@
         }
         /**
-         * {@inheritdoc}
+         * @return ShippingAddress
          */
         public static function fromArray(array $data = null)
         {
@@ Expand Down @@