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.
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?
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?
Created attachment 31263 [details] [review] 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.
(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.
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
(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.
Created attachment 31335 [details] [review] [SIGNED OFF] 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>
My bad, I thought I had checked the box to make the attribute searchable, but apparently I had missed it. Works now
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.
Patch pushed to master. Thanks Jonathan!