-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix admin grid filter backslash escaping issue #39535
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
base: 2.4-develop
Are you sure you want to change the base?
Fix admin grid filter backslash escaping issue #39535
Conversation
Hi @wubinworks. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@magento run all tests |
…ckslash-escaping-issue
@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.
Hello @wubinworks,
Thanks for the contribution!
The code changes seems good to us, but please look into the following review comments:
- Fix the static test failures, other failures seems flaky to me. Hence re-running the tests.
- Consider adding some automated tests like unit or integration which verifies filtering behavior with backslash characters. Specifically:
- Unit tests could ensure Filter::applyFilter() properly handles backslashes
- Integration tests could verify admin grid filtering works with values containing backslashes
Thanks
@wubinworks I wanted to check if you are working on the static failures & automated test coverage. If not, Can I take care of them? Just let me know what works best for you. |
@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.
Hello @wubinworks,
Thanks for making the changes!
Some static tests are still failing, please fix those as well.
Thanks
Fixed admin grid filter backslash escaping issue.
Description (*)
The Admin Grid Filter cannot return correct result for keyword that contains backslash. For example
Magento\Store
.This pull request fixed it by escaping backslash.
Related Pull Requests
Fixed Issues (if relevant)
listing component
cannot hit when field value contains\
#39513Manual testing scenarios (*)
Magento\Store
.Magento\Store
in title filter to search and then check the search result.Magento\Store
after merging this branch.Questions or comments
Contribution checklist (*)