| 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 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | 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 |
||
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 |
To recreate: Go to addbooks.pl and try a catalog search that starts with a number like 3rd degree, 7th heaven, or 6th target. The searches will result in an ugly error page and the following in the error logs: 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 DBIx::Class::Exception::throw('DBIx::Class::Exception', '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', 0) called at /usr/share/perl5/DBIx/Class/Schema.pm line 1118 DBIx::Class::Schema::throw_exception('Koha::Schema=HASH(0x560206b40608)', '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') called at /usr/share/perl5/DBIx/Class/Storage.pm line 113 DBIx::Class::Storage::throw_exception('DBIx::Class::Storage::DBI::mysql=HASH(0x560206d0e768)', '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') called at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1501 DBIx::Class::Storage::DBI::__ANON__('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', 'DBI::st=HASH(0x56020b84bbe8)', undef) called at /kohadevbox/koha/C4/Breeding.pm line 99 C4::Breeding::BreedingSearch(undef, '7th heaven') called at /kohadevbox/koha/cataloguing/addbooks.pl line 123 eval {...} at /kohadevbox/koha/cataloguing/addbooks.pl line 2 Note: If I search for something like '33rd Heaven', 77th target, I get no error