Bugzilla – Attachment 182282 Details for
Bug 39637
Patron search generates URIs that are too long and can cause "HTTP 414 URI Too Long" errors
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 39637: Build the patron search query on the server side
Bug-39637-Build-the-patron-search-query-on-the-ser.patch (text/plain), 12.65 KB, created by
David Nind
on 2025-05-12 14:06:31 UTC
(
hide
)
Description:
Bug 39637: Build the patron search query on the server side
Filename:
MIME Type:
Creator:
David Nind
Created:
2025-05-12 14:06:31 UTC
Size:
12.65 KB
patch
obsolete
>From 8992e433208cf6e90eb03fd655e158827089d7bd 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 > >Signed-off-by: David Nind <david@davidnind.com> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 39637
:
181005
|
182264
|
182282
|
182283
|
183273
|
183274