{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388

konarshankar07 · 2020-03-22T05:50:20Z

Description (*)

This PR is the part of Adobe stock integration

Dependent to

magento/adobe-stock-integration#1021

Fixed Issues (if relevant)

Image size is not passed to image-uploader when inserting an image from new media gallery adobe-stock-integration#1010: Image size is not passed to image-uploader when inserting an image from new media gallery

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

m2-assistant · 2020-03-22T05:50:22Z

Hi @konarshankar07. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

rogyar · 2020-03-22T12:31:49Z

app/code/Magento/MediaGallery/Test/Unit/Model/DataExtractorTest.php

+     * @param array $data
+     * @param object $model
+     */
+    protected function checkAssetValues(array $expectedData, array $data, $model)


I would suggest using private scope for the private methods. protected scope assumes that we consider an inheritance of this test class by another test class. It's not a big deal, however, it's recommended to be strict with scoping and hide implementation details in such cases.

rogyar · 2020-03-22T12:36:38Z

@magento run all tests

sivaschenko

Thanks for the pull request @konarshankar07 ! Please see my code review notes

sivaschenko · 2020-03-22T12:28:50Z

app/code/Magento/MediaGalleryApi/Api/Data/AssetInterface.php

@@ -65,6 +65,13 @@ public function getHeight(): int;
     */
    public function getWidth(): int;

+    /**
+     * Retrieve full licensed asset's size


Suggested change

* Retrieve full licensed asset's size

* Retrieve asset file size in bytes

sivaschenko · 2020-03-22T12:30:48Z

app/code/Magento/MediaGallery/etc/db_schema.xml

@@ -14,6 +14,7 @@
        <column xsi:type="varchar" name="content_type" length="255" nullable="true" comment="Content Type"/>
        <column xsi:type="int" name="width" padding="10" unsigned="true" nullable="false" identity="false" default="0" comment="Width"/>
        <column xsi:type="int" name="height" padding="10" unsigned="true" nullable="false" identity="false" default="0" comment="Height"/>
+        <column xsi:type="int" name="size" padding="10" unsigned="true" nullable="false" identity="false" default="0" comment="Size"/>


Suggested change

<column xsi:type="int" name="size" padding="10" unsigned="true" nullable="false" identity="false" default="0" comment="Size"/>

<column xsi:type="int" name="size" padding="10" unsigned="true" nullable="false" identity="false" comment="Asset file size in bytes"/>

sivaschenko · 2020-03-22T12:32:38Z

app/code/Magento/MediaGallery/Test/Unit/Model/DataExtractorTest.php

@@ -122,7 +144,7 @@ public function assetProvider()
            ],
            'Keyword conversion without interface' => [
                Keyword::class,
-                null,
+                '',


this value should be either a valid interface name or null

As per function argument type declarations for the method is string so instead of null I just replaced with the empty string

Let's keep it logically correct: it should be either interface name as string or null

sivaschenko · 2020-03-22T12:35:06Z

app/code/Magento/MediaGallery/Test/Unit/Model/DataExtractorTest.php

@@ -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) {


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?

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

$this->assertEquals(array_keys($expectedData), array_keys($expectedData));

sivaschenko · 2020-03-24T12:24:06Z

Failed tests look to be random and not relevant to the changes in the PR

magento-engcom-team · 2020-03-24T12:24:28Z

Hi @sivaschenko, thank you for the review.
ENGCOM-7196 has been created to process this Pull Request
✳️ @sivaschenko, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Bravo · 2020-03-24T15:48:54Z

✔️ QA Passed

Switched to the related Adobe Stock Integration PR 1021 and to the current PR

The image size is displayed, no NaN undefined error

engcom-Echo · 2020-03-25T09:57:11Z

Failed functional tests not related to the changes in this PR

magento-engcom-team · 2020-03-26T13:56:39Z

Hi @rogyar, thank you for the review.
ENGCOM-7196 has been created to process this Pull Request

… inserting an image from new media gallery #27388

m2-assistant · 2020-03-28T22:21:43Z

Hi @konarshankar07, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

konarshankar07 added 3 commits March 22, 2020 11:09

Partial Implementaion of ASI issue

b870638

Added missing changes from previous commit

6156274

Updated PHPDOC block

e31dafd

konarshankar07 requested a review from sivaschenko March 22, 2020 05:50

ghost added the Progress: pending review label Mar 22, 2020

magento-engcom-team added Component: MediaGallery Component: MediaGalleryApi Release Line: 2.4 labels Mar 22, 2020

konarshankar07 mentioned this pull request Mar 22, 2020

#1010: Image size is not passed to image-uploader when inserting an image from new media gallery magento/adobe-stock-integration#1021

Merged

rogyar self-assigned this Mar 22, 2020

rogyar reviewed Mar 22, 2020

View reviewed changes

sivaschenko requested changes Mar 22, 2020

View reviewed changes

ghost assigned sivaschenko Mar 22, 2020

ghost added Progress: needs update and removed Progress: pending review labels Mar 22, 2020

Code review changes

57db694

sivaschenko approved these changes Mar 24, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 24, 2020

sivaschenko added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Mar 24, 2020

engcom-Bravo self-assigned this Mar 24, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Mar 24, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Mar 24, 2020

engcom-Echo self-assigned this Mar 24, 2020

ghost added Progress: accept and removed Progress: extended testing labels Mar 25, 2020

rogyar approved these changes Mar 26, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: accept labels Mar 26, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Mar 26, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Mar 26, 2020

ghost added Progress: accept and removed Progress: extended testing labels Mar 26, 2020

magento-engcom-team pushed a commit that referenced this pull request Mar 28, 2020

ENGCOM-7196: {ASI} :- Image size is not passed to image-uploader when…

259e16d

… inserting an image from new media gallery #27388

magento-engcom-team merged commit 60ac99c into magento:2.4-develop Mar 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388

{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388

konarshankar07 commented Mar 22, 2020 •

edited by slavvka

Loading

m2-assistant bot commented Mar 22, 2020

rogyar Mar 22, 2020

rogyar commented Mar 22, 2020

sivaschenko left a comment

sivaschenko Mar 22, 2020

sivaschenko Mar 22, 2020

sivaschenko Mar 22, 2020

konarshankar07 Mar 23, 2020

sivaschenko Mar 23, 2020

sivaschenko Mar 22, 2020

konarshankar07 Mar 23, 2020

sivaschenko commented Mar 24, 2020

magento-engcom-team commented Mar 24, 2020

engcom-Bravo commented Mar 24, 2020

engcom-Echo commented Mar 25, 2020

magento-engcom-team commented Mar 26, 2020

m2-assistant bot commented Mar 28, 2020

	* Retrieve full licensed asset's size
	* Retrieve asset file size in bytes

	<column xsi:type="int" name="size" padding="10" unsigned="true" nullable="false" identity="false" default="0" comment="Size"/>
	<column xsi:type="int" name="size" padding="10" unsigned="true" nullable="false" identity="false" comment="Asset file size in bytes"/>

{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388

{ASI} :- Image size is not passed to image-uploader when inserting an image from new media gallery #27388

Conversation

konarshankar07 commented Mar 22, 2020 • edited by slavvka Loading

Description (*)

Dependent to

Fixed Issues (if relevant)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Mar 22, 2020

Choose a reason for hiding this comment

rogyar commented Mar 22, 2020

sivaschenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko commented Mar 24, 2020

magento-engcom-team commented Mar 24, 2020

engcom-Bravo commented Mar 24, 2020

engcom-Echo commented Mar 25, 2020

magento-engcom-team commented Mar 26, 2020

m2-assistant bot commented Mar 28, 2020

konarshankar07 commented Mar 22, 2020 •

edited by slavvka

Loading