https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff 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=30165 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131707&action=edit Bug 30165: Make q parameter 'multi' This patch changes the q_param definition so the defined query parameter is repeatable. This way JSON::Validator will always generate an arrayref for it and won't skip occurences. The objects.search helper is updated to always consider the 'q' parameter as an array, as expected. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/query.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Does this need to depend on bug 30194? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131706|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|30194 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30194 [Bug 30194] Update required JSON::Validator version -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131759&action=edit Bug 30165: Regression tests -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 131761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131761&action=edit Bug 30165: (follow-up) Fix GET /acquisitions/orders This patch fixes the particular use case of the orders route, which has a slightly modified version of the objects.search helped, embeded in the controller itself. This controller gets adjusted to the fact q will now be an array. Because of the latter, we end up requiring more code duplication regarding the query fix, so I moved it to an internal sub that gets reused. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/* => FAIL: It t/db_dependent/api/v1/acquisitions_orders.t fails! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131760|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 131804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131804&action=edit Bug 30165: Make q parameter 'multi' This patch changes the q_param definition so the defined query parameter is repeatable. This way JSON::Validator will always generate an arrayref for it and won't skip occurences. The objects.search helper is updated to always consider the 'q' parameter as an array, as expected. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/query.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=30165 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All seems to work in testing and unit tests are passing. Signing off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- - # FIXME The following lines are an ugly fix to deal with isbn and ean searches - # This must NOT be reused or extended - # Instead we need a better and global solution in a Koha::*Biblio method +The Koha::Biblio representation includes the biblioitem.* attributes. This is handy +for API consumers but as they are different tables, converting the queries that mention +biblioitem columns can be tricky. This method renames known column names as used on Koha's +UI. Well, that's not equivalent :D -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #131803|0 |1 is obsolete| | Attachment #131804|0 |1 is obsolete| | Attachment #131805|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131821&action=edit Bug 30165: Regression tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 131823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131823&action=edit Bug 30165: (follow-up) Fix GET /acquisitions/orders This patch fixes the particular use case of the orders route, which has a slightly modified version of the objects.search helped, embeded in the controller itself. This controller gets adjusted to the fact q will now be an array. Because of the latter, we end up requiring more code duplication regarding the query fix, so I moved it to an internal sub that gets reused. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/* => FAIL: It t/db_dependent/api/v1/acquisitions_orders.t fails! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed CC| |fridolin.somers@biblibre.co | |m --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- This breaks t/db_dependent/Koha/REST/Plugin/Objects.t See https://jenkins.koha-community.org/view/master/job/Koha_Master_D10/556/testR... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 132139 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=132139&action=edit [OPTION 2] Bug 30165: (follow-up) Allow objects.search usage outside OpenAPI Our objects.search helper is written for being used in our API controllers, which get their query parameters processed by the OpenAPI plugin, using JSON::Validator. Particularly, the 'q' parameter is defined as 'multi' on our spec, which means objects.search always gets it as an arrayref. As the Objects.t tests are not using the OpenAPI plugin, a hashref is generated as only one q= is being passed. This patch adds an extra validation on objects.search, for the non-arrayref use case and does the right thing. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => FAIL: Tests fail! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Fridolin Somers from comment #24)

I'd say option 2. This may be useful in some other usages of Objects.search

I agree. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com --- Comment #28 from Andrew Nugged <nugged@gmail.com> --- Seems this is the source of 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 if $q =~ m/.*(isbn|ean|publisher).*/; -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30677 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 [Bug 30677] Unknown column 'biblioitem.title' in 'where clause' 500 error in API /api/v1/acquisitions/orders -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30165 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30677 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30677 [Bug 30677] Unknown column 'biblioitem.title' in 'where clause' 500 error in API /api/v1/acquisitions/orders -- You are receiving this mail because: You are watching all bug changes.