Summary: | Errors when doing a cataloging search which starts with a number + letter | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Cataloging | Assignee: | Lucas Gass (lukeg) <lucas> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | barbara.johnson, david, fridolin.somers, jonathan.druart, kyle, m.de.rooy, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes an error that occurs in cataloging search when entering a search term with ten characters (like "7th Heaven" or "2nd editio") - Koha thinks you are entering an ISBN10 number, gets confused and delivers an error page. Searching now works as expected for ISBN13/ISBN10 (without the '-'s), title and author searches.
|
Version(s) released in: |
21.11.00,21.05.05,20.11.14
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 29437 | ||
Attachments: |
Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search
Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search |
Description
Lucas Gass (lukeg)
2021-10-25 21:48:32 UTC
All the example searches I gave ( 6th target, 7th heaven, 3rd degree) contain 10 characters, including the whitespace. This appears to be the only way to recreate this. addbooks.pl is assuming this is an ISBN, I think we now have better ways to check that now. Created attachment 126866 [details] [review] Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search To test: 1. Go to cataloging search and enter something like "7th Heaven". 2. Get an error when searching, Koha thinks you entered an ISBN 3. Apply patch 4. Try the same search, it should be a proper title search now 5. Find some stuff in the catalog with ISBN numbers in them. 6. The search should properly return ISBN13/ISBN10 searches, without with out the '-'. Even after applying the patch I still got an error log with 57 lines of stuff in it so I'm setting to Failed QA. C4::Breeding::BreedingSearch(): DBI Exception: DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '' at line 5 at /kohadevbox/koha/cataloguing/addbooks.pl line 123 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77 74: my $self = { msg => $msg }; 75: bless $self => $class; 76: 77: die $self; 78: } 79: 80: =head2 rethrow Show function arguments in DBIx::Class::Schema::throw_exception at /usr/share/perl5/DBIx/Class/Schema.pm line 1118 1115: }; 1116: } 1117: 1118: DBIx::Class::Exception->throw($args[0], $self->stacktrace); 1119: } 1120: 1121: =head2 deploy Show function arguments in DBIx::Class::Storage::throw_exception at /usr/share/perl5/DBIx/Class/Storage.pm line 113 110: my $self = shift; 111: 112: if (ref $self and $self->schema) { 113: $self->schema->throw_exception(@_); 114: } 115: else { 116: DBIx::Class::Exception->throw(@_); Show function arguments in DBIx::Class::Storage::DBI::__ANON__ at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1501 1498: # supplied handles (which must be preserved) 1499: $_[1]->{HandleError} = bless sub { 1500: if ($weak_self) { 1501: $weak_self->throw_exception("DBI Exception: $_[0]"); 1502: } 1503: else { 1504: # the handler may be invoked by something totally out of Show function arguments in C4::Breeding::BreedingSearch at /kohadevbox/koha/C4/Breeding.pm line 99 96: push(@bind,"$isbn%"); 97: } 98: $sth = $dbh->prepare($query); 99: $sth->execute(@bind); 100: while (my $data = $sth->fetchrow_hashref) { 101: $results[$count] = $data; 102: # FIXME - hack to reflect difference in name Show function arguments in (eval) at /kohadevbox/koha/cataloguing/addbooks.pl line 123 120: push @{$breeding_loop}, { 121: id => $resultsbr->{import_record_id}, 122: isbn => $resultsbr->{isbn}, 123: copyrightdate => $resultsbr->{copyrightdate}, 124: editionstatement => $resultsbr->{editionstatement}, 125: file => $resultsbr->{file_name}, 126: title => $resultsbr->{title}, in CGI::Compile::ROOT::kohadevbox_koha_cataloguing_addbooks_2epl::__ANON__ at /kohadevbox/koha/cataloguing/addbooks.pl line 2 1: #!/usr/bin/perl 2: 3: 4: # Copyright 2000-2002 Katipo Communications 5: # Show function arguments in CGI::Compile::ROOT::kohadevbox_koha_cataloguing_addbooks_2epl::addbooks_2epl at /usr/share/perl5/CGI/Compile.pm line 151 148: # this is necessary for MSWin32 149: my $orig_warn = $SIG{__WARN__} || sub { warn(@_) }; 150: local $SIG{__WARN__} = sub { $orig_warn->(@_) unless $_[0] =~ /^No such signal/ }; 151: $code->($self, $data, $path, $dir, \@args) 152: }; 153: }; 154: in CGI::Emulate::PSGI::__ANON__ at /usr/share/perl5/CGI/Emulate/PSGI.pm line 30 27: local *STDOUT = $stdout; 28: local *STDERR = $env->{'psgi.errors'}; 29: 30: $code->(); 31: } 32: } 33: Show function arguments in Plack::App::WrapCGI::call at /usr/share/perl5/Plack/App/WrapCGI.pm line 95 92: 93: sub call { 94: my($self, $env) = @_; 95: $self->_app->($env); 96: } 97: 98: 1; Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::App::CGIBin::serve_path at /usr/share/perl5/Plack/App/CGIBin.pm line 50 47: my $app = $self->{_compiled}->{$file} ||= Plack::App::WrapCGI->new( 48: script => $file, execute => $self->would_exec($file), 49: )->to_app; 50: $app->($env); 51: } 52: 53: 1; Show function arguments in Plack::App::File::call at /usr/share/perl5/Plack/App/File.pm line 34 31: $env->{'plack.file.PATH_INFO'} = ''; 32: } 33: 34: return $self->serve_path($env, $file); 35: } 36: 37: sub locate_file { Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::LogWarn::call at /usr/share/perl5/Plack/Middleware/LogWarn.pm line 22 19: message => join '', @_ 20: }); 21: }; 22: my $res = $self->app->($env); 23: 24: return $res; 25: } Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::Log4perl::call at /usr/share/perl5/Plack/Middleware/Log4perl.pm line 30 27: $self->logger->$level($args->{message}); 28: }; 29: 30: $self->app->($env); 31: } 32: 33: 1; Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::HTTPExceptions::try {...} at /usr/share/perl5/Plack/Middleware/HTTPExceptions.pm line 20 17: my($self, $env) = @_; 18: 19: my $res = try { 20: $self->app->($env); 21: } catch { 22: $self->transform_error($_, $env); 23: }; in (eval) at /usr/share/perl5/Try/Tiny.pm line 100 97: if ( $wantarray ) { 98: @ret = $try->(); 99: } elsif ( defined $wantarray ) { 100: $ret[0] = $try->(); 101: } else { 102: $try->(); 103: }; in Try::Tiny::try at /usr/share/perl5/Try/Tiny.pm line 93 90: 91: # failed will be true if the eval dies, because 1 will not be returned 92: # from the eval body 93: my $failed = not eval { 94: $@ = $prev_error; 95: 96: # evaluate the try block in the correct context Show function arguments in Plack::Middleware::HTTPExceptions::call at /usr/share/perl5/Plack/Middleware/HTTPExceptions.pm line 23 20: $self->app->($env); 21: } catch { 22: $self->transform_error($_, $env); 23: }; 24: 25: return $res if ref $res eq 'ARRAY'; 26: Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::ErrorDocument::call at /usr/share/perl5/Plack/Middleware/ErrorDocument.pm line 15 12: my $self = shift; 13: my $env = shift; 14: 15: my $r = $self->app->($env); 16: 17: $self->response_cb($r, sub { 18: my $r = shift; Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::App::URLMap::call at /usr/share/perl5/Plack/App/URLMap.pm line 71 68: return $self->response_cb($app->($env), sub { 69: $env->{PATH_INFO} = $orig_path_info; 70: $env->{SCRIPT_NAME} = $orig_script_name; 71: }); 72: } 73: 74: DEBUG && warn "All matching failed.\n"; Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Koha::Middleware::RealIP::call at /kohadevbox/koha/Koha/Middleware/RealIP.pm line 64 61: } 62: } 63: 64: return $self->app->($env); 65: } 66: 67: =head2 get_real_ip Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Koha::Middleware::SetEnv::call at /kohadevbox/koha/Koha/Middleware/SetEnv.pm line 104 101: %setenvs 102: }; 103: 104: return $self->app->($env); 105: } 106: 107: =head1 AUTHOR Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::Static::call at /usr/share/perl5/Plack/Middleware/Static.pm line 18 15: return $res; 16: } 17: 18: return $self->app->($env); 19: } 20: 21: sub _handle_static { Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::ReverseProxy::call at /usr/share/perl5/Plack/Middleware/ReverseProxy.pm line 68 65: } 66: } 67: 68: $self->app->($env); 69: } 70: 71: 1; Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::Lint::call at /usr/share/perl5/Plack/Middleware/Lint.pm line 24 21: my $env = shift; 22: 23: $self->validate_env($env); 24: my $res = $self->app->($env); 25: return $self->validate_res($res); 26: } 27: Show function arguments in Plack::Middleware::StackTrace::try {...} at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: in (eval) at /usr/share/perl5/Try/Tiny.pm line 100 97: if ( $wantarray ) { 98: @ret = $try->(); 99: } elsif ( defined $wantarray ) { 100: $ret[0] = $try->(); 101: } else { 102: $try->(); 103: }; in Plack::Middleware::StackTrace::call at /usr/share/perl5/Try/Tiny.pm line 93 90: 91: # failed will be true if the eval dies, because 1 will not be returned 92: # from the eval body 93: my $failed = not eval { 94: $@ = $prev_error; 95: 96: # evaluate the try block in the correct context Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in Plack::Middleware::AccessLog::call at /usr/share/perl5/Plack/Middleware/AccessLog.pm line 27 24: my $self = shift; 25: my($env) = @_; 26: 27: my $res = $self->app->($env); 28: 29: if ( ref($res) && ref($res) eq 'ARRAY' ) { 30: my $content_length = Plack::Util::content_length($res->[2]); Show function arguments in Plack::Component::__ANON__ at /usr/share/perl5/Plack/Component.pm line 50 47: sub to_app { 48: my $self = shift; 49: $self->prepare_app; 50: return sub { $self->call(@_) }; 51: } 52: 53: Show function arguments in (eval) at /usr/share/perl5/Plack/Util.pm line 145 142: sub run_app($$) { 143: my($app, $env) = @_; 144: 145: return eval { $app->($env) } || do { 146: my $body = "Internal Server Error"; 147: $env->{'psgi.errors'}->print($@); 148: [ 500, [ 'Content-Type' => 'text/plain', 'Content-Length' => length($body) ], [ $body ] ]; in Plack::Util::run_app at /usr/share/perl5/Plack/Util.pm line 145 142: sub run_app($$) { 143: my($app, $env) = @_; 144: 145: return eval { $app->($env) } || do { 146: my $body = "Internal Server Error"; 147: $env->{'psgi.errors'}->print($@); 148: [ 500, [ 'Content-Type' => 'text/plain', 'Content-Length' => length($body) ], [ $body ] ]; Show function arguments in Starman::Server::dispatch_request at /usr/share/perl5/Starman/Server.pm line 195 192: my ($self, $env) = @_; 193: 194: # Run PSGI apps 195: my $res = Plack::Util::run_app($self->{app}, $env); 196: 197: if (ref $res eq 'CODE') { 198: $res->(sub { $self->_finalize_response($env, $_[0]) }); Show function arguments in Starman::Server::process_request at /usr/share/perl5/Starman/Server.pm line 298 295: 296: $self->_prepare_env($env); 297: 298: $self->dispatch_request($env); 299: 300: DEBUG && warn "[$$] Request done\n"; 301: Show function arguments in Net::Server::run_client_connection at /usr/share/perl5/Net/Server.pm line 72 69: $self->post_accept_hook($c); # user customizable hook 70: 71: my $ok = $self->allow_deny($c) && $self->allow_deny_hook($c); # do allow/deny check on client info 72: if ($ok) { 73: $self->process_request($c); # This is where the core functionality of a Net::Server should be. 74: } else { 75: $self->request_denied_hook($c); # user customizable hook Show function arguments in (eval) at /usr/share/perl5/Net/Server/PreFork.pm line 225 222: $prop->{'connected'} = 1; 223: print $write "$$ processing\n"; 224: 225: my $ok = eval { $self->run_client_connection; 1 }; 226: if (! $ok) { 227: print $write "$$ exiting\n"; 228: die $@; in Net::Server::PreFork::run_child at /usr/share/perl5/Net/Server/PreFork.pm line 225 222: $prop->{'connected'} = 1; 223: print $write "$$ processing\n"; 224: 225: my $ok = eval { $self->run_client_connection; 1 }; 226: if (! $ok) { 227: print $write "$$ exiting\n"; 228: die $@; Show function arguments in Net::Server::PreFork::run_n_children at /usr/share/perl5/Net/Server/PreFork.pm line 183 180: if ($prop->{'child_communication'}) { 181: $prop->{'parent_sock'} = $childsock; 182: } 183: $self->run_child; 184: } 185: } 186: } Show function arguments in Net::Server::PreFork::loop at /usr/share/perl5/Net/Server/PreFork.pm line 107 104: 105: my $start = $prop->{'min_servers'}; 106: $self->log(3, "Beginning prefork ($start processes)"); 107: $self->run_n_children($start); 108: 109: $self->run_parent; 110: } Show function arguments in Net::Server::run at /usr/share/perl5/Net/Server.pm line 58 55: $self->post_bind; # allow for chrooting, becoming a different user and group 56: 57: $self->pre_loop_hook; # user customizable hook 58: $self->loop; # repeat accept/process cycle 59: 60: $self->server_close; # close the server and release the port 61: } Show function arguments in Starman::Server::run at /usr/share/perl5/Starman/Server.pm line 106 103: max_requests => $options->{max_requests} || 1000, 104: user => $options->{user} || $>, 105: group => $options->{group} || $), 106: listen => $options->{backlog} || 1024, 107: check_for_waiting => 1, 108: no_client_stdout => 1, 109: %extra Show function arguments in Plack::Handler::Starman::run at /usr/share/perl5/Plack/Handler/Starman.pm line 25 22: } 23: $self->{net_server_args} = \%nsa if %nsa; 24: 25: Starman::Server->new->run($app, {%$self}); 26: } 27: 28: 1; Show function arguments in Plack::Loader::Delayed::run at /usr/share/perl5/Plack/Loader/Delayed.pm line 20 17: }; 18: 19: $server->{psgi_app_builder} = $self->{builder}; 20: $server->run($app); 21: } 22: 23: 1; Show function arguments in Plack::Runner::run at /usr/share/perl5/Plack/Runner.pm line 277 274: $loader->preload_app($app); 275: 276: my $server = $self->load_server($loader); 277: $loader->run($server); 278: } 279: 280: 1; Show function arguments at /usr/bin/starman line 38 35: } 36: 37: $runner->set_options(argv => \@argv); 38: $runner->run; 39: 40: __END__ 41: Created attachment 126946 [details] [review] Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search To test: 1. Go to cataloging search and enter something like "7th Heaven". 2. Get an error when searching, Koha thinks you entered an ISBN 3. Apply patch 4. Try the same search, it should be a proper title search now 5. Find some stuff in the catalog with ISBN numbers in them. 6. The search should properly return ISBN13/ISBN10 searches, without with out the '-'. Signed-off-by: David Nind <david@davidnind.com> (In reply to Barbara Johnson from comment #4) > Even after applying the patch I still got an error log with 57 lines of > stuff in it so I'm setting to Failed QA. We must have been testing at the same time! The patch worked for me using koha-testing-docker - I did a flush_memcached and restart_all after applying the patch. What an interesting error! I am reproducing this with "2nd editio" It works perfectly for me too - I also verified that ISBN searches still find what they are supposed to find. I think it was the missing "restart" that caused the problems for Barbara. Created attachment 127128 [details] [review] Bug 29319: Use Business::ISBN to check ISBNs on addbooks.pl/cataloging search To test: 1. Go to cataloging search and enter something like "7th Heaven". 2. Get an error when searching, Koha thinks you entered an ISBN 3. Apply patch 4. Try the same search, it should be a proper title search now 5. Find some stuff in the catalog with ISBN numbers in them. 6. The search should properly return ISBN13/ISBN10 searches, without with out the '-'. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.05 I prefer not to push on 20.11.x to keep stable behavior in cataloguing (In reply to Fridolin Somers from comment #12) > I prefer not to push on 20.11.x to keep stable behavior in cataloguing Please explain this. Whats your problem with this patch? (In reply to Marcel de Rooy from comment #13) > (In reply to Fridolin Somers from comment #12) > > I prefer not to push on 20.11.x to keep stable behavior in cataloguing > > Please explain this. Whats your problem with this patch? Sorry I don't remember well. My wish was to keep 20.11.x really stable. I let the decision to Victor , actual release maintainer of 20.11.x. (In reply to Marcel de Rooy from comment #13) > (In reply to Fridolin Somers from comment #12) > > I prefer not to push on 20.11.x to keep stable behavior in cataloguing > > Please explain this. Whats your problem with this patch? Sorry I don't remember well. My wish was to keep 20.11.x really stable. I let the decision to Victor Grousset, actual release maintainer of 20.11.x. Backported: Pushed to 20.11.x branch for 20.11.14 |