-
Notifications
You must be signed in to change notification settings - Fork 203
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
EZP-31287: Added FullText search Fields for Email and ISBN #3023
Conversation
It's not searchable using FullText Criterion to be precise. What is the use case here? |
Use case: Be able to find the data using search (typically in backend, but in this case frontend might also be relevant). Similar to the one we discussed in user (login / email), in that case typically for admins in backend, given default permissions. |
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.
@alongosz yeah, you are right, besides providing whole e-mail and finding invalid Contents with parts of email string, Solr and Legacy search results are also a bit inconsistent with each other. I'll try to dig more into it later on, it's probably much more complicated. Shouldn't we have a different ticket for this e-mail part searching, though? This issue was occurring even before this PR has been created. |
@alongosz may I know what kind of tests am I missing? The |
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.
Good direction, needs some changes though.
eZ/Publish/Core/Search/Legacy/Content/Mapper/FullTextMapper.php
Outdated
Show resolved
Hide resolved
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.
Almost good, final CS remark for internal impl.
eZ/Publish/Core/Search/Legacy/Content/Mapper/FullTextMapper.php
Outdated
Show resolved
Hide resolved
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.
QA approved on eZPlatform-ee 2.5 with diff.
You can merge it up. |
Merged into 1.1: ezsystems/ezplatform-kernel@04bde0e |
Email address
andISBN
fieldtypes were not searchable due to lackingFullTextField
search field assignment.Also citing @alongosz from the related PR comment (#2206 (comment)):
The e-mail address was split into parts when indexing, that's why it was behaving like that. In order to prevent such behavior for chosen field types this specific commit has been created: 59859e8 which handles such case and is easily adaptable for other field types.
// @alongosz note:
QA
Besides tests for LSE, do a regression for Solr as well please, because in the past I had some unexpected results there with this task. Should not be affected though because changes are related to LSE only. But... you know ;)
// -- end of the note
// author's note:
Old parts of the e-mail string will still be able to be found when searching for the full e-mail address as they were indexed like that previously, before this change. Example:
eZ Platform
- foldersupport@ez.no
- e-mailWhen searching for
support@ez.no
one will find eZ Platform folder as well.TODO:
$ composer fix-cs
).