From 8992e433208cf6e90eb03fd655e158827089d7bd Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Tue, 15 Apr 2025 16:11:30 +0200 Subject: [PATCH] Bug 39637: Build the patron search query on the server side Building the patron search query on the browser side causes the URI to grow rapidly and can reach the HTTP server limit, resulting in a 414 error (Request-URI Too Long) This patch moves the logic to the backend (more precisely, in Koha::REST::V1::Patrons) so that only user input needs to be sent as URL parameters. Test plan: 1. Log in to the staff interface 2. In the top search bar, select "Patrons search" and type "a b c d e f g h i j k l m n o p q r s t u v w x y z" and submit the form 3. The patrons table should load without errors (but will probably be empty) 4. Test the various filters on the left sidebar and verify that they work exactly as before the patch. 5. Modify system preferences DefaultPatronSearchFields and DefaultPatronSearchMethod and verify that they work exactly as before the patch. 6. If not done already, create searchable patron attributes and verify that search works for those too. 7. prove t/db_dependent/api/v1/patrons.t Signed-off-by: David Nind --- Koha/REST/V1/Patrons.pm | 106 +++++++++++++++++- api/v1/swagger/paths/patrons.yaml | 12 ++ .../prog/en/includes/patron-search.inc | 19 +--- t/db_dependent/api/v1/patrons.t | 55 ++++++++- 4 files changed, 173 insertions(+), 19 deletions(-) diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm index 9d25ef7cba..14e535013c 100644 --- a/Koha/REST/V1/Patrons.pm +++ b/Koha/REST/V1/Patrons.pm @@ -20,6 +20,7 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; use Koha::Database; +use Koha::DateUtils; use Koha::Exceptions; use Koha::Patrons; use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages ); @@ -46,15 +47,26 @@ sub list { my $c = shift->openapi->valid_input or return; return try { + my $patrons_rs = Koha::Patrons->search(); - my $query = {}; my $restricted = $c->param('restricted'); $c->req->params->remove('restricted'); - $query->{debarred} = { '!=' => undef } - if $restricted; + if ($restricted) { + $patrons_rs = $patrons_rs->search( { debarred => { '!=' => undef } } ); + } + + my $search = $c->param('search'); + my $search_type = $c->param('search_type'); + my $search_fields = $c->param('search_fields'); + $c->req->params->remove('search'); + $c->req->params->remove('search_type'); + $c->req->params->remove('search_fields'); + + my $query = $c->_buildPatronSearchQuery( + { search => $search, search_type => $search_type, search_fields => $search_fields } ); + $patrons_rs = $patrons_rs->search( $query, { join => 'extended_attributes' } ) if $query; - my $patrons_rs = Koha::Patrons->search($query); - my $patrons = $c->objects->search($patrons_rs); + my $patrons = $c->objects->search($patrons_rs); return $c->render( status => 200, @@ -65,6 +77,90 @@ sub list { }; } +sub _buildPatronSearchQuery { + my ( $self, $args ) = @_; + my ( $search, $search_type, $search_fields ) = @$args{qw(search search_type search_fields)}; + + $search //= ''; + $search =~ s/^\s+|\s+$//; + if ( $search eq '' ) { + return; + } + + $search_type //= C4::Context->preference('DefaultPatronSearchMethod') // 'contains'; + my $leading_wildcard = $search_type eq 'contains' ? '%' : ''; + + my $is_standard = 0; + if ( !defined $search_fields || $search_fields eq '' || $search_fields eq 'standard' ) { + $is_standard = 1; + $search_fields = C4::Context->preference('DefaultPatronSearchFields') + // 'firstname|preferred_name|middle_name|surname|othernames|cardnumber|userid'; + } elsif ( $search_fields eq 'full_address' ) { + $search_fields = 'streetnumber|streettype|address|address2|city|state|zipcode|country'; + } elsif ( $search_fields eq 'all_emails' ) { + $search_fields = 'email|emailpro|B_email'; + } elsif ( $search_fields eq 'all_phones' ) { + $search_fields = 'phone|phonepro|B_phone|altcontactphone|mobile'; + } + my @search_fields = split /\|/, $search_fields; + + my @searched_attribute_fields = map { s/^_ATTR_//r } grep { /^_ATTR_/ } @search_fields; + @search_fields = grep { !/^_ATTR_/ } @search_fields; + + my @words = split /[\s,]+/, $search; + @words = grep { length > 0 } @words; + + my @queries; + + my @word_subquery_and; + foreach my $word (@words) { + my @word_subquery_or; + foreach my $field (@search_fields) { + push @word_subquery_or, { "me.$field" => { like => sprintf( '%s%s%s', $leading_wildcard, $word, '%' ) } }; + + if ( $field eq 'dateofbirth' ) { + eval { + my $dt = Koha::DateUtils::dt_from_string($word); + my $date = Koha::DateUtils::output_pref( { dt => $dt, dateformat => 'rfc3339', dateonly => 1 } ); + push @word_subquery_or, { "me.$field" => $date }; + }; + } + } + push @word_subquery_and, \@word_subquery_or; + } + push @queries, { '-and' => \@word_subquery_and }; + + my @search_subquery_or; + foreach my $field (@search_fields) { + push @search_subquery_or, { "me.$field" => { like => sprintf( '%s%s%s', $leading_wildcard, $search, '%' ) } }; + } + push @queries, { '-or' => \@search_subquery_or }; + + if ( $is_standard && !@searched_attribute_fields ) { + @searched_attribute_fields = + Koha::Patron::Attribute::Types->search( { staff_searchable => 1, searched_by_default => 1 } ) + ->get_column('code'); + } + + if ( C4::Context->preference('ExtendedPatronAttributes') && 0 < @searched_attribute_fields ) { + + # Add each word for each extended patron attributes + my @extended_attribute_codes_to_search = @searched_attribute_fields; + my @extended_attribute_subquery_and; + foreach my $word (@words) { + push @extended_attribute_subquery_and, [ + { + 'extended_attributes.attribute' => { like => sprintf( '%s%s%s', $leading_wildcard, $word, '%' ) }, + 'extended_attributes.code' => \@extended_attribute_codes_to_search, + }, + ]; + } + push @queries, { '-and' => \@extended_attribute_subquery_and }; + } + + return \@queries; +} + =head3 get Controller function that handles retrieving a single Koha::Patron object diff --git a/api/v1/swagger/paths/patrons.yaml b/api/v1/swagger/paths/patrons.yaml index a7df190ba3..6ba277d3ca 100644 --- a/api/v1/swagger/paths/patrons.yaml +++ b/api/v1/swagger/paths/patrons.yaml @@ -9,6 +9,18 @@ produces: - application/json parameters: + - name: search + in: query + required: false + type: string + - name: search_type + in: query + required: false + type: string + - name: search_fields + in: query + required: false + type: string - name: patron_id in: query description: Search on patron_id diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc index 85184bdc6a..542832319b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc @@ -382,19 +382,6 @@ "-and": function(){ let filters = []; - let search_type = patron_search_form.find(".searchtype_filter").val(); - let search_fields = patron_search_form.find(".searchfieldstype_filter").val() || "standard"; - let pattern = patron_search_form.find(".search_patron_filter").val(); - - filters = buildPatronSearchQuery( - pattern, - { - search_type: search_type, - search_fields: search_fields, - ...(typeof extended_attribute_types != 'undefined' && {extended_attribute_types: extended_attribute_types}) - } - ); - let f_sort1 = patron_search_form.find("select[name='sort1_filter']").val(); if ( f_sort1 ) { filters.push({ @@ -785,6 +772,12 @@ fixedHeader: false, }, typeof table_settings !== 'undefined' ? table_settings : null, 1, additional_filters, undefined, external_filter_nodes, parent_block.find(".search_description"), additional_search_descriptions ); + patrons_table.on('preXhr.dt', function (e, settings, data) { + data.search_type = patron_search_form.find(".searchtype_filter").val(); + data.search_fields = patron_search_form.find(".searchfieldstype_filter").val() || "standard"; + data.search = patron_search_form.find(".search_patron_filter").val(); + }); + patron_search_form.on('submit', filter); patron_search_form.on('submit', update_search_type); patron_search_form.on('submit', function(){ diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 6ca12dd053..2be985bc20 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -82,7 +82,7 @@ subtest 'list() tests' => sub { subtest 'librarian access tests' => sub { - plan tests => 15; + plan tests => 16; $schema->storage->txn_begin; @@ -157,6 +157,59 @@ subtest 'list() tests' => sub { ->status_is( 200, "Works, doesn't explode" ); }; + subtest 'search & search_type & search_fields' => sub { + plan tests => 27; + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + surname => 'Doswell', + firstname => 'Celeste', + dateofbirth => '2001-12-31', + }, + } + ); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=celest")->status_is(200) + ->json_is( '/0/patron_id' => $patron->id ); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=elest")->status_is(200)->json_hasnt('/0'); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=elest&search_type=contains")->status_is(200) + ->json_is( '/0/patron_id' => $patron->id ); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=elest&search_type=contains&search_fields=surname") + ->status_is(200)->json_hasnt('/0'); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=elest&search_type=contains&search_fields=firstname") + ->status_is(200)->json_is( '/0/patron_id' => $patron->id ); + + $t->get_ok("//$userid:$password@/api/v1/patrons?search=celeste&search_fields=surname|firstname") + ->status_is(200)->json_is( '/0/patron_id' => $patron->id ); + + t::lib::Mocks::mock_preference( 'dateformat', 'metric' ); + $t->get_ok("//$userid:$password@/api/v1/patrons?search=31/12/2001&search_fields=dateofbirth") + ->status_is(200)->json_is( '/0/patron_id' => $patron->id ); + + t::lib::Mocks::mock_preference( 'dateformat', 'iso' ); + $t->get_ok("//$userid:$password@/api/v1/patrons?search=31/12/2001&search_fields=dateofbirth") + ->status_is(200)->json_hasnt('/0'); + + my $patron_attribute_type = $builder->build_object( + { + class => 'Koha::Patron::Attribute::Types', + value => { + staff_searchable => 1, + searched_by_default => 1, + } + } + ); + $patron->add_extended_attribute( { code => $patron_attribute_type->code, attribute => '1357924680' } ); + $t->get_ok("//$userid:$password@/api/v1/patrons?search=1357924680&search_fields=standard")->status_is(200) + ->json_is( '/0/patron_id' => $patron->id ); + }; + subtest 'searching date and date-time fields' => sub { plan tests => 12; -- 2.39.5