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

make tags lowercase before comparing them #3051

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

vinkaks
Copy link
Contributor

@vinkaks vinkaks commented Apr 16, 2023

Attempt to fix #3050. Issue results in a false error.

Issue is due to the applied tags vs user submitted tags check being case sensitive even though the database search used to generate applied tags is case insentitive.

@jiru
Copy link
Member

jiru commented Jul 29, 2023

Thanks for your PR! It appears the default collation makes varchar type case-insensitve by default, whereas the code assumes it is case-sensitve. Even though I didn’t know tags are case insensitive, we may want to keep it like this.

\App\Model\Search::filterByTags() is supposed to return the list of tags that have been successfully applied among the provided tags. Because that method is doing the SQL query, I think it would make more sense to have it handle the case thing, instead of its caller App\Form\SentencesSearchForm::setDataTags(). The caller shouldn’t have to deal with such implementation details. Can you try modifying filterByTags() in a way it works regardless of the case?

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.

Clearer message with redirection of tags in the advanced search
2 participants