Bug 36025

Summary:	Extended attributes clause added to patron search query even when there are no searchable attributes
Product:	Koha	Reporter:	Andreas Jonsson <andreas.jonsson>
Component:	Patrons	Assignee:	Andreas Jonsson <andreas.jonsson>
Status:	Patch doesn't apply ---	QA Contact:	Testopia <testopia>
Severity:	minor
Priority:	P5 - low	CC:	gmcharlt, kyle.m.hall, kyle, martin.renvoize, nick, pedro.amorim, tomascohen
Version:	23.11
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34519 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35361 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30645
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 36025 Selenium test for the query parameters Bug 36025 Incorrect clause in patron search Bug 36025: Selenium test for the query parameters Bug 36025: Incorrect clause in patron search

Description Andreas Jonsson 2024-02-07 20:16:26 UTC

In bug 34519 there is a commit labelled "ensure we always have an array":

-            [% SET extended_attribute_types = ExtendedAttributeTypes.codes( staff_searchable => 1 ) %]
+            [% SET extended_attribute_types = [ ExtendedAttributeTypes.codes( staff_searchable => 1 ) ] %]

Somehow this expands into an array containing the empty string as an element if no attributes are searchable by default.

This is likely a bug in DBIx::Class.  It would be better to add the workaround for this in the ExtendedAtributeTypes template plugin.

The get_column method of ResultSet returns values of type ResultSetColumn according to the documentation.  But in reality it does not.  It returns an array.

So we must rely on a bug already by using this method.  Consistently using it in list context might be helpful as a workaround.

sub codes {
    my ( $self, $params ) = @_;

    my @codes = Koha::Patron::Attribute::Types->search($params)->get_column('code');

    return \@codes;
}

The consequence is that a clause for extended attributes is added to the patron search query.  This is significantly slower than not having such a clause, due to some other issues with DBIx::Class.  (Se bug 33554 and bug 35361.)

Comment 1 Andreas Jonsson 2024-02-08 01:33:20 UTC

There is also another issue in buildPatronSearchQuery that still adds the clause. Someone is trying to use an array as a boolean value in Javascript.  (A work related brain injury caused by excessive Perl programming?)

--- a/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
@@ -650,7 +650,7 @@ function buildPatronSearchQuery(term, options) {
     q.push({ "-or": term_subquery_or });
 
     // Add each pattern for each extended patron attributes
-    if ( typeof options !== 'undefined' && ( (options.search_fields == 'standard' && options.extended_attribute_types) || ( searched_attribute_fields.length > 0 ) ) && extendedPatronAttributes) {
+    if ( typeof options !== 'undefined' && ( (options.search_fields == 'standard' && options.extended_attribute_types.length > 0) || ( searched_attribute_fields.length > 0 ) ) && extendedPatronAttributes) {
         extended_attribute_codes_to_search = (searched_attribute_fields.length > 0) ? searched_attribute_fields : options.extended_attribute_types;
         extended_attribute_subquery_and = [];
         patterns.forEach(function (pattern, i) {

Comment 2 Andreas Jonsson 2024-02-08 01:35:08 UTC

Created attachment 161873 [details] [review]
Bug 36025 Selenium test for the query parameters

The test verifies that there is no clause for extended
attributes in the patron search query when no patron
attribute is searchable by default.

Comment 3 Andreas Jonsson 2024-02-08 01:35:10 UTC

Created attachment 161874 [details] [review]
Bug 36025 Incorrect clause in patron search

This patch removes the search clause for
extended_attributes when there is no patron
attribute types which are searchable by default.

The clause was incorrectly added because the array
extended_attribute_types was rendered as [""] in
the template patron-search.inc most likely due to
a bug in DBIx::Class.

Test plan:

* Make sure that no borrower attribute type is set to "searched by default"
  in Koha administration -> Patron attribute types.
* Enable the network monitring in the developer tools in your web browser
  (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network"
  tab).
* Go to the patrons view members-home.pl (click patrons in top menu bar).
* Search for a string that will match multiple borrowers (to not be automatically
  redirected)
* Examine the ajax-request and make sure that "extended_attributes.code" is NOT
  part of the query.
* Also, in koha-testing-docker with selenium enabled,
  run 'perl t/db_dependent/selenium/patrons_search.t'

Comment 4 Nick Clemens (kidclamp) 2024-02-08 14:16:41 UTC

Created attachment 161914 [details] [review]
Bug 36025: Selenium test for the query parameters

The test verifies that there is no clause for extended
attributes in the patron search query when no patron
attribute is searchable by default.

NC amended patch - tidied

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 5 Nick Clemens (kidclamp) 2024-02-08 14:16:43 UTC

Created attachment 161915 [details] [review]
Bug 36025: Incorrect clause in patron search

This patch removes the search clause for
extended_attributes when there is no patron
attribute types which are searchable by default.

The clause was incorrectly added because the array
extended_attribute_types was rendered as [""] in
the template patron-search.inc most likely due to
a bug in DBIx::Class.

Test plan:

* Make sure that no borrower attribute type is set to "searched by default"
  in Koha administration -> Patron attribute types.
* Enable the network monitring in the developer tools in your web browser
  (Ctrl + Shift + E in firefox or Ctrl + Shift + I and select the "Network"
  tab).
* Go to the patrons view members-home.pl (click patrons in top menu bar).
* Search for a string that will match multiple borrowers (to not be automatically
  redirected)
* Examine the ajax-request and make sure that "extended_attributes.code" is NOT
  part of the query.
* Also, in koha-testing-docker with selenium enabled,
  run 'perl t/db_dependent/selenium/patrons_search.t'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>