[Bug 30645] New: Patron search not building the query correctly for extended attributes if several terms passed
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Bug ID: 30645 Summary: Patron search not building the query correctly for extended attributes if several terms passed Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Depends on: 30639 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30639 [Bug 30639] Patron search does not split search terms -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 134253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134253&action=edit Bug 30645: Add tests to highlight the problem -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33554 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Jonathan is this still the case? i.e. is the patron search still not building the query correctly for extended attributes if several terms passed? If so, the tests patch doesn't apply anymore. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134253|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 154949 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154949&action=edit Bug 30645: Add tests to highlight the problem -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Yes, this is still valid. Tests rebased. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Same issue happens when searching ILL requests but with illrequestattributes instead. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 159414 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159414&action=edit Bug 30645: Consider multiple instances of extended_attributes in query params This is a POC but its fully functional afaik. Im not 100% happy with the code as Im addressing the 3 possible related metadata tables individually. Im also not sure if this is the best place to put this code, please advise. Testing in the UI, this is fixing the patron search and the licenses additional fields search (requires bug 35287 applied) ILL is still not working, but I think its because the query builder in the javascript is not breaking apart multiple search words, in other words, this fix should also fix ILL illrequestattributes once the javascript is looked at. Joubu can you please rebase your test patch? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion Assignee|koha-bugs@lists.koha-commun |pedro.amorim@ptfs-europe.co |ity.org |m CC| |katrin.fischer@bsz-bw.de, | |matt.blenkinsop@ptfs-europe | |.com, tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- After some consideration, I think we should put this into the API plugin side.. so REST/Plugin/Query.pm We already have the Mixin which does this for additional_fields with a filter_by_ method... though I'm not sure how well advertised/documented that is.. and I think it may well confuse object consumers later having this level of magic in the base class search method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154949|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 159450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159450&action=edit Bug 30645: Add tests to highlight the problem -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Pedro Amorim from comment #6)
Joubu can you please rebase your test patch?
Done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32755 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32755 [Bug 32755] Make it possible to show additional fields for baskets in acq search results -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35287 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35287 [Bug 35287] Add additional fields support to ERM Licenses -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Patron search not building |API searches across joined |the query correctly for |extended attributes |extended attributes if |database query is built |several terms passed |incorrectly when several | |terms are passed Severity|normal |major -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|API searches across joined |Generated DBIC query |extended attributes |incorrect for api searches |database query is built |across joined extended |incorrectly when several |attributes when several |terms are passed |terms are passed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159414|0 |1 is obsolete| | --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 159651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159651&action=edit Bug 30645: Consider multiple instances of extended_attributes in query params This will rewrite a query like: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes.value":{ "like":"123%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } To: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes_2.value":{ "like":"123%" }, "extended_attributes_2.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } And it'll also add the number of required 'join' attributes to the query attributes to match it. Test plan (with Joubu's test patch applied), run: prove t/db_dependent/selenium/patrons_search.t Or Test plan, k-t-d: 1) Create 2 patron attributes, visit: /cgi-bin/koha/admin/patron-attr-types.pl 2) Name the first attribute 'test1' and the second attribute 'test2' 3) On both, tick the 'Searchable' and 'Search by default' checkboxes 4) Edit a koha borrower, example visit: /cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 5) Put '123' on the test1 attribute, and 'abc' on the test2 attribute 6) Visit the main patrons page: /cgi-bin/koha/members/members-home.pl 7) On the first left side filter input called 'Search', type '123' and search. Notice you get the patron result 8) Repeat step 7, but type 'abc' instead of '123'. Notice you get the patron result 9) Now do the same search but type '123 abc'. Notice you do not get the patron result. 10) Apply patch. Repeat test plan. Notice you get the patron result in step 9). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159651|0 |1 is obsolete| | --- Comment #11 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 159652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159652&action=edit Bug 30645: Consider multiple instances of extended_attributes in query params This will rewrite a query like: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes.value":{ "like":"123%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } To: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes_2.value":{ "like":"123%" }, "extended_attributes_2.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } And it'll also add the number of required 'join' attributes to the query attributes to match it. Test plan (with Joubu's test patch applied), run: prove t/db_dependent/selenium/patrons_search.t Or Test plan, k-t-d: 1) Create 2 patron attributes, visit: /cgi-bin/koha/admin/patron-attr-types.pl 2) Name the first attribute 'test1' and the second attribute 'test2' 3) On both, tick the 'Searchable' and 'Search by default' checkboxes 4) Edit a koha borrower, example visit: /cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 5) Put '123' on the test1 attribute, and 'abc' on the test2 attribute 6) Visit the main patrons page: /cgi-bin/koha/members/members-home.pl 7) On the first left side filter input called 'Search', type '123' and search. Notice you get the patron result 8) Repeat step 7, but type 'abc' instead of '123'. Notice you get the patron result 9) Now do the same search but type '123 abc'. Notice you do not get the patron result. 10) Apply patch. Repeat test plan. Notice you get the patron result in step 9). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159450|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 159675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159675&action=edit Bug 30645: Add tests to highlight the problem Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159652|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 159676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159676&action=edit Bug 30645: Consider multiple instances of extended_attributes in query params This will rewrite a query like: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes.value":{ "like":"123%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } To: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes_2.value":{ "like":"123%" }, "extended_attributes_2.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } And it'll also add the number of required 'join' attributes to the query attributes to match it. Test plan (with Joubu's test patch applied), run: prove t/db_dependent/selenium/patrons_search.t Or Test plan, k-t-d: 1) Create 2 patron attributes, visit: /cgi-bin/koha/admin/patron-attr-types.pl 2) Name the first attribute 'test1' and the second attribute 'test2' 3) On both, tick the 'Searchable' and 'Search by default' checkboxes 4) Edit a koha borrower, example visit: /cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 5) Put '123' on the test1 attribute, and 'abc' on the test2 attribute 6) Visit the main patrons page: /cgi-bin/koha/members/members-home.pl 7) On the first left side filter input called 'Search', type '123' and search. Notice you get the patron result 8) Repeat step 7, but type 'abc' instead of '123'. Notice you get the patron result 9) Now do the same search but type '123 abc'. Notice you do not get the patron result. 10) Apply patch. Repeat test plan. Notice you get the patron result in step 9). Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes patron searching release notes| |where there are multiple | |patron attributes - all | |patron attributes are now | |searched. Before this, if | |your search included values | |from multiple patron | |attributes, no results | |would be found. Example: if | |you have two patron | |attributes with the value | |'abc' in one and '123' in | |another, searching for | |either one worked as | |expected, but no results | |would be found searching | |for 'abc 123'. CC| |david@davidnind.com --- Comment #14 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. To run the tests, start up KTD with selenium: ktd --selenium up 2. I ran the tests (these passed) and added patron attributes to test that patron search works as expected. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | Status|Signed Off |Failed QA --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, this is great but we need lower level tests, for the new mojo helper. There are several example tests. We can take a look together on monday if you feel like. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Generated DBIC query |Generated DBIC query |incorrect for api searches |incorrect for API searches |across joined extended |across joined extended |attributes when several |attributes when several |terms are passed |terms are passed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #16 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Tomás Cohen Arazi from comment #15)
Hi, this is great but we need lower level tests, for the new mojo helper. There are several example tests. We can take a look together on monday if you feel like.
Hi, yes please. Help is greatly appreciated, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #17 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 160208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160208&action=edit Bug 30645: Add REST/Plugin/Query tests prove t/Koha/REST/Plugin/Query.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160208|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 160216 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=160216&action=edit Bug 30645: Add REST/Plugin/Query tests prove t/Koha/REST/Plugin/Query.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- WARN t/db_dependent/selenium/patrons_search.t WARN tidiness The file is less tidy than before (bad/messy lines before: 91, now: 100) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #20 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 161495 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161495&action=edit Bug 30645: (QA follow-up): Tidy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Please do not tidy the whole file, bug 35329 is on its way to master and it's going to conflict. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I guess performances (see bug 33554) will be even worst with this change :-/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161495|0 |1 is obsolete| | --- Comment #23 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 161528 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161528&action=edit Bug 30645: (QA follow-up): Tidy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #24 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #22)
I guess performances (see bug 33554) will be even worst with this change :-/
I've done some testing myself and cannot find evidence of this. Clean k-t-d (before applying patches): 1) Visit patrons home: http://localhost:8081/cgi-bin/koha/members/members-home.pl 2) Open network tools and click "Search" 3) Do this several times (like 10), take note of the average time it takes for the request to come back (for me, averaging around 185-195ms) 4) Run the following command (!!DEV KTD ONLY!!): bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/scripts/dev-dat...) 30000 5) If all goes well, the above command will add 30.000 patrons, 300 searchable borrower_attribute_types and 30.000 borrower_attributes. It may take a few minutes to finish, but required to test performance. 6) After 5) is done. Repeat 3) Notice it now takes a while longer (for me, its now averaging aroudn 225ms) 7) Apply patches from 30645, restart plack and flush_memcached 8) Notice if there's any difference, for me, I'm averaging about the same 225ms as before. 9) Do more testing, search for searchable fields specifically, having tried that myself I don't see any impact. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I don't believe this makes the query any slower, and this is a bugfix whilst the other is a performance enhancement. I think this is important as is and if we want to introduce an entirely new route for 'lookup', then that should be judged and used on its own merit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36025 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161528|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #26 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #9)
(In reply to Pedro Amorim from comment #6)
Joubu can you please rebase your test patch?
Done.
Sorry Joubu, can you please rebase your test patch again? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159675|0 |1 is obsolete| | --- Comment #27 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 164982 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164982&action=edit Bug 30645: Add tests to highlight the problem Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off --- Comment #28 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Pedro Amorim from comment #26)
(In reply to Jonathan Druart from comment #9)
(In reply to Pedro Amorim from comment #6)
Joubu can you please rebase your test patch?
Done.
Sorry Joubu, can you please rebase your test patch again?
Done, it was painful and boring! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159676|0 |1 is obsolete| | --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165009&action=edit Bug 30645: Consider multiple instances of extended_attributes in query params This will rewrite a query like: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes.value":{ "like":"123%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } To: { "-and":[ [ { "extended_attributes.value":{ "like":"abc%" }, "extended_attributes.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ], [ { "extended_attributes_2.value":{ "like":"123%" }, "extended_attributes_2.code":[ [ "arbitrary_attr_code", "another_attr_code" ] ] } ] ] } And it'll also add the number of required 'join' attributes to the query attributes to match it. Test plan (with Joubu's test patch applied), run: prove t/db_dependent/selenium/patrons_search.t Or Test plan, k-t-d: 1) Create 2 patron attributes, visit: /cgi-bin/koha/admin/patron-attr-types.pl 2) Name the first attribute 'test1' and the second attribute 'test2' 3) On both, tick the 'Searchable' and 'Search by default' checkboxes 4) Edit a koha borrower, example visit: /cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51 5) Put '123' on the test1 attribute, and 'abc' on the test2 attribute 6) Visit the main patrons page: /cgi-bin/koha/members/members-home.pl 7) On the first left side filter input called 'Search', type '123' and search. Notice you get the patron result 8) Repeat step 7, but type 'abc' instead of '123'. Notice you get the patron result 9) Now do the same search but type '123 abc'. Notice you do not get the patron result. 10) Apply patch. Repeat test plan. Notice you get the patron result in step 9). Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #160216|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165010&action=edit Bug 30645: Add REST/Plugin/Query tests prove t/Koha/REST/Plugin/Query.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164982|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165011&action=edit Bug 30645: Add tests to highlight the problem Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We've been using this fix in production for some time now.. it would be good to get a final QA round on it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This has been around for too long and really must be part of the next release and preferably ported back too. Setting to PQA.. it's had testing and feedback. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I had a little trouble here with * Proving /kohadevbox/koha/t/db_dependent/selenium/patrons_search.t KO! But this test is currently failing for me on master as well, right after a reset_all. Help appreciated! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I'll push this one as it doesn't seem to be the cause of the breaking test, but we should definitely check on the test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #37 from Fridolin Somers <fridolin.somers@biblibre.com> --- Backport on 23.11.x fails on t/db_dependent/selenium/patrons_search.t Please create patches for 23.11.x if possible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #38 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'm not sure if this is a problem with the ILL requests DT only, but I have:
SELECT COUNT(*) FROM illrequests; 54 SELECT COUNT(*) FROM illrequestattributes; 754
That'd be -roughly- around 13/14 different attributes per request. Using the DT filter box for searching "fra" is generating this query: ```sql SELECT COUNT( * ) FROM (SELECT `me`.`illrequest_id` FROM `illrequests` `me` LEFT JOIN `illrequestattributes` `extended_attributes` ON `extended_attributes`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_2` ON `extended_attributes_2`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_3` ON `extended_attributes_3`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_4` ON `extended_attributes_4`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_5` ON `extended_attributes_5`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_6` ON `extended_attributes_6`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_7` ON `extended_attributes_7`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illrequestattributes` `extended_attributes_8` ON `extended_attributes_8`.`illrequest_id` = `me`.`illrequest_id` LEFT JOIN `illbatches` `ill_batch` ON `ill_batch`.`ill_batch_id` = `me`.`batch_id` JOIN `branches` `library` ON `library`.`branchcode` = `me`.`branchcode` LEFT JOIN `borrowers` `patron` ON `patron`.`borrowernumber` = `me`.`borrowernumber` WHERE ( ( ( `me`.`cost` LIKE '%fra%' OR `me`.`placed` LIKE '%fra%' OR `me`.`status` LIKE '%fra%' OR `me`.`backend` LIKE '%fra%' OR `me`.`orderid` LIKE '%fra%' OR `me`.`replied` LIKE '%fra%' OR `me`.`updated` LIKE '%fra%' OR `me`.`accessurl` LIKE '%fra%' OR `me`.`biblio_id` LIKE '%fra%' OR `me`.`completed` LIKE '%fra%' OR `me`.`notesopac` LIKE '%fra%' OR `me`.`notesstaff` LIKE '%fra%' OR `me`.`price_paid` LIKE '%fra%' OR `ill_batch`.`name` LIKE '%fra%' OR `me`.`illrequest_id` LIKE '%fra%' OR `library`.`branchname` LIKE '%fra%' OR ( `extended_attributes_8`.`type` = 'type' AND `extended_attributes_8`.`value` LIKE '%fra%' ) OR ( `extended_attributes_6`.`type` = 'year' AND `extended_attributes_6`.`value` LIKE '%fra%' ) OR ( `extended_attributes_4`.`type` = 'issue' AND `extended_attributes_4`.`value` LIKE '%fra%' ) OR ( `extended_attributes_7`.`type` = 'pages' AND `extended_attributes_7`.`value` LIKE '%fra%' ) OR ( `extended_attributes_2`.`type` = 'title' AND `extended_attributes_2`.`value` LIKE '%fra%' ) OR ( `extended_attributes`.`type` = 'author' AND `extended_attributes`.`value` LIKE '%fra%' ) OR ( `extended_attributes_5`.`type` = 'volume' AND `extended_attributes_5`.`value` LIKE '%fra%' ) OR ( `extended_attributes_3`.`type` = 'article_title' AND `extended_attributes_3`.`value` LIKE '%fra%' ) OR `patron`.`surname` LIKE '%fra%' OR `patron`.`firstname` LIKE '%fra%' OR `patron`.`cardnumber` LIKE '%fra%' ) AND `status` NOT IN ( 'COMP' ) AND ( `status_alias` NOT IN ( 'COMP' ) OR `status_alias` IS NULL ) ) ) GROUP BY `me`.`illrequest_id`) `me`; ``` which kills MariaDB \o/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #39 from Liz Rea <wizzyrea@gmail.com> --- It definitely hangs up the mariadb threads in a way that never seems to complete: | 46 | koha_reach2 | localhost | koha_reach2 | Query | 82188 | Sending data | SELECT COUNT( * ) FROM (SELECT `me`.`illrequest_id` FROM `illrequests` `me` LEFT JOIN `illrequestatt | 0.000 | sorry I didn't get the full query for this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |37389 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37389 [Bug 37389] ILL requests table hangs when searching -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 --- Comment #40 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Liz Rea from comment #39)
It definitely hangs up the mariadb threads
(In reply to Tomás Cohen Arazi from comment #38)
Using the DT filter box for searching "fra" is generating this query which kills MariaDB \o/
Opened bug 37389. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable CC| |phil@chetcolibrary.org --- Comment #41 from Phil Ringnalda <phil@chetcolibrary.org> --- (In reply to Martin Renvoize (ashimema) from comment #33)
and preferably ported back too.
Passage of time, it's on stable now, but... (In reply to Fridolin Somers from comment #37)
Backport on 23.11.x fails on t/db_dependent/selenium/patrons_search.t Please create patches for 23.11.x if possible.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Chip Halvorsen <Chip.Halvorsen@WestlakeLibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Chip.Halvorsen@WestlakeLibr | |ary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|35287 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35287 [Bug 35287] Add additional fields support to ERM licenses -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org