MAGETWO-18815: code review #7

Author: Cristian Partica

app/code/Magento/Tax/Block/Adminhtml/Rate/Form.php

@@ -294,39 +294,4 @@ protected function _prepareForm()
 
         return parent::_prepareForm();
     }
-
-    /**
-     * Extract tax rate data in a format which is
-     *
-     * @param \Magento\Tax\Api\Data\TaxRateInterface $taxRate
-     * @return array
-     */
-    protected function extractTaxRateData($taxRate)
-    {
-        $formData = [
-            'tax_calculation_rate_id' => $taxRate->getId(),
-            'tax_country_id' => $taxRate->getTaxCountryId(),
-            'tax_region_id' => $taxRate->getTaxRegionId(),
-            'tax_postcode' => $taxRate->getTaxPostcode(),
-            'code' => $taxRate->getCode(),
-            'rate' => $taxRate->getRate(),
-            'zip_is_range' => false,
-        ];
-
-        if ($taxRate->getZipFrom() && $taxRate->getZipTo()) {
-            $formData['zip_is_range'] = true;
-            $formData['zip_from'] = $taxRate->getZipFrom();
-            $formData['zip_to'] = $taxRate->getZipTo();
-        }
-
-        if ($taxRate->getTitles()) {
-            $titleData = [];
-            foreach ($taxRate->getTitles() as $title) {
-                $titleData[] = [$title->getStoreId() => $title->getValue()];
-            }
-            $formData['title'] = $titleData;
-        }
-
-        return $formData;
-    }
 }

app/code/Magento/Tax/Controller/Adminhtml/Rate/AjaxLoad.php

@@ -12,7 +12,7 @@
 class AjaxLoad extends \Magento\Tax\Controller\Adminhtml\Rate
 {
     /**
-     * Show Edit Form
+     * Json needed for the Ajax Edit Form
      *
      * @return void
      */

app/code/Magento/Tax/Model/Calculation/Rate/Converter.php

@@ -15,12 +15,12 @@ class Converter
     /**
      * @var \Magento\Tax\Api\Data\TaxRateInterfaceFactory
      */
-    protected $_taxRateDataObjectFactory;
+    protected $taxRateDataObjectFactory;
 
     /**
      * @var \Magento\Tax\Api\Data\TaxRateTitleInterfaceFactory
      */
-    protected $_taxRateTitleDataObjectFactory;
+    protected $taxRateTitleDataObjectFactory;
 
     /**
      * @param \Magento\Tax\Api\Data\TaxRateInterfaceFactory $taxRateDataObjectFactory
@@ -30,8 +30,8 @@ public function __construct(
         \Magento\Tax\Api\Data\TaxRateInterfaceFactory $taxRateDataObjectFactory,
         \Magento\Tax\Api\Data\TaxRateTitleInterfaceFactory $taxRateTitleDataObjectFactory
     ) {
-        $this->_taxRateDataObjectFactory = $taxRateDataObjectFactory;
-        $this->_taxRateTitleDataObjectFactory = $taxRateTitleDataObjectFactory;
+        $this->taxRateDataObjectFactory = $taxRateDataObjectFactory;
+        $this->taxRateTitleDataObjectFactory = $taxRateTitleDataObjectFactory;
     }
     /**
      * Convert a tax rate data object to an array of associated titles
@@ -112,7 +112,7 @@ public function createArrayFromServiceObject(\Magento\Tax\Api\Data\TaxRateInterf
      */
     public function populateTaxRateData($formData)
     {
-        $taxRate = $this->_taxRateDataObjectFactory->create();
+        $taxRate = $this->taxRateDataObjectFactory->create();
         $taxRate->setId($this->extractFormData($formData, 'tax_calculation_rate_id'))
             ->setTaxCountryId($this->extractFormData($formData, 'tax_country_id'))
             ->setTaxRegionId($this->extractFormData($formData, 'tax_region_id'))
@@ -127,7 +127,7 @@ public function populateTaxRateData($formData)
         if (isset($formData['title'])) {
             $titles = [];
             foreach ($formData['title'] as $storeId => $value) {
-                $titles[] = $this->_taxRateTitleDataObjectFactory->create()->setStoreId($storeId)->setValue($value);
+                $titles[] = $this->taxRateTitleDataObjectFactory->create()->setStoreId($storeId)->setValue($value);
             }
             $taxRate->setTitles($titles);
         }

app/code/Magento/Tax/Test/Unit/Controller/Adminhtml/Rate/AjaxLoadTest.php

@@ -16,34 +16,34 @@ class AjaxLoadTest extends \PHPUnit_Framework_TestCase
     /**
      * @var \Magento\Framework\App\Request\Http
      */
-    private $_request;
+    private $request;
 
     /**
      * @var \Magento\Framework\App\Response\Http
      */
-    private $_resultFactory;
+    private $resultFactory;
 
     /**
      * @var \Magento\Tax\Model\Calculation\RateRepository
      */
-    private $_taxRateRepository;
+    private $taxRateRepository;
 
     /*
      * test setup
      */
     public function setUp()
     {
-        $this->_request = $this->getMockBuilder('\Magento\Framework\App\Request\Http')
+        $this->request = $this->getMockBuilder('\Magento\Framework\App\Request\Http')
             ->disableOriginalConstructor()
             ->setMethods(['getParam'])
             ->getMock();
 
-        $this->_resultFactory = $this->getMockBuilder('Magento\Framework\Controller\ResultFactory')
+        $this->resultFactory = $this->getMockBuilder('Magento\Framework\Controller\ResultFactory')
             ->disableOriginalConstructor()
             ->setMethods(['create'])
             ->getMock();
 
-        $this->_taxRateRepository = $this->getMockBuilder('\Magento\Tax\Model\Calculation\RateRepository')
+        $this->taxRateRepository = $this->getMockBuilder('\Magento\Tax\Model\Calculation\RateRepository')
             ->disableOriginalConstructor()
             ->setMethods(['get'])
             ->getMock();
@@ -86,11 +86,11 @@ public function testExecute()
             ]
         );
 
-        $this->_request->expects($this->any())
+        $this->request->expects($this->any())
             ->method('getParam')
             ->will($this->returnValue($taxRateId));
 
-        $this->_taxRateRepository->expects($this->any())
+        $this->taxRateRepository->expects($this->any())
             ->method('get')
             ->with($taxRateId)
             ->will($this->returnValue($rateMock));
@@ -115,18 +115,18 @@ public function testExecute()
                 $returnArray,
             ]);
 
-        $this->_resultFactory->expects($this->any())
+        $this->resultFactory->expects($this->any())
             ->method('create')
             ->with(\Magento\Framework\Controller\ResultFactory::TYPE_JSON)
             ->willReturn($jsonObject);
 
         $notification = $objectManager->getObject(
             'Magento\Tax\Controller\Adminhtml\Rate\AjaxLoad',
             [
-                'taxRateRepository' => $this->_taxRateRepository,
+                'taxRateRepository' => $this->taxRateRepository,
                 'taxRateConverter' => $taxRateConverter,
-                'request' => $this->_request,
-                'resultFactory' => $this->_resultFactory,
+                'request' => $this->request,
+                'resultFactory' => $this->resultFactory,
             ]
         );
 
@@ -147,11 +147,11 @@ public function testExecuteLocalizedException()
 
         $objectManager = new ObjectManager($this);
 
-        $this->_request->expects($this->any())
+        $this->request->expects($this->any())
             ->method('getParam')
             ->will($this->returnValue($taxRateId));
 
-        $this->_taxRateRepository->expects($this->any())
+        $this->taxRateRepository->expects($this->any())
             ->method('get')
             ->with($taxRateId)
             ->willThrowException($noSuchEntityEx);
@@ -168,17 +168,17 @@ public function testExecuteLocalizedException()
                 'error_message' => $exceptionMessage,
             ]);
 
-        $this->_resultFactory->expects($this->any())
+        $this->resultFactory->expects($this->any())
             ->method('create')
             ->with(\Magento\Framework\Controller\ResultFactory::TYPE_JSON)
             ->willReturn($jsonObject);
 
         $notification = $objectManager->getObject(
             'Magento\Tax\Controller\Adminhtml\Rate\AjaxLoad',
             [
-                'taxRateRepository' => $this->_taxRateRepository,
-                'request' => $this->_request,
-                'resultFactory' => $this->_resultFactory,
+                'taxRateRepository' => $this->taxRateRepository,
+                'request' => $this->request,
+                'resultFactory' => $this->resultFactory,
             ]
         );
 
@@ -197,11 +197,11 @@ public function testExecuteException()
 
         $objectManager = new ObjectManager($this);
 
-        $this->_request->expects($this->any())
+        $this->request->expects($this->any())
             ->method('getParam')
             ->will($this->returnValue($taxRateId));
 
-        $this->_taxRateRepository->expects($this->any())
+        $this->taxRateRepository->expects($this->any())
             ->method('get')
             ->with($taxRateId)
             ->willThrowException($noSuchEntityEx);
@@ -218,17 +218,17 @@ public function testExecuteException()
                 'error_message' => $exceptionMessage,
             ]);
 
-        $this->_resultFactory->expects($this->any())
+        $this->resultFactory->expects($this->any())
             ->method('create')
             ->with(\Magento\Framework\Controller\ResultFactory::TYPE_JSON)
             ->willReturn($jsonObject);
 
         $notification = $objectManager->getObject(
             'Magento\Tax\Controller\Adminhtml\Rate\AjaxLoad',
             [
-                'taxRateRepository' => $this->_taxRateRepository,
-                'request' => $this->_request,
-                'resultFactory' => $this->_resultFactory,
+                'taxRateRepository' => $this->taxRateRepository,
+                'request' => $this->request,
+                'resultFactory' => $this->resultFactory,
             ]
         );