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
[TIMOB-17456]: Treat 'null' values as '' for searchableText #5949
Conversation
//Handle case sensitivity | ||
if (caseInsensitive) { | ||
searchText = searchText.toLowerCase(); | ||
searchableText = searchableText.toLowerCase(); | ||
} | ||
//String comparison | ||
if (searchableText != null && searchableText.contains(searchText)) { | ||
if (searchableText.contains(searchText)) { |
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.
Will it crash if searchableText is null?
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.
No, that's handled above, converting null to ""
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.
What if searchableText is null and searchText is ""? I think when searchableText is not specified, it means the search functionality is not enabled. No matter you want to show all the items or hide all the items in this case, you can treat this is a special situation and take care of it at the beginning so it does not need to go through all the following logic and code.
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.
Ok, I will initialize searchableText to "" instead of null.
Ran the test case with this fix. The app still crashed. |
CR and FR passed. Accepted |
[TIMOB-17456]: Treat 'null' values as '' for searchableText
testing steps in JIRA