Bug 12833 - Patron search no longer searches extended attributes
Summary: Patron search no longer searches extended attributes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 9811
Blocks: 12648 19418
  Show dependency treegraph
 
Reported: 2014-08-27 14:49 UTC by Kyle M Hall
Modified: 2017-10-10 13:18 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12833: Patron search should search on extended attributes (5.47 KB, patch)
2014-08-28 16:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 12833: Patron search should search on extended attributes (5.54 KB, patch)
2014-09-03 10:39 UTC, Nick Clemens
Details | Diff | Splinter Review
[PASSED QA] Bug 12833: Patron search should search on extended attributes (5.70 KB, patch)
2014-09-08 21:32 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 2014-08-27 14:49:41 UTC
Bug 9811 introduced C4::Utils::DataTables::Members::search for searching patrons which does not use C4::Members::Search at all. This is a huge problem that will cause either duplicated code, or missing features! Case in point, the code in C4::Utils::DataTables::Members::search does not take into account searching of extended patron attributes, which introduces a regression.
Comment 1 Jonathan Druart 2014-08-28 08:43:32 UTC
I don't know how to fix that.
When should we search on the patron attributes?
If the search is done on the birth date, I suppose we don't want.
Should we add a "patron attributes" input in the search form?
Comment 2 Kyle M Hall 2014-08-28 11:15:56 UTC
Patron attributes should be part of the default patron search. The way to fix this is to remove the database queries from C4::Utils::DataTables::Members::search and use a call to C4::Members::Search instead.

(In reply to Jonathan Druart from comment #1)
> I don't know how to fix that.
> When should we search on the patron attributes?
> If the search is done on the birth date, I suppose we don't want.
> Should we add a "patron attributes" input in the search form?
Comment 3 Jonathan Druart 2014-08-28 16:23:54 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-08-28 16:25:12 UTC
(In reply to Kyle M Hall from comment #2)
> Patron attributes should be part of the default patron search. The way to
> fix this is to remove the database queries from
> C4::Utils::DataTables::Members::search and use a call to C4::Members::Search
> instead.

I will try do to that later.
Comment 5 Nick Clemens 2014-09-03 03:15:06 UTC
I wasn't sure if you were redoing the patch after Kyle's comment or not, current patch doesn't seem to change behavior, I can test again with new patch when ready
Comment 6 Jonathan Druart 2014-09-03 07:16:28 UTC
(In reply to Nick Clemens from comment #5)
> I wasn't sure if you were redoing the patch after Kyle's comment or not,
> current patch doesn't seem to change behavior, I can test again with new
> patch when ready

Hello Nick,
Could detail what does not work? I tried this patch and it seemed to work as expected.
Comment 7 Nick Clemens 2014-09-03 10:39:57 UTC Comment hidden (obsolete)
Comment 8 Nick Clemens 2014-09-03 10:40:45 UTC
My bad, I thought I had checked the box to make the attribute searchable, but apparently I had missed it.  Works now
Comment 9 Katrin Fischer 2014-09-08 21:32:12 UTC
Created attachment 31476 [details] [review]
[PASSED QA] Bug 12833: Patron search should search on extended attributes

Before Bug 9811, the patron search searched on extended attributes.

This patch restore this behavior.

Test plan:
0/ Create a patron attribute PA
1/ Create a patron A (cardnumber CNA) with PA="foo"
2/ Create a patron B (cardnumber CNB) with PA="foo bar"
3/ Search for CNA should redirect on the patron detail page.
4/ Search for "foo" should display the search result with 2 results.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
'Searchable' patron attributes can now be searched for again.
Works as described, passes stests and QA script.
Comment 10 Tomás Cohen Arazi 2014-09-09 13:54:58 UTC
Patch pushed to master.

Thanks Jonathan!