MAGETWO-59801: [Performance Bug] Tax Rules Form unusable with large # of tax rates

- CR related changes

Author: iivashchenko

app/code/Magento/Tax/Block/Grid/Renderer/Codes.php

@@ -5,6 +5,9 @@
  */
 namespace Magento\Tax\Block\Grid\Renderer;
 
+/**
+ * Provides tax rates codes for each tax rule in the grid.
+  */
 class Codes extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\AbstractRenderer
 {
     /**
@@ -16,6 +19,7 @@ class Codes extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\AbstractR
     public function render(\Magento\Framework\DataObject $row)
     {
         $ratesCodes = $row->getTaxRatesCodes();
+
         return is_array($ratesCodes) ? implode(', ', $ratesCodes) : '';
     }
 }

app/code/Magento/Tax/Controller/Adminhtml/Rule/AjaxLoadRates.php

@@ -13,18 +13,20 @@
 use Magento\Framework\Api\SearchCriteriaBuilder;
 use Magento\Tax\Model\Calculation\Rate;
 
+/**
+ * Class AjaxLoadRates is intended to load existing
+ * Tax rates as options for a select element.
+ */
 class AjaxLoadRates extends Action
 {
-    const PAGE_PARAM_NAME = 'p';
-    const FILTER_PARAM_NAME = 's';
-    const SEARCH_CONDITION_TYPE = 'like';
-
     /**
      * @var RatesProvider
      */
     private $ratesProvider;
 
-    /** @var SearchCriteriaBuilder */
+    /**
+     * @var SearchCriteriaBuilder
+     */
     private $searchCriteriaBuilder;
 
     /**
@@ -49,20 +51,20 @@ public function __construct(
      */
     public function execute()
     {
-        $ratesPage = (int) $this->getRequest()->getParam(self::PAGE_PARAM_NAME);
-        $ratesFilter = trim($this->getRequest()->getParam(self::FILTER_PARAM_NAME));
+        $ratesPage = (int) $this->getRequest()->getParam('p');
+        $ratesFilter = trim($this->getRequest()->getParam('s'));
 
         try {
             if (!empty($ratesFilter)) {
                 $this->searchCriteriaBuilder->addFilter(
                     Rate::KEY_CODE,
                     '%'.$ratesFilter.'%',
-                    self::SEARCH_CONDITION_TYPE
+                    'like'
                 );
             }
 
             $searchCriteria = $this->searchCriteriaBuilder
-                ->setPageSize(RatesProvider::PAGE_SIZE)
+                ->setPageSize($this->ratesProvider->getPageSize())
                 ->setCurrentPage($ratesPage)
                 ->create();
 
@@ -71,7 +73,7 @@ public function execute()
             $response = [
                 'success' => true,
                 'errorMessage' => '',
-                'result'=>$options,
+                'result'=> $options,
             ];
         } catch (\Exception $e) {
             $response = [

app/code/Magento/Tax/Model/Api/SearchCriteria/JoinProcessor/RateCode.php

@@ -9,8 +9,8 @@
 use Magento\Framework\Data\Collection\AbstractDb;
 
 /**
- * Class RateCode
- * @package Magento\Tax\Model\Api\SearchCriteria\JoinProcessor
+ * Provides additional SQL JOIN to ensure search of required
+ * tax rule by tax rate code in Tax Rules grid.
  */
 class RateCode implements CustomJoinInterface
 {

app/code/Magento/Tax/Model/Rate/Provider.php

@@ -3,7 +3,6 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
-
 namespace Magento\Tax\Model\Rate;
 
 use Magento\Framework\Convert\DataObject as Converter;
@@ -12,17 +11,25 @@
 use Magento\Tax\Model\Calculation\Rate;
 
 /**
- * Paged Tax Rates source model.
+ * Provides filtered tax rates models
+ * as options for select element.
  */
 class Provider
 {
-    const PAGE_SIZE = 100;
-
-    /** @var TaxRateRepositoryInterface */
+    /**
+     * @var TaxRateRepositoryInterface
+     */
     private $taxRateRepository;
 
-    /** @var Converter */
-    protected $converter;
+    /**
+     * @var Converter
+     */
+    private $converter;
+
+    /**
+     * @var int
+     */
+    private $pageSize = 100;
 
     /**
      * Initialize dependencies.
@@ -54,4 +61,14 @@ public function toOptionArray(SearchCriteriaInterface $searchCriteria)
             Rate::KEY_CODE
         );
     }
+
+    /**
+     * Returns predefined size of tax rates list
+     *
+     * @return int
+     */
+    public function getPageSize()
+    {
+        return (int) $this->pageSize;
+    }
 }

app/code/Magento/Tax/Model/Rate/Source.php

@@ -61,7 +61,7 @@ public function toOptionArray()
     {
         if (!$this->options) {
             $searchCriteria = $this->searchCriteriaBuilder
-                ->setPageSize(RateProvider::PAGE_SIZE)
+                ->setPageSize($this->rateProvider->getPageSize())
                 ->setCurrentPage(1)
                 ->create();
 

app/code/Magento/Tax/Test/Unit/Block/Adminhtml/Rule/Edit/FormTest.php

@@ -18,6 +18,11 @@
 use Magento\Tax\Model\TaxClass\Source\Product;
 use PHPUnit_Framework_MockObject_MockObject as MockObject;
 
+/**
+ * Test for Tax Rule Edit Form
+ *
+ * Class FormTest
+ */
 class FormTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -126,7 +131,7 @@ protected function setUp()
     /**
      * Check tax lazy loading URL.
      *
-     * @covers \Magento\Tax\Block\Adminhtml\Rule\Edit\Form::getTaxRatesPageUrl
+     * @see \Magento\Tax\Block\Adminhtml\Rule\Edit\Form::getTaxRatesPageUrl
      */
     public function testTaxRatesPageUrl()
     {
@@ -143,7 +148,7 @@ public function testTaxRatesPageUrl()
      *
      * @param array $formValue
      * @param array $expected
-     * @covers \Magento\Tax\Block\Adminhtml\Rule\Edit\Form::getTaxRatesSelectConfig
+     * @see \Magento\Tax\Block\Adminhtml\Rule\Edit\Form::getTaxRatesSelectConfig
      * @dataProvider formValuesDataProvider
      */
     public function testTaxRatesSelectConfig($formValue, $expected)

app/code/Magento/Tax/Test/Unit/Block/Grid/Renderer/CodesTest.php

@@ -9,6 +9,11 @@
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Tax\Block\Grid\Renderer\Codes;
 
+/**
+ * Test for Tax Rates codes column of Tax Rules grid.
+ *
+ * Class CodesTest
+ */
 class CodesTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -27,7 +32,7 @@ protected function setUp()
      *
      * @param array $ratesCodes
      * @param string $expected
-     * @covers Magento\Tax\Block\Grid\Renderer\Codes::render
+     * @see Magento\Tax\Block\Grid\Renderer\Codes::render
      * @dataProvider ratesCodesDataProvider
      */
     public function testRenderCodes($ratesCodes, $expected)

app/code/Magento/Tax/Test/Unit/Controller/Adminhtml/Rule/AjaxLoadRatesTest.php

@@ -58,22 +58,18 @@ protected function setUp()
 
         $this->searchCriteriaBuilder = $this->getMockBuilder(SearchCriteriaBuilder::class)
             ->disableOriginalConstructor()
-            ->setMethods(['setPageSize', 'setCurrentPage', 'create'])
             ->getMock();
 
         $this->request = $this->getMockBuilder(Request::class)
             ->disableOriginalConstructor()
-            ->setMethods(['getParam'])
             ->getMock();
 
         $this->resultFactory = $this->getMockBuilder(ResultFactory::class)
             ->disableOriginalConstructor()
-            ->setMethods(['create'])
             ->getMock();
 
         $this->ratesProvider = $this->getMockBuilder(RatesProvider::class)
             ->disableOriginalConstructor()
-            ->setMethods(['toOptionArray'])
             ->getMock();
     }
 

app/code/Magento/Tax/i18n/en_US.csv

@@ -174,3 +174,4 @@ Rate,Rate
 "Order Total Incl. Tax","Order Total Incl. Tax"
 "Order Total","Order Total"
 "Your credit card will be charged for","Your credit card will be charged for"
+"An error occurred while loading tax rates.","An error occurred while loading tax rates."

dev/tests/integration/testsuite/Magento/Tax/Controller/Adminhtml/RuleTest.php

@@ -17,6 +17,8 @@
 use Magento\Tax\Model\Rate\Provider as RatesProvider;
 
 /**
+ * Tests for Tax Rules controllers.
+ *
  * @magentoAppArea adminhtml
  */
 class RuleTest extends \Magento\TestFramework\TestCase\AbstractBackendController
@@ -57,6 +59,11 @@ class RuleTest extends \Magento\TestFramework\TestCase\AbstractBackendController
      */
     private $dataObjectHelper;
 
+    /**
+     * @var RatesProvider
+     */
+    private $taxRatesProvider;
+
     /**
      * @inheritdoc
      */
@@ -70,6 +77,7 @@ protected function setUp()
         $this->taxRateFixtureFactory = new TaxRuleFixtureFactory();
         $this->countryFactory = $this->_objectManager->create(CountryFactory::class);
         $this->regionFactory = $this->_objectManager->create(RegionFactory::class);
+        $this->taxRatesProvider = $this->_objectManager->create(RatesProvider::class);
 
         $this->_generateTaxRates();
     }
@@ -104,7 +112,7 @@ public function testAjaxLoadRates($postData, $itemsCount)
      */
     private function _generateTaxRates()
     {
-        $ratesCount = RatesProvider::PAGE_SIZE + 1;
+        $ratesCount = $this->taxRatesProvider->getPageSize() + 1;
         for ($i = 0; $i <= $ratesCount; $i++) {
             $taxData = [
                 'tax_country_id' => 'US',
@@ -132,8 +140,10 @@ private function _generateTaxRates()
      */
     public function ajaxActionDataProvider()
     {
+        $taxRatesProvider = Bootstrap::getObjectManager()->create(RatesProvider::class);
+
         return [
-            [['p' => 1], RatesProvider::PAGE_SIZE],
+            [['p' => 1], $taxRatesProvider->getPageSize()],
             [['p' => 1, 's' => 'no_such_code'], 0]
         ];
     }