Skip to content
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

Introduce used in filter #1531

Merged
merged 22 commits into from Jul 7, 2020
Merged

Introduce used in filter #1531

merged 22 commits into from Jul 7, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 25, 2020

Description (*)

Add Used In filter to the media gallery to allow the user to filter by the area where the images are used in: pages, categories, blocks, products.

Fixed Issues (if relevant)

  1. Fixes Introduce Used In filter #1462: Introduce Used In filter

Manual testing scenarios (*)

in order to test the search must put images on these area pages, categories, blocks, products.

  1. Admin > content > media gallery - click filter button and select your search.

…lery to allow the user to filter by the area where the images are used in: pages, categories, blocks, products
@ghost
Copy link
Author

ghost commented Jun 25, 2020

@magento run all tests

@@ -85,6 +85,7 @@ public function execute(int $assetId): array
'image_url' => $this->getUrl($asset->getPath()),
'title' => $asset->getTitle(),
'path' => $asset->getPath(),
'description' => $asset->getDescription(),
Copy link
Member

@sivaschenko sivaschenko Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this PR contains the changes from #1512. Can you please ensure each PR contains only the changes related to the task

Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @gregbalonzo ! The implementation looks good, however, it should be moved to the MediaGalleryUi module

* See COPYING.txt for license details.
*/

namespace Magento\AdobeStockImageAdminUi\Model\SearchCriteria\CollectionProcessor\FilterProcessor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the files to MediaGalleryUi module.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried moving the file AdobeStockImageAdminUi/Model/SearchCriteria/CollectionProcessor/FilterProcessor/EntityType.php
to
stock-integration/MediaGalleryUi/Model/SearchCriteria/CollectionProcessor/FilterProcessor/
i cannot get to work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that all the implementation should be moved to MediaGalleryUi module, not just a single file (for sure, the namespaces should be changed accordingly).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi sergii, done moving the implementation MediaGalleryUi module

@gabrieldagama
Copy link
Contributor

@magento run all tests

*/
public function toOptionArray(): array
{
return [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we inject this array here via di.xml? That will allow other extensions to easily add their entity to this.

Copy link
Contributor

@coderimus coderimus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gregbalonzo , thank you for your contribution. Please, check my suggestions below.

@ghost
Copy link
Author

ghost commented Jun 30, 2020

@magento run all tests

@coderimus
Copy link
Contributor

Hello @gregbalonzo ! Thank you for the provided changes. Please, fix the conflict at the image-details.html

@ghost
Copy link
Author

ghost commented Jun 30, 2020

@magento run all tests

Copy link
Contributor

@chalov-anton chalov-anton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ QA Failed

Please observe the following points from less important to more important:

  • Since plural is used with Blocks, Categories, Products, please use plural for Pages also
    used_in_filter_pages

  • there is a frontend issue when applying filters NOT from Standalone Media Gallery, for example from Catalog - Categories
    used_in_filter

  • if there is an image which is used in two entities, for example in Product and in Category, and both filters are applied - an Item (Magento\Framework\View\Element\UiComponent\DataProvider\Document) with the same ID "1" already exists. text is displayed
    used_in_filter_cat_cat
    works properly when only one of the filters is applied

@chalov-anton
Copy link
Contributor

chalov-anton commented Jul 3, 2020

@ghost ghost dismissed stale reviews from coderimus and gabrieldagama via 48d0853 July 3, 2020 14:47
@gabrieldagama
Copy link
Contributor

@magento run all tests

@gabrieldagama
Copy link
Contributor

@magento run all tests

gabrieldagama
gabrieldagama previously approved these changes Jul 6, 2020
@chalov-anton
Copy link
Contributor

✔️ QA Passed

@sivaschenko sivaschenko merged commit 5738b5b into magento:2.0-develop Jul 7, 2020
@ghost
Copy link

ghost commented Jul 7, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Used In filter
5 participants