https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 99556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99556&action=edit Bug 24720: Strip special chars from search fields Strip all non alphanumerical characters from beginning of search field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #2 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Maybe we need a unit test for this change -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Séverine Queune <severine.queune@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99556|0 |1 is obsolete| | --- Comment #3 from Séverine Queune <severine.queune@bulac.fr> --- Created attachment 111567 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111567&action=edit Bug 24720: Strip special chars from search fields Strip all non alphanumerical characters from beginning of search field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #5 from David Cook <dcook@prosentient.com.au> --- I agree with Frido. This behaviour should be separated into a separate function and unit tested. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- I agree with the ask for unit tests - they can be added to tests for marc_records_to_documents in t/db_dependent/Koha/SearchEngine/Elasticsearch.t Additionally, I think this should be optional in search field configs: 1 - Title already has options for 'non-filing' characters - so if you want to ignore a leading '[' you can 2 - Author names may contain real punctation, consider the band '!!!' https://en.wikipedia.org/wiki/!!! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111567|0 |1 is obsolete| | --- Comment #9 from David Gustafsson <glasklas@gmail.com> --- Created attachment 124759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124759&action=edit Bug 24720: Strip special chars from sort fields Strip all non alphanumerical characters from beginning of sort field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. 9. Run tests in t/db_dependent/Koha/SearchEngine/Elasticsearch.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 --- Comment #11 from David Gustafsson <glasklas@gmail.com> --- (In reply to David Cook from comment #6)

I will say though that my own manual texting of the regular expression looks good.

I've plugged in English, French, and Chinese into my tests and it's all working as expected. (I tried Arabic as well and I think that worked too, although I imagine Séverine could confirm that better than me.)

If we can just get those automated unit tests to prove that, then I think we'd be good to go.

I added one simple test. I first attempted to test multiple variants with different scripts but it turned out to be more effort than think is warranted for this simple line of code. Firstly, since sort field are concatenated you have to add multiple different fields with mappings for different strings to be testable (since multiple values on same field are concatenated), no big deal but is it really worth it? (Could break into separate method and test that, but then we are missing out making sure it's actually called in Koha::SearchEngine::Elasticsearch::marc_records_to_documents and don't get the same coverage, and we would be creating a method just to be able to test this piece of code in isolation, which if we where to be consistent and treat the whole code-base this way would create a mess). I don't know if an issue with my terminal/editor or something else, but when testing other scripts, like Chinese I also get error messages like "UTF-8 "\xC3" does not map to Unicode at /usr/share/perl5/MARC/File/Encode.pm line 35." before even reaching the regular expression. What we are testing is effectively be the trivial regular expression "s/^\W+//u", that is to replace all non word unicode characters at the beginning of the string with the empty string (remove them). I think the Perl core implementation regarding separating word and non-word characters in different scripts is not something that we should be writing tests for, I would trust Perl in this case. But sure, to make sure we get to that piece of code, and that the regexp does not do something completely unexpected doesn't hurt. (In reply to Nick Clemens from comment #7)

I agree with the ask for unit tests - they can be added to tests for marc_records_to_documents in t/db_dependent/Koha/SearchEngine/Elasticsearch.t

Additionally, I think this should be optional in search field configs: 1 - Title already has options for 'non-filing' characters - so if you want to ignore a leading '[' you can 2 - Author names may contain real punctation, consider the band '!!!' https://en.wikipedia.org/wiki/!!!

(In reply to David Cook from comment #8)

Good point, Nick!

There's nothing more frustrating for a library user than the search not retrieving records that you know it has...

I made a mistake describing the bug as "Strip special chars from search fields", so I can understand the confusion. It should be sort, not search fields. I have corrected this in the commit message. This fix doesn't have any impact on search fields so will not effect what is search on (the stripping of special characters for search fields is already handled in Elastic). So the band name '!!!' would still be searchable, but would probably have the same sort weight as other strings containing only non-word characters, but this is more of an edge case that will probably never have any major impact in real life situations. To for example start allowing some special characters and not others I think could be a potientially larger source of confusion, it's also hard to know how to decide which characters should be exempted. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Remove special characters |Remove special characters |from beginning of search |from beginning of sort |fields |fields -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124759|0 |1 is obsolete| | --- Comment #13 from David Gustafsson <glasklas@gmail.com> --- Created attachment 125209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125209&action=edit Bug 24720: Strip special chars from sort fields Strip all non alphanumerical characters from beginning of sort field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. 9. Run tests in t/db_dependent/Koha/SearchEngine/Elasticsearch.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 --- Comment #15 from David Gustafsson <glasklas@gmail.com> --- Rebased against master. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=26472 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125210|0 |1 is obsolete| | --- Comment #18 from David Gustafsson <glasklas@gmail.com> --- Created attachment 162674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162674&action=edit Bug 24720: Add test -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125209|0 |1 is obsolete| | --- Comment #20 from Shi Yao Wang <shi-yao.wang@inLibro.com> --- Created attachment 171988 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171988&action=edit Bug 24720: Strip special chars from sort fields Strip all non alphanumerical characters from beginning of sort field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. 9. Run tests in t/db_dependent/Koha/SearchEngine/Elasticsearch.t Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 Shi Yao Wang <shi-yao.wang@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |shi-yao.wang@inLibro.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171988|0 |1 is obsolete| | --- Comment #23 from David Gustafsson <glasklas@gmail.com> --- Created attachment 177567 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177567&action=edit Bug 24720: Strip special chars from sort fields Strip all non alphanumerical characters from beginning of sort field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. 9. Run tests in t/db_dependent/Koha/SearchEngine/Elasticsearch.t Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 --- Comment #25 from David Gustafsson <glasklas@gmail.com> --- Created attachment 177569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177569&action=edit Bug 24720: Don't strip if contains only non-word characters -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 --- Comment #26 from David Gustafsson <glasklas@gmail.com> --- (In reply to Kyle M Hall (khall) from comment #22)

...

Additionally, I think this should be optional in search field configs: 1 - Title already has options for 'non-filing' characters - so if you want to ignore a leading '[' you can 2 - Author names may contain real punctation, consider the band '!!!' https://en.wikipedia.org/wiki/!!!

I don't think this issue has been addressed

1. I think I addressed the first one previously, in that non-filing characters is not always set correctly, and at least for us, it's not realistic to fix this manually with regard to the amount of biblios affected. 2. Now changed the regexp so that special characters are only stripped if the rest of the title contains some word characters, so if only special characters are present the sort field will be left as is. Could perhaps consider adding a syspref for enabling/disabling this feature if there still are reservations, but personally unfortunately don't have time to do this right now. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177567|0 |1 is obsolete| | --- Comment #27 from David Gustafsson <glasklas@gmail.com> --- Created attachment 189767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189767&action=edit Bug 24720: Strip special chars from sort fields Strip all non alphanumerical characters from beginning of sort field values so that sorting is performed on the first alphanumeric character and not special characters like "-" or "[". To test: 1. Prepend "[" to the title of a biblio and save 2. Perform a search that matches this biblio and at least one more biblio. 3. Sort alphabetically a-z. 4. The biblio with "[" should appear first 5. Apply the patch 6. Perform a full reindexing 7. Perform steps 2-3 again 8. The result should now be alphabetically sorted, ignoring the initial bracket in the title. 9. Run tests in t/db_dependent/Koha/SearchEngine/Elasticsearch.t Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #177569|0 |1 is obsolete| | --- Comment #29 from David Gustafsson <glasklas@gmail.com> --- Created attachment 189769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189769&action=edit Bug 24720: Don't strip if contains only non-word characters -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189768|0 |1 is obsolete| | --- Comment #31 from David Gustafsson <glasklas@gmail.com> --- Created attachment 189771 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189771&action=edit Bug 24720: Add test Signed-off-by: Shi Yao Wang <shi-yao.wang@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24720 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #33 from David Gustafsson <glasklas@gmail.com> --- Rebased against master. -- You are receiving this mail because: You are watching all bug changes.