-
Notifications
You must be signed in to change notification settings - Fork 312
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: Facet returns no result for a term having accent characters #3031
Conversation
Facets e2e tests are consistently failing in this one @burhandodhy. Do you mind giving a look before we merge it? Thanks! |
@felipeelia Tests are fixed. The problem was that the |
@burhandodhy after giving this one a deeper look, it seems we might be going in the wrong direction. Instead of fixing the sanitization of the slug, your new code actually showed me we have a different problem: If we create a post with a meta field with that same value, the meta facet will generate a URL with a different value than the taxonomy: https://ep-es7.test/?ep_filter_category=%25d8%25ba%25d9%258a%25d8%25b1-%25d9%2585%25d8%25b5%25d9%2586%25d9%2581&ep_meta_filter_my_first_field=%D8%BA%D9%8A%D8%B1-%D9%85%D8%B5%D9%86%D9%81 (see how Investigating it a little bit, it seems things work well if we use |
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.
@burhandodhy great progress on this one, thanks. I've left yet another comments/suggestions/changes here, let me know your thoughts about it.
One thing that I'm trying to do is to add unit tests for all filters/actions being called. Do you think it would be possible to add unit tests for these new filters as well? Thanks!
ps.: there is also a conflict with the facets e2e tests, if you don't mind fixing it.
@felipeelia I made the changes that you requested. I also added the tests for the 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.
We are almost there, @burhandodhy!
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.
Thanks for all the hard work on this one @burhandodhy!
Description of the Change
This PR fixes the issue where the facet returns no result for a term having accent characters
Closes #3023
How to test the Change
غير-مصنف
غير-مصنف
from taxonomy widgetغير-مصنف
categoryChangelog Entry
Credits
Props @burhandodhy
Checklist: