-
Notifications
You must be signed in to change notification settings - Fork 9.4k
{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,9 @@ | |
|
||
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; | ||
use Magento\MediaGallery\Model\Asset; | ||
use Magento\MediaGallery\Model\DataExtractor; | ||
use Magento\MediaGallery\Model\Keyword; | ||
use Magento\MediaGalleryApi\Api\Data\AssetInterface; | ||
use Magento\MediaGalleryApi\Api\Data\KeywordInterface; | ||
use Magento\MediaGallery\Model\DataExtractor; | ||
use PHPUnit\Framework\MockObject\MockObject; | ||
use PHPUnit\Framework\TestCase; | ||
|
||
|
@@ -54,22 +53,41 @@ public function testExtractData(string $class, $interfaceClass, array $expectedD | |
'data' => $data, | ||
] | ||
); | ||
$receivedData = $this->dataExtractor->extract($model, $interfaceClass); | ||
$this->checkValues($expectedData, $receivedData, $model); | ||
if ($interfaceClass) { | ||
$receivedData = $this->dataExtractor->extract($model, $interfaceClass); | ||
$this->checkAssetValues($expectedData, $receivedData, $model); | ||
} else { | ||
$receivedData = $this->dataExtractor->extract($model); | ||
$this->checkKeyWordValues($expectedData, $receivedData, $model); | ||
} | ||
} | ||
|
||
/** | ||
* @param array $expectedData | ||
* @param array $data | ||
* @param object $model | ||
*/ | ||
private function checkAssetValues(array $expectedData, array $data, $model) | ||
{ | ||
foreach ($expectedData as $expectedDataKey => $expectedDataItem) { | ||
$this->assertEquals($data[$expectedDataKey] ?? null, $model->{$expectedDataItem['method']}()); | ||
$this->assertEquals($data[$expectedDataKey] ?? null, $expectedDataItem['value']); | ||
} | ||
$this->assertEquals(array_keys($expectedData), array_keys($data)); | ||
} | ||
|
||
/** | ||
* @param array $expectedData | ||
* @param array $data | ||
* @param object $model | ||
*/ | ||
protected function checkValues(array $expectedData, array $data, $model) | ||
private function checkKeyWordValues(array $expectedData, array $data, $model) | ||
{ | ||
foreach ($expectedData as $expectedDataKey => $expectedDataItem) { | ||
$this->assertEquals($data[$expectedDataKey] ?? null, $model->{$expectedDataItem['method']}()); | ||
$this->assertEquals($data[$expectedDataKey] ?? null, $expectedDataItem['value']); | ||
} | ||
$this->assertEquals(array_keys($expectedData), array_keys($expectedData)); | ||
$this->assertEquals(array_keys($expectedData), array_keys(array_slice($data, 0, 2))); | ||
} | ||
|
||
/** | ||
|
@@ -102,13 +120,17 @@ public function assetProvider() | |
'value' => 'content_type', | ||
'method' => 'getContentType', | ||
], | ||
'height' => [ | ||
'value' => 4, | ||
'method' => 'getHeight', | ||
], | ||
'width' => [ | ||
'value' => 3, | ||
'method' => 'getWidth', | ||
], | ||
'height' => [ | ||
'value' => 4, | ||
'method' => 'getHeight', | ||
'size' => [ | ||
'value' => 300, | ||
'method' => 'getSize', | ||
], | ||
'created_at' => [ | ||
'value' => '2019-11-28 10:40:09', | ||
|
@@ -122,7 +144,7 @@ public function assetProvider() | |
], | ||
'Keyword conversion without interface' => [ | ||
Keyword::class, | ||
null, | ||
'', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this value should be either a valid interface name or null There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per function argument type declarations for the method is string so instead of null I just replaced with the empty string There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep it logically correct: it should be either interface name as string or null |
||
[ | ||
'id' => [ | ||
'value' => 2, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the reason to change the test logic after adding a
size
field. Only data provider should be changed. What is the reason for this change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to improve the Unit test for this line
magento2/app/code/Magento/MediaGallery/Test/Unit/Model/DataExtractorTest.php
Line 72 in 1a77b70