[Bug 30677] New: Unknown column 'biblioitem.title' in 'where clause' 500 error in API /api/v1/acquisitions/orders
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Bug ID: 30677 Summary: Unknown column 'biblioitem.title' in 'where clause' 500 error in API /api/v1/acquisitions/orders Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: nugged@gmail.com CC: tomascohen@gmail.com Depends on: 30165 The exception: [ERROR] GET /api/v1/acquisitions/orders: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'biblioitem.title' in 'where clause' at /usr/share/perl5/Data/Page.pm line 49 Because here is the experimental dump of what sub table_name_fixer replaces: in: 'q' => '{"-and":[{"basket.vendor_id":2},[[{"basket.name":{"like":"%f%"}}],[{"basket.basket_group.name":{"like":"%f%"}}],[{"me.order_id":{"like":"%f%"}}],[{"biblio.title":{"like":"%f%"}},{"biblio.author":{"like":"%f%"}},{"biblio.isbn":{"like":"%f%"}},{"biblio.publisher":{"like":"%f%"}},{"me.internal_note":{"like":"%f%"}},{"me.vendor_note":{"like":"%f%"}}],[{"me.replacement_price":{"like":"%f%"}}],[{"me.quantity":{"like":"%f%"}}],[{"me.ecost":{"like":"%f%"}}],[{"fund.name":{"like":"%f%"}}]]]}' }; out: 'q' => '{"-and":[{"basket.vendor_id":2},[[{"basket.name":{"like":"%f%"}}],[{"basket.basket_group.name":{"like":"%f%"}}],[{"me.order_id":{"like":"%f%"}}],[{"biblio.biblioitem.title":{"like":"%f%"}},{"biblio.biblioitem.author":{"like":"%f%"}},{"biblio.biblioitem.isbn":{"like":"%f%"}},{"biblio.biblioitem.publisher":{"like":"%f%"}},{"me.internal_note":{"like":"%f%"}},{"me.vendor_note":{"like":"%f%"}}],[{"me.replacement_price":{"like":"%f%"}}],[{"me.quantity":{"like":"%f%"}}],[{"me.ecost":{"like":"%f%"}}],[{"fund.name":{"like":"%f%"}}]]]}' }; That because this: $q =~ s|biblio\.|biblio\.biblioitem\.|g if $q =~ m/.*(isbn|ean|publisher).*/; should be with lookahead to exclude false matches: $q =~ s{biblio\.(?=isbn|ean|publisher)}{biblio\.biblioitem\.}g; Making a fix. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 [Bug 30165] Several q parameters break the filters -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nugged@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|30165 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 [Bug 30165] Several q parameters break the filters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #1 from Andrew Nugged <nugged@gmail.com> --- Created attachment 134612 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134612&action=edit Bug 30677: Make s/// change only proper strings. To reproduce / test: 1. List form Test Koha all available acquisitions orders https://koha-kktest.lib.helsinki.fi/cgi-bin/koha/acqui/histsearch.pl with any (empty) search request to list them all, 2. open browser's JS console, to see Ajax errors (500), or run in parallel 'tail -F -n0 /var/log/koha/*/*-error.log', 3. start typing in Search field to sub-filter-out ajax-generated search results any existing word in orders below, 4. see that search dies with (in log: [ERROR] GET /api/v1/acquisitions/orders: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'biblioitem.title' in 'where clause' 5. apply the patch, 6. start typing in Search field to sub-filter-out ajax-generated search results: it should filter our what you want. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |30165 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 [Bug 30165] Several q parameters break the filters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #2 from Andrew Nugged <nugged@gmail.com> --- we need to add Tests, and some reproduction stuck: some settings, TBU... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- There are some regression tests here: https://git.koha-community.org/Koha-community/Koha/src/branch/master/t/db_de... you should extend them with your cases if you confirm them. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 134724 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134724&action=edit Bug 30677: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134724|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134900 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134900&action=edit Bug 30677: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134901 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134901&action=edit Bug 30677: Use lookahead in regex for biblioitem replacement This patch takes Andrew's suggested fix using a lookahead regex to correct our biblio vs biblioitem table name replacements. Please use the preceeding unit test patch proposed by Jonathan to test. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All looks good to me and both manual testing and the unit test appear happy. Signing of.. over to QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Regarding comment 2 this is not ready. Andrew, any news on this? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #9 from Andrew Nugged <nugged@gmail.com> --- I was stuck in the reproduction of this issue, but an only day ago I realized that I provided wrong place where this should be reproduced if team found (and I see it is) then it's all ok as for me, -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #10 from Andrew Nugged <nugged@gmail.com> --- I want to propose as QA-phase only this: That "if" is _probably_ not needed: ... if $q =~ m/.*(isbn|ean|publisher).*/; because this anyway matches only those places: $q =~ s{biblio\.(?=isbn|ean|publisher)}{biblio\.biblioitem\.}g; so having this: $q =~ s{biblio\.(?=isbn|ean|publisher)}{biblio\.biblioitem\.}g if $q =~ m/.*(isbn|ean|publisher).*/; is something like "double-check for the same thing", But this is more like 50/50 proposal to agree with you JD/Martin. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + ->json_is( "/0/biblio/isbn", 'SOMETHING', 'Filtering by a biblioitem column works!' ) + ->json_is( "/0/biblio/title", $biblio->title, 'Filtering by a biblioitem and biblio columns works!' ); Descriptions refer tot biblioitem. I thought that was the problem in the first place. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl Status|Signed Off |BLOCKED --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Still looking -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Andrew Nugged from comment #10)
I want to propose as QA-phase only this:
That "if" is _probably_ not needed:
... if $q =~ m/.*(isbn|ean|publisher).*/;
because this anyway matches only those places:
$q =~ s{biblio\.(?=isbn|ean|publisher)}{biblio\.biblioitem\.}g;
so having this:
$q =~ s{biblio\.(?=isbn|ean|publisher)}{biblio\.biblioitem\.}g if $q =~ m/.*(isbn|ean|publisher).*/;
is something like "double-check for the same thing", But this is more like 50/50 proposal to agree with you JD/Martin.
Yes that if is horrible for a purist. And note that matching .* is an insult already. We just wanted m/isbn|etc/ and that was not even very specific matching. Also note the separator switching here: Please use s/ a / b / and m/c/ for instance, but not do s| | | and m/ / It hurts my eyes :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Please tidy both patches. FQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #13)
Yes that if is horrible for a purist. And note that matching .* is an insult already.
Language -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #15)
(In reply to Marcel de Rooy from comment #13)
Yes that if is horrible for a purist. And note that matching .* is an insult already.
Language
? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Simon Hohl <simon.hohl@dainst.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |simon.hohl@dainst.org --- Comment #17 from Simon Hohl <simon.hohl@dainst.org> --- This also affects table filtering in /cgi-bin/koha/acqui/parcel.pl?invoiceid=<number>. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 136677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136677&action=edit Bug 30677: Cleanup This patch cleans up the regular expression to remove the superflous double check and use standard delimiters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Will have a look right now ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134900|0 |1 is obsolete| | --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 136678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136678&action=edit Bug 30677: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134901|0 |1 is obsolete| | --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 136679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136679&action=edit Bug 30677: Use lookahead in regex for biblioitem replacement This patch takes Andrew's suggested fix using a lookahead regex to correct our biblio vs biblioitem table name replacements. Please use the preceeding unit test patch proposed by Jonathan to test. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136677|0 |1 is obsolete| | --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 136680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136680&action=edit Bug 30677: Cleanup This patch cleans up the regular expression to remove the superflous double check and use standard delimiters Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_21_11_candidate, | |rel_22_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|22.11.00 |22.11.00, 22.05.x released in| | CC| |lucas@bywatersolutions.com --- Comment #24 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for 22.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.11.00, 22.05.x |22.11.00, 22.05.x, 21.11.11 released in| | Status|Pushed to stable |Pushed to oldstable CC| |arthur.suzuki@biblibre.com --- Comment #25 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- pushed to 21.11.x for 21.11.11, thx! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |victor@tuxayo.net Status|Pushed to oldstable |RESOLVED --- Comment #26 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Missing dependencies for 21.05.x, it shouldn't be affected, no backport. Nothing to document, marking resolved. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 31193 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org