Bug 13822 - Patron autocomplete search is severly limited
Summary: Patron autocomplete search is severly limited
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13892
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-11 14:47 UTC by Kyle M Hall
Modified: 2016-06-21 21:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13822 - Patron autocomplete search is severly limited (2.66 KB, patch)
2015-03-11 14:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13822 - Patron autocomplete search is severly limited (2.68 KB, patch)
2015-03-19 14:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
[Signed-off] Bug 13822 - Patron autocomplete search is severly limited (2.86 KB, patch)
2015-03-20 09:49 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 13822 [QA Followup] - Make search do a 'starts with' search (753 bytes, patch)
2015-03-24 11:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
[Signed-off] Bug 13822 [QA Followup] - Make search do a 'starts with' search (1.02 KB, patch)
2015-03-24 13:09 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 13822 - Patron autocomplete search is severly limited (2.17 KB, patch)
2015-03-27 15:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13822 - Patron autocomplete search is severly limited (2.32 KB, patch)
2015-03-27 16:00 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 13822 - Patron autocomplete search is severly limited (2.39 KB, patch)
2015-04-06 10:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-03-11 14:47:32 UTC
The script that returns data for the patron autocomplete does not use C4::Members::Search. Instead it uses bespoke code that does not behave like the traditional search and is very limited in how it can search.

If, for example, I search for "hall, k" in the standard search, I would get "Kyle Hall" back as a result. For the autocomplete search, I will not.

This script should use C4::Members::Search to provide better searching and to keep the code base DRY.
Comment 1 Kyle M Hall 2015-03-11 14:58:19 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-03-13 05:12:38 UTC
Tested for signing off, search behaviour is much better now!

However, I propose to keep sorting, something like:

my $order_by = 'surname, firstname';
my ($results) = Search( $term, $order_by, [ 10 ] );
Comment 3 Kyle M Hall 2015-03-19 14:02:24 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-03-19 14:03:03 UTC
Done!

(In reply to Marc Véron from comment #2)
> Tested for signing off, search behaviour is much better now!
> 
> However, I propose to keep sorting, something like:
> 
> my $order_by = 'surname, firstname';
> my ($results) = Search( $term, $order_by, [ 10 ] );
Comment 5 Marc Véron 2015-03-20 09:49:30 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-03-22 19:32:31 UTC
I am not sure about this change. With the patch applied a search for sch will bring up "Fischer" - so it does a 'contains' search. I think on a big system this will make the autocomplete harder to handle - it might bring up a lot of unrelated searches. There might be reasons to make the autocomplete more strict than the normal patron search, where you can pick your options.
Comment 7 Marc Véron 2015-03-23 16:46:11 UTC
I did a short test with following line:

my ($results) = Search( $term, $order_by, [ 10 ],'','','start_with' );

This way, the search would be more restricted.

To do a 'contains' search, the user can still use the wildcard character %

'Test' would give
Testmann, Fritz
Tester, Hans

'Hans' would give
Meier, Hans
Müller, Hansli


'%est' would give
Hatfield, Lester 
Testman, Fritz
Tester, Hans
West, Mae

My proposition is to have the 'start_with' version and change the explanation from:
Enter patron card number or partial name:
to something like:
Enter first digits/letters of patron card number or name. Prepend % for inline search.
Comment 8 Katrin Fischer 2015-03-23 16:53:16 UTC
Hm... the actual search matches the text, but the autocomplete is a different thing... I am not sure about this here. Would be good to test on a big database maybe. Maybe one of the bigger libraries could weigh in how they expect this to work?
Comment 9 Kyle M Hall 2015-03-24 11:29:11 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2015-03-24 13:09:11 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2015-03-27 15:22:48 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2015-03-27 15:25:30 UTC
Patch rewritten to take into account bug 13892. Completely new version will need a new sign off.
Comment 13 Marc Véron 2015-03-27 16:00:41 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2015-04-06 09:59:30 UTC
This works ok, I am seeing a small issue with and without the patch - filed bug 13957 for it.
Comment 15 Katrin Fischer 2015-04-06 10:01:34 UTC
Created attachment 37522 [details] [review]
[PASSED QA] Bug 13822 - Patron autocomplete search is severly limited

The script that returns data for the patron autocomplete does not use C4::Members::Search. Instead it uses bespoke code that does not behave like the traditional search and is very limited in how it can search.

If, for example, I search for "Kyle Hall" in the standard search, I would get "Kyle Hall" back as a result. For the autocomplete search, I will not.

This script should use C4::Members::Search to provide better searching and to keep the code base DRY.

Test Plan:
1) Enable the system preference CircAutocompl
2) Create a user with the first name "Test" and the surname "User"
3) Perform a checkout autocomplete search for "Test User"
4) Note you do not get the user as a result
5) Apply this patch
6) Try different combinations of "Test" and "User" such as
   Test User
   User Test
   U Test
   Test U
   etc.
7) Note these searches now work

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Tomás Cohen Arazi 2015-05-14 14:18:55 UTC
Patch pushed to master.

Thanks Kyle!

tatus: Pushed to Master
Comment 17 Tom Misilo 2015-09-16 17:27:28 UTC
Is there any way to get this included in 3.18?