#1390 Add related content information to the delete asset warning #1401

ProkopovVitaliy · 2020-05-30T12:02:59Z

Description (*)

This PR adds related content information to the delete asset warning

Fixed Issues (if relevant)

Closes Add related content information to the delete asset warning #1390: Add related content information to the delete asset warning
Closes Story 59: User sees warning when deleting image if it's used on storefront #1310: Story 59: User sees warning when deleting image if it's used on storefront

Manual testing scenarios (*)

Open Media Gallery
Upload an image
Insert the image to Page/Block/Product/Category wysiwyg field
Delete the uploaded image

Expected result

The warning message appears with the information that the image is used in the content

Nazar65

Thank you for the PR, could you please take a look at my review comments ?

Nazar65 · 2020-05-30T12:36:20Z

MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js

+         * @return string
+         */
+        getContentMessage: function(recordPath, recordRelatedContentCount) {
+            return $.mage.__('This image is used in '+ recordRelatedContentCount +' page(s). Are you sure you want to delete "' + recordPath + '" image?');


Image also can be used in blocks and products as well, i think we should create a proper message for all cases like in 1 page-pages block-blocks product-products.

@Nazar65 could i use message like: "This image is used in 2 cms_page(s) or (cms_block, catalog_category, etc)" ? Just to use entity_type field from media_content_asset table.

I think it can be the same behaviour, as like this displayed in view details page

@Nazar65 created message for all cases. For example:

Nazar65 · 2020-05-30T12:40:46Z

@magento run all test

Nazar65 · 2020-05-30T12:41:13Z

@magento run all tests

…ation into #1390

ProkopovVitaliy · 2020-06-01T08:02:29Z

@magento run all tests

…ation into #1390

ProkopovVitaliy · 2020-06-01T10:56:56Z

@magento run all tests

MediaGalleryUi/Ui/Component/Listing/Provider.php

gabrieldagama

Hi @ProkopovVitaliy, thanks for the pull request, it looks good!

I would suggest using the media_gallery/image/detailsendpoint to fetch details about the image when the delete button is triggered (similar to what we are doing on the Image Details button), that endpoint already has the information that we need to display the information on the modal. Using this approach we don't need to change the DataProvider and just use javascript to handle the messaging.

Please let me know if you have any questions, we can discuss it on further details.

sivaschenko · 2020-06-04T12:22:19Z

The base branch was changed from 2.0.0-stabilization to 2.0-develop as this PR is targeting the next release

ProkopovVitaliy · 2020-06-08T09:00:13Z

@gabrieldagama Thank you for recommendation.

As i can see MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js action returns much useful information, but how do you think will this information redundant in this case?

If i should to use MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js in MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js to get details before confirmation window opens, don't you think it might slow down response?

gabrieldagama · 2020-06-08T10:40:36Z

Hi @ProkopovVitaliy, thanks for your reply!

Yes, you are right, the MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js does return more information than we actually need at the deletion of the image, but since we are returning information about just one image (not a collection) I believe the performance shouldn't be a problem.

We could implement a new endpoint to handle just the used in data, but I think the performance benefit over the already implemented controller will be small, what do you think?

ProkopovVitaliy · 2020-06-08T11:08:20Z

We could implement a new endpoint to handle just the used in data, but I think the performance benefit over the already implemented controller will be small, what do you think?

@gabrieldagama I thinks new endpoint will be better. Yes performance benefit will be small but it could make functional more decoupled.
But if this is redundant for our goals, i will be use implemented action ( MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js).

On the other hand, if we want to use MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js inside MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js we are required to pass the parameter imageDetailsUrl to getDetails action. I don't see any other option to do it other than getDetails('/media_gallery/image/details', imgId) in MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js that will be a hardcoded. What do you you think about it ?

gabrieldagama · 2020-06-08T12:46:53Z

HI @ProkopovVitaliy

Good point! I believe we can extract the baseContent (confirmation message) from MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js and have it as a parameter. By doing that we will be able to handle the getDetails call on the MediaGalleryUi/view/adminhtml/web/js/grid/columns/image/actions.js and avoid coupling the two actions, what do you think? Thanks!

ProkopovVitaliy · 2020-06-08T15:25:07Z

@gabrieldagama This is cool idea. Thanks!
so can we also identify items for details array ? for example we get array like:

There are no possible to identify what kind are each detail item. What if i implement next changes: MediaGalleryUi/etc/di.xml
from

        <arguments>
            <argument name="detailsProviders" xsi:type="array">
                <item name="10" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Type</item>
                <item name="20" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\CreatedAt</item>
                <item name="30" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\UpdatedAt</item>
                <item name="40" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Width</item>
                <item name="50" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Height</item>
                <item name="60" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Size</item>
                <item name="70" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\UsedIn</item>
            </argument>
        </arguments>
    </type>

to

<type name="Magento\MediaGalleryUi\Model\AssetDetailsProviderPool">
        <arguments>
            <argument name="detailsProviders" xsi:type="array">
                <item name="type" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Type</item>
                <item name="created_at" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\CreatedAt</item>
                <item name="updated_at" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\UpdatedAt</item>
                <item name="width" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Width</item>
                <item name="height" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Height</item>
                <item name="size" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\Size</item>
                <item name="used_in" xsi:type="object">Magento\MediaGalleryUi\Model\AssetDetailsProvider\UsedIn</item>
            </argument>
        </arguments>
    </type>

As result we get an object with identifiable elements.

It will be much easier to work with them in the future and now we will be able to get what we specifically need.
Can i implement this idea, how do you think ?

gabrieldagama · 2020-06-09T09:28:01Z

Hi @ProkopovVitaliy,

Those numbers on the XML are used to define the array order that gets returned on the endpoint and used on the frontend, if you click on the context menu (...) besides the image, you have the View Details link, that modal has the information ordered based on the endpoint response.

I agree that string keys are more straight forward and clear, but since this is attached to another functionality I would suggest on this PR to use the numbers as key for now and we can look into refactoring that functionality later, it would require us to find another way to sort the endpoint response, or the frontend to know what order is correct.

For now, maybe we can just wrap the fetch of the Used In information on a function and when we change the endpoint will be easier for us to change how we are getting the information from the response.

Thanks!

… replaced to endpoint using

ProkopovVitaliy · 2020-06-09T12:41:16Z

@magento run all test

ProkopovVitaliy · 2020-06-11T16:59:35Z

@magento run all tests

ProkopovVitaliy · 2020-06-11T17:02:53Z

@gabrieldagama Hi.

Fixed QA remarks.

Since the image can be deleted from different files, I decided to create an action and implement the functionality in it.

This was done in order not to make dependencies between MediaGalleryUi/view/adminhtml/web/js/action/deleteImage.js and MediaGalleryUi/view/adminhtml/web/js/action/getDetails.js. I think this implementation will allow us to easily reuse the code.

Thanks.

ProkopovVitaliy · 2020-06-11T18:12:56Z

@magento run all tests

ProkopovVitaliy · 2020-06-12T04:30:45Z

@magento run Static Tests

ProkopovVitaliy · 2020-06-12T05:32:21Z

@magento run Static Tests

ProkopovVitaliy · 2020-06-12T11:28:20Z

@magento run all tests

ProkopovVitaliy · 2020-06-12T12:03:04Z

@magento run all tests

ProkopovVitaliy · 2020-06-13T06:38:11Z

@magento run Functional Tests B2B

ProkopovVitaliy · 2020-06-15T08:36:30Z

@gabrieldagama Hi. I fixed found bugs, could you please approve my changes ? Thanks.

gabrieldagama

Thanks for the updates @ProkopovVitaliy! It looks great!

chalov-anton · 2020-06-15T14:13:36Z

✔️ QA Passed

ghost · 2020-06-15T15:27:39Z

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

ProkopovVitaliy and others added 3 commits May 29, 2020 17:55

Add related content information to the delete asset warning

84eae32

corrected message text

e7889e5

Merge branch '2.0.0-stabilization' into #1390

f17ecca

Nazar65 reviewed May 30, 2020

View reviewed changes

ProkopovVitaliy and others added 4 commits June 1, 2020 10:56

changed message for delete confirmation

3b0cd3c

Merge branch '#1390' of github.com:ProkopovVitaliy/adobe-stock-integr…

eedc6ab

…ation into #1390

Merge branch '2.0.0-stabilization' into #1390

ee31084

Update Provider.php

3a1c9d1

ProkopovVitaliy added 2 commits June 1, 2020 13:55

corrected static test

41a0c4a

Merge branch '#1390' of github.com:ProkopovVitaliy/adobe-stock-integr…

ff3dd10

…ation into #1390

ProkopovVitaliy commented Jun 1, 2020

View reviewed changes

MediaGalleryUi/Ui/Component/Listing/Provider.php Outdated Show resolved Hide resolved

gabrieldagama suggested changes Jun 2, 2020

View reviewed changes

m2-community-project bot assigned gabrieldagama Jun 2, 2020

m2-community-project bot added the Progress: changes requested label Jun 2, 2020

sivaschenko changed the base branch from 2.0.0-stabilization to 2.0-develop June 4, 2020 12:21

lenaorobei added this to the 2.1.0 - Part 1 milestone Jun 4, 2020

lenaorobei linked an issue Jun 4, 2020 that may be closed by this pull request

Add related content information to the delete asset warning #1390

Closed

processing of additional information for the removal process has been…

18fecea

… replaced to endpoint using

m2-community-project bot assigned chalov-anton Jun 11, 2020

m2-community-project bot added the Progress: changes requested label Jun 11, 2020

fixed confirmation message for deleting ffrom detail window

05443d7

ProkopovVitaliy dismissed gabrieldagama’s stale review via 05443d7 June 11, 2020 16:54

ProkopovVitaliy and others added 2 commits June 11, 2020 21:12

fixed code style

48d3a3e

Merge branch '2.0-develop' into #1390

c7e80a4

fixed eslint errors

728ccbb

removed extra char

5b1feeb

gabrieldagama approved these changes Jun 15, 2020

View reviewed changes

m2-community-project bot added Progress: acceptance testing and removed Progress: changes requested labels Jun 15, 2020

m2-community-project bot added Progress: ready for delivery and removed Progress: acceptance testing labels Jun 15, 2020

gabrieldagama merged commit 5b97a20 into magento:2.0-develop Jun 15, 2020

m2-community-project bot removed the Progress: ready for delivery label Jun 15, 2020

chalov-anton mentioned this pull request Jun 22, 2020

[MFTF] Cover User sees warning when deleting image if it's used on storefront #1511

Closed

sivaschenko added the Award: bug fix label Jun 29, 2020

sivaschenko mentioned this pull request Jul 2, 2020

Update master branch from develop branch #1555

Merged

sivaschenko removed this from the 2.1.0 milestone Aug 25, 2020

#1390 Add related content information to the delete asset warning #1401

#1390 Add related content information to the delete asset warning #1401

Uh oh!

Conversation

ProkopovVitaliy commented May 30, 2020 • edited by lenaorobei Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Expected result

Uh oh!

Nazar65 left a comment

Choose a reason for hiding this comment

Uh oh!

Nazar65 May 30, 2020

Choose a reason for hiding this comment

Uh oh!

ProkopovVitaliy May 30, 2020 • 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!

Nazar65 May 30, 2020

Choose a reason for hiding this comment

Uh oh!

ProkopovVitaliy Jun 1, 2020

Choose a reason for hiding this comment

Uh oh!

Nazar65 commented May 30, 2020

Uh oh!

Nazar65 commented May 30, 2020

Uh oh!

ProkopovVitaliy commented Jun 1, 2020

Uh oh!

ProkopovVitaliy commented Jun 1, 2020

Uh oh!

Uh oh!

gabrieldagama left a comment • 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!

sivaschenko commented Jun 4, 2020

Uh oh!

ProkopovVitaliy commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabrieldagama commented Jun 8, 2020

Uh oh!

ProkopovVitaliy commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabrieldagama commented Jun 8, 2020

Uh oh!

ProkopovVitaliy commented Jun 8, 2020

Uh oh!

gabrieldagama commented Jun 9, 2020

Uh oh!

ProkopovVitaliy commented Jun 9, 2020

Uh oh!

ProkopovVitaliy commented Jun 11, 2020

Uh oh!

ProkopovVitaliy commented Jun 11, 2020

Uh oh!

ProkopovVitaliy commented Jun 11, 2020

Uh oh!

ProkopovVitaliy commented Jun 12, 2020

Uh oh!

ProkopovVitaliy commented Jun 12, 2020

Uh oh!

ProkopovVitaliy commented Jun 12, 2020

Uh oh!

ProkopovVitaliy commented Jun 12, 2020

Uh oh!

ProkopovVitaliy commented Jun 13, 2020

Uh oh!

ProkopovVitaliy commented Jun 15, 2020

Uh oh!

gabrieldagama left a comment

Choose a reason for hiding this comment

Uh oh!

chalov-anton commented Jun 15, 2020

ProkopovVitaliy commented May 30, 2020 •

edited by lenaorobei

Loading

ProkopovVitaliy May 30, 2020 •

edited

Loading

gabrieldagama left a comment •

edited

Loading

ProkopovVitaliy commented Jun 8, 2020 •

edited

Loading

ProkopovVitaliy commented Jun 8, 2020 •

edited

Loading