🔃 [EngCom] Public Pull Requests - 2.3-develop

magento-engcom-team · web-flow · commit 250045e0dcfa · 2019-02-24T23:40:04.000+02:00
Accepted Public Pull Requests: - #21020: Make the module list more deterministic (by @ajardin) - #20596: Optimize snail_case replacement to PascalCase (by @lbajsarowicz) - #20621: Fix referenced to "store", changing to "scope" in Framework/Mail components (by @gwharton) - #20371: Issue Fixed: #8086: Multiline admin field is broken (by @vivekkumarcedcoss) Fixed GitHub Issues: - #8479: Sequence of module load order should be deterministic (reported by @scottsb) has been fixed in #21020 by @ajardin in 2.3-develop branch Related commits: 1. 7b9c605 2. 78916b3 - #16116: Modules sort order in config.php is being inconsistent when no changes being made (reported by @comdiler) has been fixed in #21020 by @ajardin in 2.3-develop branch Related commits: 1. 7b9c605 2. 78916b3 - #8086: Multiline admin field is broken (reported by @wintermute-84) has been fixed in #20371 by @vivekkumarcedcoss in 2.3-develop branch Related commits: 1. ddb1137 - #18115: Multiline field is broken (reported by @escopecz) has been fixed in #20371 by @vivekkumarcedcoss in 2.3-develop branch Related commits: 1. ddb1137
diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/GridOnly.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/GridOnly.php
@@ -1,12 +1,16 @@
 <?php
 /**
- *
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
 namespace Magento\Catalog\Controller\Adminhtml\Product;
 
-class GridOnly extends \Magento\Catalog\Controller\Adminhtml\Product
+use Magento\Framework\App\Action\HttpGetActionInterface;
+
+/**
+ * Get specified tab grid controller.
+ */
+class GridOnly extends \Magento\Catalog\Controller\Adminhtml\Product implements HttpGetActionInterface
 {
     /**
      * @var \Magento\Framework\Controller\Result\RawFactory
@@ -47,7 +51,7 @@ public function execute()
         $this->productBuilder->build($this->getRequest());
 
         $block = $this->getRequest()->getParam('gridOnlyBlock');
-        $blockClassSuffix = str_replace(' ', '_', ucwords(str_replace('_', ' ', $block)));
+        $blockClassSuffix = ucwords($block, '_');
 
         /** @var \Magento\Framework\Controller\Result\Raw $resultRaw */
         $resultRaw = $this->resultRawFactory->create();
diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Category/CurrentUrlRewritesRegeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Category/CurrentUrlRewritesRegeneratorTest.php
@@ -252,7 +252,7 @@ protected function getCurrentRewritesMocks($currentRewrites)
                 ->disableOriginalConstructor()->getMock();
             foreach ($urlRewrite as $key => $value) {
                 $url->expects($this->any())
-                    ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))
+                    ->method('get' . str_replace('_', '', ucwords($key, '_')))
                     ->will($this->returnValue($value));
             }
             $rewrites[] = $url;
diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Product/CurrentUrlRewritesRegeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Product/CurrentUrlRewritesRegeneratorTest.php
@@ -294,7 +294,7 @@ protected function getCurrentRewritesMocks($currentRewrites)
                 ->disableOriginalConstructor()->getMock();
             foreach ($urlRewrite as $key => $value) {
                 $url->expects($this->any())
-                    ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))
+                    ->method('get' . str_replace('_', '', ucwords($key, '_')))
                     ->will($this->returnValue($value));
             }
             $rewrites[] = $url;
diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/AfterImportDataObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/AfterImportDataObserverTest.php
@@ -694,7 +694,7 @@ protected function currentUrlRewritesRegeneratorGetCurrentRewritesMocks($current
                 ->disableOriginalConstructor()->getMock();
             foreach ($urlRewrite as $key => $value) {
                 $url->expects($this->any())
-                    ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))
+                    ->method('get' . str_replace('_', '', ucwords($key, '_')))
                     ->will($this->returnValue($value));
             }
             $rewrites[] = $url;
diff --git a/app/code/Magento/Config/Model/Config.php b/app/code/Magento/Config/Model/Config.php
@@ -424,6 +424,11 @@ protected function _processGroup(
                 if (!isset($fieldData['value'])) {
                     $fieldData['value'] = null;
                 }
+                
+                if ($field->getType() == 'multiline' && is_array($fieldData['value'])) {
+                    $fieldData['value'] = trim(implode(PHP_EOL, $fieldData['value']));
+                }
+                
                 $data = [
                     'field' => $fieldId,
                     'groups' => $groups,
diff --git a/app/code/Magento/Sales/Model/Order/Email/SenderBuilder.php b/app/code/Magento/Sales/Model/Order/Email/SenderBuilder.php
@@ -106,7 +106,7 @@ protected function configureEmailTemplate()
         $this->transportBuilder->setTemplateIdentifier($this->templateContainer->getTemplateId());
         $this->transportBuilder->setTemplateOptions($this->templateContainer->getTemplateOptions());
         $this->transportBuilder->setTemplateVars($this->templateContainer->getTemplateVars());
-        $this->transportBuilder->setFromByStore(
+        $this->transportBuilder->setFromByScope(
             $this->identityContainer->getEmailIdentity(),
             $this->identityContainer->getStore()->getId()
         );
diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Email/SenderBuilderTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Email/SenderBuilderTest.php
@@ -76,7 +76,7 @@ protected function setUp()
                 'setTemplateIdentifier',
                 'setTemplateOptions',
                 'setTemplateVars',
-                'setFromByStore',
+                'setFromByScope',
             ]
         );
 
@@ -103,7 +103,7 @@ protected function setUp()
             ->method('getEmailIdentity')
             ->will($this->returnValue($emailIdentity));
         $this->transportBuilder->expects($this->once())
-            ->method('setFromByStore')
+            ->method('setFromByScope')
             ->with($this->equalTo($emailIdentity), 1);
 
         $this->identityContainerMock->expects($this->once())
@@ -146,7 +146,7 @@ public function testSend()
             ->method('getId')
             ->willReturn(1);
         $this->transportBuilder->expects($this->once())
-            ->method('setFromByStore')
+            ->method('setFromByScope')
             ->with($identity, 1);
         $this->transportBuilder->expects($this->once())
             ->method('addTo')
@@ -176,7 +176,7 @@ public function testSendCopyTo()
             ->method('addTo')
             ->with($this->equalTo('example@mail.com'));
         $this->transportBuilder->expects($this->once())
-            ->method('setFromByStore')
+            ->method('setFromByScope')
             ->with($identity, 1);
         $this->identityContainerMock->expects($this->once())
             ->method('getStore')
diff --git a/app/code/Magento/Tax/Test/Unit/Model/Calculation/RateRepositoryTest.php b/app/code/Magento/Tax/Test/Unit/Model/Calculation/RateRepositoryTest.php
@@ -252,7 +252,7 @@ private function getTaxRateMock(array $taxRateData)
         foreach ($taxRateData as $key => $value) {
             // convert key from snake case to upper case
             $taxRateMock->expects($this->any())
-                ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))
+                ->method('get' . str_replace('_', '', ucwords($key, '_')))
                 ->will($this->returnValue($value));
         }
 
diff --git a/app/code/Magento/Tax/Test/Unit/Model/Sales/Total/Quote/ShippingTest.php b/app/code/Magento/Tax/Test/Unit/Model/Sales/Total/Quote/ShippingTest.php
@@ -133,7 +133,7 @@ private function getMockObject($className, array $objectState)
         $getterValueMap = [];
         $methods = ['__wakeup'];
         foreach ($objectState as $key => $value) {
-            $getterName = 'get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key)));
+            $getterName = 'get' . str_replace('_', '', ucwords($key, '_'));
             $getterValueMap[$getterName] = $value;
             $methods[] = $getterName;
         }
diff --git a/dev/tests/functional/tests/app/Magento/CatalogSearch/Test/Fixture/CatalogSearchQuery/QueryText.php b/dev/tests/functional/tests/app/Magento/CatalogSearch/Test/Fixture/CatalogSearchQuery/QueryText.php
@@ -61,7 +61,7 @@ private function createProducts(FixtureFactory $fixtureFactory, $productsData)
                 $searchValue = isset($productData[2]) ? $productData[2] : $productData[1];
                 if ($this->data === null) {
                     if ($product->hasData($searchValue)) {
-                        $getProperty = 'get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $searchValue)));
+                        $getProperty = 'get' . str_replace('_', '', ucwords($searchValue, '_'));
                         $this->data = $product->$getProperty();
                     } else {
                         $this->data = $searchValue;
diff --git a/dev/tests/functional/tests/app/Magento/Widget/Test/Handler/Widget/Curl.php b/dev/tests/functional/tests/app/Magento/Widget/Test/Handler/Widget/Curl.php
@@ -172,7 +172,7 @@ protected function prepareWidgetInstance(array $data)
         $widgetInstances = [];
         foreach ($data['widget_instance'] as $key => $widgetInstance) {
             $pageGroup = $widgetInstance['page_group'];
-            $method = 'prepare' . str_replace(' ', '', ucwords(str_replace('_', ' ', $pageGroup))) . 'Group';
+            $method = 'prepare' . str_replace('_', '', ucwords($pageGroup, '_')) . 'Group';
             if (!method_exists(__CLASS__, $method)) {
                 throw new \Exception('Method for prepare page group "' . $method . '" is not exist.');
             }
diff --git a/lib/internal/Magento/Framework/Api/SimpleDataObjectConverter.php b/lib/internal/Magento/Framework/Api/SimpleDataObjectConverter.php
@@ -58,7 +58,7 @@ public function convertKeysToCamelCase(array $dataArray)
             if (is_array($fieldValue) && !$this->_isSimpleSequentialArray($fieldValue)) {
                 $fieldValue = $this->convertKeysToCamelCase($fieldValue);
             }
-            $fieldName = lcfirst(str_replace(' ', '', ucwords(str_replace('_', ' ', $fieldName))));
+            $fieldName = lcfirst(str_replace('_', '', ucwords($fieldName, '_')));
             $response[$fieldName] = $fieldValue;
         }
         return $response;
@@ -148,7 +148,7 @@ protected function _unpackAssociativeArray($data)
      */
     public static function snakeCaseToUpperCamelCase($input)
     {
-        return str_replace(' ', '', ucwords(str_replace('_', ' ', $input)));
+        return str_replace('_', '', ucwords($input, '_'));
     }
 
     /**
diff --git a/lib/internal/Magento/Framework/Code/NameBuilder.php b/lib/internal/Magento/Framework/Code/NameBuilder.php
@@ -26,7 +26,7 @@ public function buildClassName($parts)
         $separator = '\\';
         $string = join($separator, $parts);
         $string = str_replace('_', $separator, $string);
-        $className = str_replace(' ', $separator, ucwords(str_replace($separator, ' ', $string)));
+        $className = ucwords($string, $separator);
         return $className;
     }
 }
diff --git a/lib/internal/Magento/Framework/DataObject.php b/lib/internal/Magento/Framework/DataObject.php
@@ -64,8 +64,8 @@ public function addData(array $arr)
      *
      * If $key is an array, it will overwrite all the data in the object.
      *
-     * @param string|array  $key
-     * @param mixed         $value
+     * @param string|array $key
+     * @param mixed $value
      * @return $this
      */
     public function setData($key, $value = null)
@@ -111,7 +111,7 @@ public function unsetData($key = null)
      * and retrieve corresponding member. If data is the string - it will be explode
      * by new line character and converted to array.
      *
-     * @param string     $key
+     * @param string $key
      * @param string|int $index
      * @return mixed
      */
@@ -202,7 +202,7 @@ protected function _getData($key)
      */
     public function setDataUsingMethod($key, $args = [])
     {
-        $method = 'set' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key)));
+        $method = 'set' . str_replace('_', '', ucwords($key, '_'));
         $this->{$method}($args);
         return $this;
     }
@@ -216,12 +216,13 @@ public function setDataUsingMethod($key, $args = [])
      */
     public function getDataUsingMethod($key, $args = null)
     {
-        $method = 'get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key)));
+        $method = 'get' . str_replace('_', '', ucwords($key, '_'));
         return $this->{$method}($args);
     }
 
     /**
      * If $key is empty, checks whether there's any data in the object
+     *
      * Otherwise checks if the specified attribute is set.
      *
      * @param string $key
@@ -272,8 +273,8 @@ public function convertToArray(array $keys = [])
     /**
      * Convert object data into XML string
      *
-     * @param array   $keys array of keys that must be represented
-     * @param string  $rootName root node name
+     * @param array $keys array of keys that must be represented
+     * @param string $rootName root node name
      * @param bool $addOpenTag flag that allow to add initial xml node
      * @param bool $addCdata flag that require wrap all values in CDATA
      * @return string
@@ -436,7 +437,7 @@ protected function _underscore($name)
      *
      * Example: key1="value1" key2="value2" ...
      *
-     * @param   array  $keys array of accepted keys
+     * @param   array $keys array of accepted keys
      * @param   string $valueSeparator separator between key and value
      * @param   string $fieldSeparator separator between key/value pairs
      * @param   string $quote quoting sign
diff --git a/lib/internal/Magento/Framework/DataObject/Copy.php b/lib/internal/Magento/Framework/DataObject/Copy.php
@@ -239,7 +239,7 @@ protected function _setFieldsetFieldValue($target, $targetCode, $value)
      */
     protected function getAttributeValueFromExtensibleDataObject($source, $code)
     {
-        $method = 'get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $code)));
+        $method = 'get' . str_replace('_', '', ucwords($code, '_'));
 
         $methodExists = method_exists($source, $method);
         if ($methodExists == true) {
@@ -273,7 +273,7 @@ protected function getAttributeValueFromExtensibleDataObject($source, $code)
      */
     protected function setAttributeValueFromExtensibleDataObject($target, $code, $value)
     {
-        $method = 'set' . str_replace(' ', '', ucwords(str_replace('_', ' ', $code)));
+        $method = 'set' . str_replace('_', '', ucwords($code, '_'));
 
         $methodExists = method_exists($target, $method);
         if ($methodExists == true) {
diff --git a/lib/internal/Magento/Framework/Mail/Template/TransportBuilder.php b/lib/internal/Magento/Framework/Mail/Template/TransportBuilder.php
@@ -177,29 +177,30 @@ public function setReplyTo($email, $name = null)
     /**
      * Set mail from address
      *
-     * @deprecated This function sets the from address for the first store only.
-     * new function setFromByStore introduced to allow setting of from address
-     * based on store.
-     * @see setFromByStore()
+     * @deprecated This function sets the from address but does not provide
+     * a way of setting the correct from addresses based on the scope.
+     * @see setFromByScope()
      *
      * @param string|array $from
      * @return $this
+     * @throws \Magento\Framework\Exception\MailException
      */
     public function setFrom($from)
     {
-        return $this->setFromByStore($from, null);
+        return $this->setFromByScope($from, null);
     }
 
     /**
-     * Set mail from address by store
+     * Set mail from address by scopeId
      *
      * @param string|array $from
-     * @param string|int $store
+     * @param string|int $scopeId
      * @return $this
+     * @throws \Magento\Framework\Exception\MailException
      */
-    public function setFromByStore($from, $store = null)
+    public function setFromByScope($from, $scopeId = null)
     {
-        $result = $this->_senderResolver->resolve($from, $store);
+        $result = $this->_senderResolver->resolve($from, $scopeId);
         $this->message->setFromAddress($result['email'], $result['name']);
         return $this;
     }
@@ -256,6 +257,7 @@ public function setTemplateOptions($templateOptions)
      * Get mail transport
      *
      * @return \Magento\Framework\Mail\TransportInterface
+     * @throws LocalizedException
      */
     public function getTransport()
     {
diff --git a/lib/internal/Magento/Framework/Mail/Test/Unit/Template/TransportBuilderTest.php b/lib/internal/Magento/Framework/Mail/Test/Unit/Template/TransportBuilderTest.php
@@ -167,20 +167,20 @@ public function getTransportDataProvider()
     /**
      * @return void
      */
-    public function testSetFromByStore()
+    public function testSetFromByScope()
     {
         $sender = ['email' => 'from@example.com', 'name' => 'name'];
-        $store = 1;
+        $scopeId = 1;
         $this->senderResolverMock->expects($this->once())
             ->method('resolve')
-            ->with($sender, $store)
+            ->with($sender, $scopeId)
             ->willReturn($sender);
         $this->messageMock->expects($this->once())
             ->method('setFromAddress')
             ->with($sender['email'], $sender['name'])
             ->willReturnSelf();
 
-        $this->builder->setFromByStore($sender, $store);
+        $this->builder->setFromByScope($sender, $scopeId);
     }
 
     /**
diff --git a/lib/internal/Magento/Framework/Module/ModuleList/Loader.php b/lib/internal/Magento/Framework/Module/ModuleList/Loader.php
@@ -126,24 +126,29 @@ private function getModuleConfigs()
      *
      * @param array $origList
      * @return array
-     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+     * @throws \Exception
      */
-    private function sortBySequence($origList)
+    private function sortBySequence(array $origList): array
     {
         ksort($origList);
+        $modules = $this->prearrangeModules($origList);
+
         $expanded = [];
-        foreach ($origList as $moduleName => $value) {
+        foreach (array_keys($modules) as $moduleName) {
+            $sequence = $this->expandSequence($origList, $moduleName);
+            asort($sequence);
+
             $expanded[] = [
                 'name' => $moduleName,
-                'sequence' => $this->expandSequence($origList, $moduleName),
+                'sequence' => $sequence,
             ];
         }
 
         // Use "bubble sorting" because usort does not check each pair of elements and in this case it is important
         $total = count($expanded);
         for ($i = 0; $i < $total - 1; $i++) {
             for ($j = $i; $j < $total; $j++) {
-                if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'])) {
+                if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'], true)) {
                     $temp = $expanded[$i];
                     $expanded[$i] = $expanded[$j];
                     $expanded[$j] = $temp;
@@ -159,6 +164,27 @@ private function sortBySequence($origList)
         return $result;
     }
 
+    /**
+     * Prearrange all modules by putting those from Magento before the others
+     *
+     * @param array $modules
+     * @return array
+     */
+    private function prearrangeModules(array $modules): array
+    {
+        $breakdown = ['magento' => [], 'others' => []];
+
+        foreach ($modules as $moduleName => $moduleDetails) {
+            if (strpos($moduleName, 'Magento_') !== false) {
+                $breakdown['magento'][$moduleName] = $moduleDetails;
+            } else {
+                $breakdown['others'][$moduleName] = $moduleDetails;
+            }
+        }
+
+        return array_merge($breakdown['magento'], $breakdown['others']);
+    }
+
     /**
      * Accumulate information about all transitive "sequence" references
      *
diff --git a/lib/internal/Magento/Framework/Module/PackageInfo.php b/lib/internal/Magento/Framework/Module/PackageInfo.php
diff --git a/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php b/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php

Original file line number	Diff line number	Diff line change
`@@ -252,7 +252,7 @@ protected function getCurrentRewritesMocks($currentRewrites)`
`252`	`252`	`->disableOriginalConstructor()->getMock();`
`253`	`253`	`foreach ($urlRewrite as $key => $value) {`
`254`	`254`	`$url->expects($this->any())`
`255`		`- ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))`
	`255`	`+ ->method('get' . str_replace('_', '', ucwords($key, '_')))`
`256`	`256`	`->will($this->returnValue($value));`
`257`	`257`	`}`
`258`	`258`	`$rewrites[] = $url;`
Original file line number	Diff line number	Diff line change
`@@ -294,7 +294,7 @@ protected function getCurrentRewritesMocks($currentRewrites)`
`294`	`294`	`->disableOriginalConstructor()->getMock();`
`295`	`295`	`foreach ($urlRewrite as $key => $value) {`
`296`	`296`	`$url->expects($this->any())`
`297`		`- ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))`
	`297`	`+ ->method('get' . str_replace('_', '', ucwords($key, '_')))`
`298`	`298`	`->will($this->returnValue($value));`
`299`	`299`	`}`
`300`	`300`	`$rewrites[] = $url;`
Original file line number	Diff line number	Diff line change
`@@ -694,7 +694,7 @@ protected function currentUrlRewritesRegeneratorGetCurrentRewritesMocks($current`
`694`	`694`	`->disableOriginalConstructor()->getMock();`
`695`	`695`	`foreach ($urlRewrite as $key => $value) {`
`696`	`696`	`$url->expects($this->any())`
`697`		`- ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))`
	`697`	`+ ->method('get' . str_replace('_', '', ucwords($key, '_')))`
`698`	`698`	`->will($this->returnValue($value));`
`699`	`699`	`}`
`700`	`700`	`$rewrites[] = $url;`
Original file line number	Diff line number	Diff line change
`@@ -252,7 +252,7 @@ private function getTaxRateMock(array $taxRateData)`
`252`	`252`	`foreach ($taxRateData as $key => $value) {`
`253`	`253`	`// convert key from snake case to upper case`
`254`	`254`	`$taxRateMock->expects($this->any())`
`255`		`- ->method('get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key))))`
	`255`	`+ ->method('get' . str_replace('_', '', ucwords($key, '_')))`
`256`	`256`	`->will($this->returnValue($value));`
`257`	`257`	`}`
`258`	`258`
Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,7 @@ private function getMockObject($className, array $objectState)`
`133`	`133`	`$getterValueMap = [];`
`134`	`134`	`$methods = ['__wakeup'];`
`135`	`135`	`foreach ($objectState as $key => $value) {`
`136`		`- $getterName = 'get' . str_replace(' ', '', ucwords(str_replace('_', ' ', $key)));`
	`136`	`+ $getterName = 'get' . str_replace('_', '', ucwords($key, '_'));`
`137`	`137`	`$getterValueMap[$getterName] = $value;`
`138`	`138`	`$methods[] = $getterName;`
`139`	`139`	`}`