From dd70295546ad980822621adb2df682128da65818 Mon Sep 17 00:00:00 2001 From: Julian Maurice <julian.maurice@biblibre.com> 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 --- 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 $patrons_rs = Koha::Patrons->search($query); - my $patrons = $c->objects->search($patrons_rs); + 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 = $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 3cdc322445..e288aa646c 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 fa86900600..c8884474d8 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({ @@ -783,6 +770,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