Bug 18094

Summary: Patron search filters are broken by searchable attributes
Product: Koha Reporter: Nick Clemens <nick>
Component: PatronsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: brendan, esiasclib2016, gmcharlt, jonathan.druart, julian.maurice, katrin.fischer, koha, kyle.m.hall, mtj, patrick.robitaille, veron
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 18094: Add tests to highlight the problem
Bug 18094: Only search in searchable patron attributes if searching in standard fields
Bug 18094: Add tests to highlight the problem
Bug 18094: Only search in searchable patron attributes if searching in standard fields
Bug 18094: Add tests to highlight the problem
Bug 18094: Only search in searchable patron attributes if searching in standard fields
[16.05.x] Bug 18094: Add tests to highlight the problem
[16.05.x] Bug 18094: Only search in searchable patron attributes if searching in standard fields

Description Nick Clemens 2017-02-10 14:32:14 UTC
When using searchable patron attributes they seem to be added to all searches as a contain

To recreate:
Add a new patron attrbute and mark it searchable
Populate a new patron with 'potato' in that field
Add/edit another patron to have email potato@invalidemail.com'
Perform a patron search with filters 'Email' and 'Starts with' and query 'potato'
You will see both patrons in your results
Remove the potato email
Perform search again
You will be directed to the patron containing the potato attribute
Comment 1 Jonathan Druart 2017-02-14 15:25:27 UTC
Created attachment 60212 [details] [review]
Bug 18094: Add tests to highlight the problem
Comment 2 Jonathan Druart 2017-02-14 15:25:31 UTC
Created attachment 60213 [details] [review]
Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.
Comment 3 Marc Véron 2017-03-15 18:03:27 UTC
Created attachment 61111 [details] [review]
Bug 18094: Add tests to highlight the problem

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 4 Marc Véron 2017-03-15 18:04:25 UTC
Created attachment 61112 [details] [review]
Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 5 Nick Clemens 2017-03-20 12:49:19 UTC
Created attachment 61291 [details] [review]
Bug 18094: Add tests to highlight the problem

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2017-03-20 12:49:26 UTC
Created attachment 61292 [details] [review]
Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Brendan Gallagher 2017-03-23 12:15:53 UTC
Pushed to Master - Should be in the 17.05 Release.  Thanks
Comment 8 Katrin Fischer 2017-03-26 20:53:47 UTC
These patches have been pushed to 16.11.x and will be in 16.11.06
.
Comment 9 Julian Maurice 2017-03-31 13:17:34 UTC
Pushed to 3.22.x, will be in 3.22.19
Comment 10 Mason James 2017-05-03 03:43:14 UTC
Pushed to 16.05.x, for 16.05.12 release
Comment 11 Jonathan Druart 2017-05-03 14:24:39 UTC
Created attachment 63015 [details] [review]
[16.05.x] Bug 18094: Add tests to highlight the problem

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Jonathan Druart 2017-05-03 14:25:01 UTC
Created attachment 63016 [details] [review]
[16.05.x] Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Mason James 2017-05-03 14:37:57 UTC
this patchset currently fails for me on 16.05.x

$ prove  t/db_dependent/Utils/Datatables_Members.t
t/db_dependent/Utils/Datatables_Members.t .. 1/30
    #   Failed test ''Dupont' is contained in 2 surnames and a patron attribute. Patron attribute one should not be displayed if searching in specific fields (Bug 18094)'
    #   at t/db_dependent/Utils/Datatables_Members.t line 407.
    #          got: '2'
    #     expected: '1'
    # Looks like you failed 1 test of 2.

#   Failed test 'ExtendedPatronAttributes'
#   at t/db_dependent/Utils/Datatables_Members.t line 409.
# Looks like you failed 1 test of 30.
t/db_dependent/Utils/Datatables_Members.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/30 subtests

Test Summary Report
-------------------
t/db_dependent/Utils/Datatables_Members.t (Wstat: 256 Tests: 30 Failed: 1)
  Failed test:  30
  Non-zero exit status: 1
Files=1, Tests=30,  2 wallclock secs ( 0.03 usr  0.01 sys +  1.74 cusr  0.22 csys =  2.00 CPU)
Result: FAIL
Comment 14 Jonathan Druart 2017-05-03 14:45:13 UTC
You need the 2 patches, not just the tests.
Comment 15 Koha Team University Lyon 3 2017-06-08 12:10:35 UTC
I've tried to use this in 3.22.20 but I can't manage to search on a attribute that I've marked as searchable.
I'm wondering if I miss something..
After checking "searchable" for an attribute, could I launch a search directly or should I do another action (restart plack, modify my patron using this attribute ?)
Attributes are searchable in the "standard search fields" ?
Comment 16 Jonathan Druart 2017-06-08 13:25:05 UTC
(In reply to Koha Team Lyon 3 from comment #15)
> I've tried to use this in 3.22.20 but I can't manage to search on a
> attribute that I've marked as searchable.
> I'm wondering if I miss something..
> After checking "searchable" for an attribute, could I launch a search
> directly or should I do another action (restart plack, modify my patron
> using this attribute ?)
> Attributes are searchable in the "standard search fields" ?

Nope you do not have to do anything like that.
Attributes are only searchable in standard search fields.
Tested ok on 3.22.21.
Comment 17 Koha Team University Lyon 3 2017-06-08 14:38:08 UTC
Thanks Jonathan !
Ok, I got it. I have tried with attributes which have Authorized value category. I tryed to search with the description (which I can read in patron detail) but in fact the search only works with Authorized value Code !

I will open a new bug for this.

Sonia
Comment 18 Jonathan Druart 2017-06-08 15:17:04 UTC
Ha yes indeed, good catch.
Comment 19 Jonathan Druart 2017-06-08 15:18:31 UTC
At first glance I'd say it will not that easy to fix and may be a performance killer change.