#359: added Locate button for saved preview images #406

vovsky · 2019-08-31T14:53:13Z

Description (*)

This PR adds ability to locate downloaded image in a file browser

Fixed Issues (if relevant)

Locate saved preview image in Magento media gallery #359: Locate saved preview image in Magento media gallery

magento-cicd2 · 2019-08-31T14:53:16Z

All committers have signed the CLA.

diazwatson · 2019-09-09T22:40:06Z

Hi @vovsky thanks for your contribution!
Could you please fix the static tests?

FILE: ...r/magento/module-adobe-stock-asset/Model/AddIsDownloadedToSearchResult.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 46 | ERROR | [x] Missing short description
 47 | ERROR | [x] There must be exactly one blank line before tags
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

sivaschenko

Hi @vovsky , welcome to Adobe Stock Integration project and thank you for your pull request!

I have an improvement suggestion regarding the attribute passed from backend to frontend to identify that image is downloaded, please take a look at my review comments.

Feel free to contact me on slack for more details

sivaschenko · 2019-09-10T13:15:57Z

AdobeStockAsset/Model/AddIsDownloadedToSearchResult.php

+                $attribute->setValue((int)in_array($item->getId(), $downloadedIds));
+
+                $customAttributes[self::ATTRIBUTE_CODE_IS_DOWNLOADED] = $attribute;
+                $item->setCustomAttributes($customAttributes);


It would be better to add the downloaded path as a custom attribute and use this attribute to open the corresponding folder in media gallery during locate operation.

@sivaschenko agree. I have made the changes, thank you. Locate feature works now

sivaschenko · 2019-09-10T13:21:31Z

AdobeStockAsset/Model/AddIsDownloadedToSearchResult.php

+/**
+ * Class AddIsDownloadedToSearchResult
+ */
+class AddIsDownloadedToSearchResult


I would be good to use generic name for this service like AppendAttributes as later it could be reused to add additional information like is_licensed etc

@sivaschenko agree. I have made the changes, thank you

# Conflicts: # AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js # AdobeStockImageAdminUi/view/adminhtml/web/template/grid/column/image-preview.html

sivaschenko

@vovsky thanks for updates! Looks great! However, there is a specific case that is not handled by the implemented locate functionality. Please take a look at my comments

sivaschenko · 2019-09-12T14:43:03Z

AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js

+         *
+         * @param record
+         */
+        locate: function (record) {


that implementation considers only the filename. Images can be saved to different directories in the media gallery.

So after modal is closed it's needed to:

Divide the record.path to directory path and filename

Use directory path to open corresponding folder in media gallery

Use filename to select corresponding image

sivaschenko · 2019-09-12T14:44:25Z

AdobeStockAsset/Model/AppendAttributes.php

+     * @param SearchResultInterface $searchResult
+     * @return SearchResultInterface
+     */
+    public function execute(SearchResultInterface $searchResult): SearchResultInterface


I'd extract a couple of private functions to make this class easier to read

…into 359-locate-saved-preview-image

sivaschenko · 2019-09-25T18:16:54Z

Hi @vovsky can you please sign the Adobe CLA

filmaj · 2019-09-25T18:51:19Z

FYI this can be done at opensource.adobe.com/cla.html

…obe-stock-integration into 359-locate-saved-preview-image # Conflicts: # AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js

sivaschenko

Hi @vovsky , thanks for the updates! Please see my review comments.

sivaschenko · 2019-09-29T06:42:50Z

AdobeStockAsset/Model/AppendAttributes.php

+        }
+
+        if (count($itemIds)) {
+            $connection = $this->resourceConnection->getConnection();


Can you please move this select from adobe_stock_asset table to a separate class to be able to reuse that, and place this class in the ResourceModel namespace in order to indicate the access to the database. I'd suggest moving to \Magento\AdobeStockAsset\Model\ResourceModel\Asset\Load::execute(array $ids): array

It would also be great if you could cover these files with unit tests

sivaschenko · 2019-09-29T06:55:02Z

AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js

+         * @param record
+         */
+        locate: function (record) {
+            $.ajaxSetup({async: false});


Can you please extract this method to a separate JS file (image-preview.js is becoming a really long file and we are trying to limit it's responsibility to image preview rendering only).

I'd create AdobeStockImageAdminUi/view/adminhtml/web/js/media-gallery.js returning simple object:

define([ ... ], function (...) { 'use strict'; return { jsTreeRootFolderName: 'Storage Root', jsTreeFolderNameMaxLength: 20, selectImage: function (path) { ... } } }

…parate classes

ghost · 2019-10-01T13:32:56Z

Hi @vovsky, 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.

magento#359: added Locate button for saved preview images

09f1d5d

magento-engcom-team added Partner: Atwix partners-contribution labels Aug 31, 2019

vovsky changed the title ~~#359: added Locate button for saved preview images~~ #359: added Locate button for saved preview images (draft) Aug 31, 2019

diazwatson self-assigned this Sep 9, 2019

m2-community-project bot added the Progress: review label Sep 9, 2019

sivaschenko requested changes Sep 10, 2019

View reviewed changes

m2-community-project bot assigned sivaschenko Sep 10, 2019

m2-community-project bot added Progress: changes requested and removed Progress: review labels Sep 10, 2019

vovsky added 2 commits September 11, 2019 22:52

magento#359: locate downloaded image in media browser

55dd365

Merge branch 'develop' into 359-locate-saved-preview-image

e312123

# Conflicts: # AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js # AdobeStockImageAdminUi/view/adminhtml/web/template/grid/column/image-preview.html

sivaschenko requested changes Sep 12, 2019

View reviewed changes

vovsky and others added 2 commits September 22, 2019 15:21

magento#359 attempt to locate image in folder

f815215

Merge branch 'develop' of github.com:magento/adobe-stock-integration …

3430c50

…into 359-locate-saved-preview-image

sivaschenko mentioned this pull request Sep 24, 2019

Search image result has image download state. #275

Closed

vovsky added 2 commits September 27, 2019 16:41

magento#359 fixed case with image location in subfolders

020b861

Merge branch '359-locate-saved-preview-image' of github.com:vovsky/ad…

d5e4e2d

…obe-stock-integration into 359-locate-saved-preview-image # Conflicts: # AdobeStockImageAdminUi/view/adminhtml/web/js/components/grid/column/image-preview.js

vovsky changed the title ~~#359: added Locate button for saved preview images (draft)~~ #359: added Locate button for saved preview images Sep 27, 2019

sivaschenko added the cdmageconf2019 label Sep 27, 2019

sivaschenko requested changes Sep 29, 2019

View reviewed changes

sivaschenko added 2 commits October 1, 2019 11:10

Merge branch 'develop' into 359-locate-saved-preview-image

8bad080

magento#406: Extracted loading from db and locate functionality to se…

97d1343

…parate classes

sivaschenko previously approved these changes Oct 1, 2019

View reviewed changes

magento#406: Fixed static tests

caf2c35

sivaschenko dismissed their stale review via caf2c35 October 1, 2019 13:26

sivaschenko approved these changes Oct 1, 2019

View reviewed changes

sivaschenko merged commit 3568f81 into magento:develop Oct 1, 2019

m2-community-project bot removed the Progress: changes requested label Oct 1, 2019

magento-engcom-team added the Award: advanced label Oct 7, 2019

#359: added Locate button for saved preview images #406

#359: added Locate button for saved preview images #406

Uh oh!

Conversation

vovsky commented Aug 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Uh oh!

magento-cicd2 commented Aug 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diazwatson commented Sep 9, 2019

Uh oh!

sivaschenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vovsky Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sivaschenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sivaschenko commented Sep 25, 2019

Uh oh!

filmaj commented Sep 25, 2019

Uh oh!

sivaschenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vovsky commented Aug 31, 2019 •

edited

Loading

magento-cicd2 commented Aug 31, 2019 •

edited

Loading

vovsky Sep 11, 2019 •

edited

Loading