-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Fix issue with saving filters from url-applier to default view of bookmark #29861
Conversation
Hi @Nazar65. 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. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento run all tests |
Hi @sivaschenko. Just want to double-check that we are on the same page. Thank you. |
@rogyar failing test is not related to the changes in this pr, also this behaviour already covered by mftf test. |
Hi @Nazar65. Thank you for the clarification. If this assumption is correct for the current PR, that's great. Thanks |
@sivaschenko , @Nazar65 could you please clarify the automation status? I agree with @rogyar , if we are going to fix something- it should be covered with a test. If test is already created then why it's green since we have a bug in functionality? |
@sidolov this pr fixes bug with js initialization, so covering such functionality with tests is no good idea, because we will introduce one more flaky test, and in this case the ui_bookmark table also needs to be empty. |
@sidolov @rogyar the PR contains an improvement to stability of the functionality that is covered by MFTF tests. In case there is a PR that contains refactoring or implementation change to existing functionality that is covered by MFTF tests, such PR is covered with tests even if it does not change the test results. |
@magento run all tests |
Hi @sivaschenko, thank you for the review. |
✔️ QA Passed The Default view doesn't contain any bookmarks, it is empty as expected Manual testing scenario - https://studio.cucumber.io/projects/131313/test-plan/folders/1320712/scenarios/5115557 |
Already merged to 2.4.1-develop. |
…t view of bookmark #29861
Hi @Nazar65, thank you for your contribution! |
Description (*)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Open media gallery
Open view details from image
Click on Used in Categories link
Category grid rendered with url-filter applier, and asset filter applied
Default view of bookmark saved without filter
Questions or comments
Contribution checklist (*)