🔃 [Magento Community Engineering] Community Contributions - 2.3-develop latest changes

Accepted Community Pull Requests:
 - #25546: Prevent saving upload media files in root directory (by @kenboy)
 - #25629: Reverted PR 25022 (by @drpayyne)
 - #25547: Fix exception handling and cover with unit tests the GetById media asset command (by @coderimus)
 - #25613: Navigation menu 4th level/category does not appear consistently #25589 (by @fabriciosobral)
 - #25355: M2C-2205 Move LESS styling files (by @ptylek)
 - #25638: Add indexer console command output (by @brosenberger)
 - #25623: #25043 - Since   2.3.3 Store logo automatic resize on mobile/smaller displays does not respect aspect ratio (by @fabriciosobral)


Fixed GitHub Issues:
 - #24840: Drop down icon invisible for a moment if not select value and click on Load Template (reported by @hasim32) has been fixed in #25629 by @drpayyne in 2.3-develop branch
   Related commits:
     1. 16aabf7

 - #25589: Navigation menu 4th level/category does not appear consistently (reported by @dbabaev) has been fixed in #25613 by @fabriciosobral in 2.3-develop branch
   Related commits:
     1. 16fa4c4

 - #25276: Magento_Contact module MUST NOT include any design-level LESS styling  (reported by @danemacmillan) has been fixed in #25355 by @ptylek in 2.3-develop branch
   Related commits:
     1. 42d2163
     2. 2c6ad75

 - #25043: Since 2.3.3 Store logo automatic resize on mobile/smaller displays does not respect aspect ratio (reported by @gwharton) has been fixed in #25623 by @fabriciosobral in 2.3-develop branch
   Related commits:
     1. d0167a0

Author: VladimirZaets

app/code/Magento/CatalogImportExport/Model/Import/Product.php

@@ -2035,9 +2035,9 @@ protected function _saveProductTierPrices(array $tierPriceData)
     protected function _getUploader()
     {
         if ($this->_fileUploader === null) {
-            $this->_fileUploader = $this->_uploaderFactory->create();
+            $fileUploader = $this->_uploaderFactory->create();
 
-            $this->_fileUploader->init();
+            $fileUploader->init();
 
             $dirConfig = DirectoryList::getDefaultConfig();
             $dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
@@ -2048,7 +2048,7 @@ protected function _getUploader()
                 $tmpPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath('import');
             }
 
-            if (!$this->_fileUploader->setTmpDir($tmpPath)) {
+            if (!$fileUploader->setTmpDir($tmpPath)) {
                 throw new LocalizedException(
                     __('File directory \'%1\' is not readable.', $tmpPath)
                 );
@@ -2057,11 +2057,13 @@ protected function _getUploader()
             $destinationPath = $dirAddon . '/' . $this->_mediaDirectory->getRelativePath($destinationDir);
 
             $this->_mediaDirectory->create($destinationPath);
-            if (!$this->_fileUploader->setDestDir($destinationPath)) {
+            if (!$fileUploader->setDestDir($destinationPath)) {
                 throw new LocalizedException(
                     __('File directory \'%1\' is not writable.', $destinationPath)
                 );
             }
+
+            $this->_fileUploader = $fileUploader;
         }
         return $this->_fileUploader;
     }

app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php

@@ -284,9 +284,11 @@ protected function setUp()
                 ->getMock();
         $this->storeResolver =
             $this->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Product\StoreResolver::class)
-                ->setMethods([
-                    'getStoreCodeToId',
-                ])
+                ->setMethods(
+                    [
+                        'getStoreCodeToId',
+                    ]
+                )
                 ->disableOriginalConstructor()
                 ->getMock();
         $this->skuProcessor =
@@ -410,7 +412,7 @@ protected function _objectConstructor()
         $this->_filesystem->expects($this->once())
             ->method('getDirectoryWrite')
             ->with(DirectoryList::ROOT)
-            ->will($this->returnValue(self::MEDIA_DIRECTORY));
+            ->willReturn($this->_mediaDirectory);
 
         $this->validator->expects($this->any())->method('init');
         return $this;
@@ -596,9 +598,13 @@ public function testGetMultipleValueSeparatorDefault()
     public function testGetMultipleValueSeparatorFromParameters()
     {
         $expectedSeparator = 'value';
-        $this->setPropertyValue($this->importProduct, '_parameters', [
-            \Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            '_parameters',
+            [
+                \Magento\ImportExport\Model\Import::FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR => $expectedSeparator,
+            ]
+        );
 
         $this->assertEquals(
             $expectedSeparator,
@@ -618,9 +624,13 @@ public function testGetEmptyAttributeValueConstantDefault()
     public function testGetEmptyAttributeValueConstantFromParameters()
     {
         $expectedSeparator = '__EMPTY__VALUE__TEST__';
-        $this->setPropertyValue($this->importProduct, '_parameters', [
-            \Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            '_parameters',
+            [
+                \Magento\ImportExport\Model\Import::FIELD_EMPTY_ATTRIBUTE_VALUE_CONSTANT => $expectedSeparator,
+            ]
+        );
 
         $this->assertEquals(
             $expectedSeparator,
@@ -632,9 +642,12 @@ public function testDeleteProductsForReplacement()
     {
         $importProduct = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods([
-                'setParameters', '_deleteProducts'
-            ])
+            ->setMethods(
+                [
+                    'setParameters',
+                    '_deleteProducts'
+                ]
+            )
             ->getMock();
 
         $importProduct->expects($this->once())->method('setParameters')->with(
@@ -764,9 +777,13 @@ public function testGetProductWebsites()
             'key 3' => 'val',
         ];
         $expectedResult = array_keys($productValue);
-        $this->setPropertyValue($this->importProduct, 'websitesCache', [
-            $productSku => $productValue
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            'websitesCache',
+            [
+                $productSku => $productValue
+            ]
+        );
 
         $actualResult = $this->importProduct->getProductWebsites($productSku);
 
@@ -785,9 +802,13 @@ public function testGetProductCategories()
             'key 3' => 'val',
         ];
         $expectedResult = array_keys($productValue);
-        $this->setPropertyValue($this->importProduct, 'categoriesCache', [
-            $productSku => $productValue
-        ]);
+        $this->setPropertyValue(
+            $this->importProduct,
+            'categoriesCache',
+            [
+                $productSku => $productValue
+            ]
+        );
 
         $actualResult = $this->importProduct->getProductCategories($productSku);
 
@@ -1112,9 +1133,13 @@ public function testValidateRowSetAttributeSetCodeIntoRowData()
             ->disableOriginalConstructor()
             ->getMock();
         $productType->expects($this->once())->method('isRowValid')->with($expectedRowData);
-        $this->setPropertyValue($importProduct, '_productTypeModels', [
-            $newSku['type_id'] => $productType
-        ]);
+        $this->setPropertyValue(
+            $importProduct,
+            '_productTypeModels',
+            [
+                $newSku['type_id'] => $productType
+            ]
+        );
 
         //suppress option validation
         $this->_rewriteGetOptionEntityInImportProduct($importProduct);
@@ -1229,6 +1254,56 @@ public function testParseAttributesWithWrappedValuesWillReturnsLowercasedAttribu
         $this->assertArrayNotHasKey('PARAM2', $attributes);
     }
 
+    /**
+     * @param bool $isRead
+     * @param bool $isWrite
+     * @param string $message
+     * @dataProvider fillUploaderObjectDataProvider
+     */
+    public function testFillUploaderObject($isRead, $isWrite, $message)
+    {
+        $fileUploaderMock = $this
+            ->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $fileUploaderMock
+            ->method('setTmpDir')
+            ->with('pub/media/import')
+            ->willReturn($isRead);
+
+        $fileUploaderMock
+            ->method('setDestDir')
+            ->with('pub/media/catalog/product')
+            ->willReturn($isWrite);
+
+        $this->_mediaDirectory
+            ->method('getRelativePath')
+            ->willReturnMap(
+                [
+                    ['import', 'import'],
+                    ['catalog/product', 'catalog/product'],
+                ]
+            );
+
+        $this->_mediaDirectory
+            ->method('create')
+            ->with('pub/media/catalog/product');
+
+        $this->_uploaderFactory
+            ->expects($this->once())
+            ->method('create')
+            ->willReturn($fileUploaderMock);
+
+        try {
+            $this->importProduct->getUploader();
+            $this->assertNotNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
+        } catch (\Magento\Framework\Exception\LocalizedException $e) {
+            $this->assertNull($this->getPropertyValue($this->importProduct, '_fileUploader'));
+            $this->assertEquals($message, $e->getMessage());
+        }
+    }
+
     /**
      * Test that errors occurred during importing images are logged.
      *
@@ -1275,6 +1350,20 @@ function ($name) use ($throwException, $exception) {
         );
     }
 
+    /**
+     * Data provider for testFillUploaderObject.
+     *
+     * @return array
+     */
+    public function fillUploaderObjectDataProvider()
+    {
+        return [
+            [false, true, 'File directory \'pub/media/import\' is not readable.'],
+            [true, false, 'File directory \'pub/media/catalog/product\' is not writable.'],
+            [true, true, ''],
+        ];
+    }
+
     /**
      * Data provider for testUploadMediaFiles.
      *

app/code/Magento/Indexer/Console/Command/IndexerStatusCommand.php

@@ -5,19 +5,19 @@
  */
 namespace Magento\Indexer\Console\Command;
 
-use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Output\OutputInterface;
 use Magento\Framework\Indexer;
 use Magento\Framework\Mview;
 use Symfony\Component\Console\Helper\Table;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
 
 /**
  * Command for displaying status of indexers.
  */
 class IndexerStatusCommand extends AbstractIndexerManageCommand
 {
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      */
     protected function configure()
     {
@@ -29,12 +29,14 @@ protected function configure()
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
+     * @param InputInterface $input
+     * @param OutputInterface $output
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
         $table = new Table($output);
-        $table->setHeaders(['Title', 'Status', 'Update On', 'Schedule Status', 'Schedule Updated']);
+        $table->setHeaders(['ID', 'Title', 'Status', 'Update On', 'Schedule Status', 'Schedule Updated']);
 
         $rows = [];
 
@@ -43,6 +45,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $view = $indexer->getView();
 
             $rowData = [
+                'ID'                => $indexer->getId(),
                 'Title'             => $indexer->getTitle(),
                 'Status'            => $this->getStatus($indexer),
                 'Update On'         => $indexer->isScheduled() ? 'Schedule' : 'Save',
@@ -59,15 +62,20 @@ protected function execute(InputInterface $input, OutputInterface $output)
             $rows[] = $rowData;
         }
 
-        usort($rows, function ($comp1, $comp2) {
-            return strcmp($comp1['Title'], $comp2['Title']);
-        });
+        usort(
+            $rows,
+            function (array $comp1, array $comp2) {
+                return strcmp($comp1['Title'], $comp2['Title']);
+            }
+        );
 
         $table->addRows($rows);
         $table->render();
     }
 
     /**
+     * Returns the current status of the indexer
+     *
      * @param Indexer\IndexerInterface $indexer
      * @return string
      */
@@ -89,6 +97,8 @@ private function getStatus(Indexer\IndexerInterface $indexer)
     }
 
     /**
+     * Returns the pending count of the view
+     *
      * @param Mview\ViewInterface $view
      * @return string
      */

app/code/Magento/Indexer/Test/Unit/Console/Command/IndexerStatusCommandTest.php

@@ -96,47 +96,53 @@ public function testExecuteAll(array $indexers)
 
         $linesOutput = array_filter(explode(PHP_EOL, $commandTester->getDisplay()));
 
-        $spacer = '+----------------+------------------+-----------+-------------------------+---------------------+';
+        $spacer = '+-----------+----------------+------------------+-----------+-------------------------+'
+            . '---------------------+';
 
         $this->assertCount(8, $linesOutput, 'There should be 8 lines output. 3 Spacers, 1 header, 4 content.');
         $this->assertEquals($linesOutput[0], $spacer, "Lines 0, 2, 7 should be spacer lines");
         $this->assertEquals($linesOutput[2], $spacer, "Lines 0, 2, 7 should be spacer lines");
         $this->assertEquals($linesOutput[7], $spacer, "Lines 0, 2, 7 should be spacer lines");
 
         $headerValues = array_values(array_filter(explode('|', $linesOutput[1])));
-        $this->assertEquals('Title', trim($headerValues[0]));
-        $this->assertEquals('Status', trim($headerValues[1]));
-        $this->assertEquals('Update On', trim($headerValues[2]));
-        $this->assertEquals('Schedule Status', trim($headerValues[3]));
-        $this->assertEquals('Schedule Updated', trim($headerValues[4]));
+        $this->assertEquals('ID', trim($headerValues[0]));
+        $this->assertEquals('Title', trim($headerValues[1]));
+        $this->assertEquals('Status', trim($headerValues[2]));
+        $this->assertEquals('Update On', trim($headerValues[3]));
+        $this->assertEquals('Schedule Status', trim($headerValues[4]));
+        $this->assertEquals('Schedule Updated', trim($headerValues[5]));
 
         $indexer1 = array_values(array_filter(explode('|', $linesOutput[3])));
-        $this->assertEquals('Title_indexer1', trim($indexer1[0]));
-        $this->assertEquals('Ready', trim($indexer1[1]));
-        $this->assertEquals('Schedule', trim($indexer1[2]));
-        $this->assertEquals('idle (10 in backlog)', trim($indexer1[3]));
-        $this->assertEquals('2017-01-01 11:11:11', trim($indexer1[4]));
+        $this->assertEquals('indexer_1', trim($indexer1[0]));
+        $this->assertEquals('Title_indexer1', trim($indexer1[1]));
+        $this->assertEquals('Ready', trim($indexer1[2]));
+        $this->assertEquals('Schedule', trim($indexer1[3]));
+        $this->assertEquals('idle (10 in backlog)', trim($indexer1[4]));
+        $this->assertEquals('2017-01-01 11:11:11', trim($indexer1[5]));
 
         $indexer2 = array_values(array_filter(explode('|', $linesOutput[4])));
-        $this->assertEquals('Title_indexer2', trim($indexer2[0]));
-        $this->assertEquals('Reindex required', trim($indexer2[1]));
-        $this->assertEquals('Save', trim($indexer2[2]));
-        $this->assertEquals('', trim($indexer2[3]));
+        $this->assertEquals('indexer_2', trim($indexer2[0]));
+        $this->assertEquals('Title_indexer2', trim($indexer2[1]));
+        $this->assertEquals('Reindex required', trim($indexer2[2]));
+        $this->assertEquals('Save', trim($indexer2[3]));
         $this->assertEquals('', trim($indexer2[4]));
+        $this->assertEquals('', trim($indexer2[5]));
 
         $indexer3 = array_values(array_filter(explode('|', $linesOutput[5])));
-        $this->assertEquals('Title_indexer3', trim($indexer3[0]));
-        $this->assertEquals('Processing', trim($indexer3[1]));
-        $this->assertEquals('Schedule', trim($indexer3[2]));
-        $this->assertEquals('idle (100 in backlog)', trim($indexer3[3]));
-        $this->assertEquals('2017-01-01 11:11:11', trim($indexer3[4]));
+        $this->assertEquals('indexer_3', trim($indexer3[0]));
+        $this->assertEquals('Title_indexer3', trim($indexer3[1]));
+        $this->assertEquals('Processing', trim($indexer3[2]));
+        $this->assertEquals('Schedule', trim($indexer3[3]));
+        $this->assertEquals('idle (100 in backlog)', trim($indexer3[4]));
+        $this->assertEquals('2017-01-01 11:11:11', trim($indexer3[5]));
 
         $indexer4 = array_values(array_filter(explode('|', $linesOutput[6])));
-        $this->assertEquals('Title_indexer4', trim($indexer4[0]));
-        $this->assertEquals('unknown', trim($indexer4[1]));
-        $this->assertEquals('Schedule', trim($indexer4[2]));
-        $this->assertEquals('running (20 in backlog)', trim($indexer4[3]));
-        $this->assertEquals('2017-01-01 11:11:11', trim($indexer4[4]));
+        $this->assertEquals('indexer_4', trim($indexer4[0]));
+        $this->assertEquals('Title_indexer4', trim($indexer4[1]));
+        $this->assertEquals('unknown', trim($indexer4[2]));
+        $this->assertEquals('Schedule', trim($indexer4[3]));
+        $this->assertEquals('running (20 in backlog)', trim($indexer4[4]));
+        $this->assertEquals('2017-01-01 11:11:11', trim($indexer4[5]));
     }
 
     /**