-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Implemented UrlFilterApplier component #28932
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
Implemented UrlFilterApplier component #28932
Conversation
Hi @gabrieldagama. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. |
@magento run all tests |
@magento run all tests |
@magento run all tests |
@magento run all tests |
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.
Great contribution @gabrieldagama ! Thanks for the double test coverage! Please take a look at my comments
<title value="Verify that filter is applied on product grid when filters parameter is set on url"/> | ||
<description value="Accessing product grid url with filters parameter"/> | ||
<severity value="MAJOR"/> | ||
<testCaseId value="to-be-added"/> |
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.
Test case id needs to be added
<see selector="{{AdminProductGridSection.productGridNameProduct($$createSimpleProduct.name$$)}}" userInput="$$createSimpleProduct.name$$" stepKey="seeProduct"/> | ||
<seeElement selector="{{AdminProductGridFilterSection.enabledFilters}}" stepKey="seeEnabledFilters"/> | ||
<see selector="{{AdminProductGridFilterSection.enabledFilters}}" userInput="Name: $$createSimpleProduct.name$$" stepKey="seeProductNameFilter"/> | ||
<actionGroup ref="ClearFiltersAdminDataGridActionGroup" stepKey="clearGridFilter"/> |
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'd move this action group to after
section
<title value="Verify that filter is applied on block grid when filters parameter is set on url"/> | ||
<description value="Accessing block grid url with filters parameter"/> | ||
<severity value="MAJOR"/> | ||
<testCaseId value="to-be-added"/> |
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.
Test case need to be added
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.
Yes, @chalov-anton can you provide the test cases for this issue?
<see selector="{{CmsPagesPageActionsSection.pagesGridRowByTitle($$createPage.title$$)}}" userInput="$$createPage.title$$" stepKey="seePage"/> | ||
<seeElement selector="{{CmsPagesPageActionsSection.activeFilter}}" stepKey="seeEnabledFilters"/> | ||
<see selector="{{CmsPagesPageActionsSection.activeFilter}}" userInput="Title: $$createPage.title$$" stepKey="seePageTitleFilter"/> | ||
<actionGroup ref="ClearFiltersAdminDataGridActionGroup" stepKey="clearGridFilter"/> |
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'd move this action group to after
section
|
||
return Component.extend({ | ||
defaults: { | ||
filterProvider: 'componentType = filters', |
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.
how this component is behaving on a page with multiple grids?
I.e. media gallery + adobe stock
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.
Good point, will have a look on this, probably will have to introduce a parameter to handle the component namespace.
<title value="Verify that filter is applied on page grid when filters parameter is set on url"/> | ||
<description value="Accessing page grid url with filters parameter"/> | ||
<severity value="MAJOR"/> | ||
<testCaseId value="to-be-added"/> |
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.
Test case needs to be added
@magento run all tests |
Hi @sivaschenko, thank you for the review.
|
✔️ QA Passed Manual scenario on Cucumber Studio - https://studio.cucumber.io/projects/131313/test-plan/folders/1320712/scenarios/4931106 |
@magento run all tests |
@@ -9,6 +9,7 @@ | |||
<body> | |||
<referenceContainer name="content"> | |||
<uiComponent name="cms_block_listing"/> | |||
<block class="Magento\Backend\Block\Template" template="Magento_Cms::url_filter_applier.phtml" /> |
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 think we can add Urlfilter
component in ui_component instead of creating a new block. Please check the similar implementation
https://github.com/magento/adobe-stock-integration/blob/f8f7257d30bf54646536491ad5366ae082c13570/AdobeStockImageAdminUi/view/adminhtml/ui_component/adobe_stock_images_listing.xml#L55-L70
Let me know your thoughts
Thanks
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.
@konarshankar07 I think both approaches are fine
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.
@sivaschenko ... Yes, you are right but my concerns is the namespace are hard coded in the url_filter_applier.phtml
file which can be easily exported using ui_component.
Hi @gabrieldagama, thank you for your contribution! |
Description (*)
Implemented a new component to apply filters on a grid via GET URL parameter. Added the component to the product, cms_page, and cms_block grids.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)