Bug 33974 - Add ability to search biblios endpoint using any biblioitem attribute
Summary: Add ability to search biblios endpoint using any biblioitem attribute
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 33556
  Show dependency treegraph
 
Reported: 2023-06-09 16:33 UTC by Nick Clemens (kidclamp)
Modified: 2024-02-07 19:19 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Bug 33974: Add Koha::Biblios->api_query_fixer (5.99 KB, patch)
2023-06-16 15:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33974: Make biblioitem columns searchable in a generic way (9.19 KB, patch)
2023-06-16 15:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33974: (follow-up) Adapt the orders endpoint (6.14 KB, patch)
2023-06-16 15:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33974: Add Koha::Biblios->api_query_fixer (6.03 KB, patch)
2023-06-20 15:37 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33974: Make biblioitem columns searchable in a generic way (9.23 KB, patch)
2023-06-20 15:37 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33974: (follow-up) Adapt the orders endpoint (6.18 KB, patch)
2023-06-20 15:37 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33974: Add Koha::Biblios->api_query_fixer (6.08 KB, patch)
2023-06-27 16:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33974: Make biblioitem columns searchable in a generic way (9.28 KB, patch)
2023-06-27 16:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33974: (follow-up) Adapt the orders endpoint (6.24 KB, patch)
2023-06-27 16:04 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33974: Add Koha::Biblios->api_query_fixer (6.15 KB, patch)
2023-07-03 15:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33974: Make biblioitem columns searchable in a generic way (9.35 KB, patch)
2023-07-03 15:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33974: (follow-up) Adapt the orders endpoint (6.30 KB, patch)
2023-07-03 15:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33974: (QA follow-up) Remove superflous import (745 bytes, patch)
2023-07-03 15:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33974: (follow-up) biblionumber column needs special handling (1.61 KB, patch)
2023-07-03 16:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33974: (follow-up) _order_by is not always an arrayref (1.88 KB, patch)
2023-07-06 18:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2023-06-09 16:33:24 UTC
Currently the biblios endpoint will list biblios, including the fields from the biblioitems table.

For outside vendors, there is a need to search by the ISBN to retrieve biblios via an ISBN search
Comment 1 Tomás Cohen Arazi 2023-06-15 22:47:34 UTC
Preliminary implementation here:

https://gitlab.com/thekesolutions/Koha/-/tree/bug_33974_biblios_endpoint_extension?ref_type=heads
Comment 2 Tomás Cohen Arazi 2023-06-16 15:56:01 UTC
Created attachment 152433 [details] [review]
Bug 33974: Add Koha::Biblios->api_query_fixer

This patch adds the `api_query_fixer` method to the class, and adds
tests to validate its behavior.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 3 Tomás Cohen Arazi 2023-06-16 15:56:04 UTC
Created attachment 152434 [details] [review]
Bug 33974: Make biblioitem columns searchable in a generic way

This patch makes biblioitem attributes be searchable on the biblios
endpoint. It does so by using the new method in Koha::Biblios, and by
adjusting objects.search(_rs) to accept a $query_fixer arrayref of
functions to be applied to each query or order_by parameters.

The result is cleaner code to write, but complex internals.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass! Searching for biblioitem attributes works on the
API!
3. Sign off :-D
Comment 4 Tomás Cohen Arazi 2023-06-16 15:56:07 UTC
Created attachment 152435 [details] [review]
Bug 33974: (follow-up) Adapt the orders endpoint

In order to reduce the technical debt carried on the orders controller,
and to highlight the decisions made on the prior patch, I adapted the
list() orders controller using the new tools in town.

The result is this endpoint can now embed bibio, without needing to have
a custom piece of code.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass :-D
Comment 5 Tomás Cohen Arazi 2023-06-16 15:57:03 UTC
Marking NSO to get some eyes here. The Objects.t tests haven't been reviewed yet and are expected to fail. Any help is of course welcome.
Comment 6 Sam Lau 2023-06-20 15:37:31 UTC
Created attachment 152487 [details] [review]
Bug 33974: Add Koha::Biblios->api_query_fixer

This patch adds the `api_query_fixer` method to the class, and adds
tests to validate its behavior.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 7 Sam Lau 2023-06-20 15:37:33 UTC
Created attachment 152488 [details] [review]
Bug 33974: Make biblioitem columns searchable in a generic way

This patch makes biblioitem attributes be searchable on the biblios
endpoint. It does so by using the new method in Koha::Biblios, and by
adjusting objects.search(_rs) to accept a $query_fixer arrayref of
functions to be applied to each query or order_by parameters.

The result is cleaner code to write, but complex internals.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass! Searching for biblioitem attributes works on the
API!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 8 Sam Lau 2023-06-20 15:37:35 UTC
Created attachment 152489 [details] [review]
Bug 33974: (follow-up) Adapt the orders endpoint

In order to reduce the technical debt carried on the orders controller,
and to highlight the decisions made on the prior patch, I adapted the
list() orders controller using the new tools in town.

The result is this endpoint can now embed bibio, without needing to have
a custom piece of code.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 9 Nick Clemens (kidclamp) 2023-06-27 16:04:19 UTC
Created attachment 152765 [details] [review]
Bug 33974: Add Koha::Biblios->api_query_fixer

This patch adds the `api_query_fixer` method to the class, and adds
tests to validate its behavior.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens (kidclamp) 2023-06-27 16:04:21 UTC
Created attachment 152766 [details] [review]
Bug 33974: Make biblioitem columns searchable in a generic way

This patch makes biblioitem attributes be searchable on the biblios
endpoint. It does so by using the new method in Koha::Biblios, and by
adjusting objects.search(_rs) to accept a $query_fixer arrayref of
functions to be applied to each query or order_by parameters.

The result is cleaner code to write, but complex internals.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass! Searching for biblioitem attributes works on the
API!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens (kidclamp) 2023-06-27 16:04:24 UTC
Created attachment 152767 [details] [review]
Bug 33974: (follow-up) Adapt the orders endpoint

In order to reduce the technical debt carried on the orders controller,
and to highlight the decisions made on the prior patch, I adapted the
list() orders controller using the new tools in town.

The result is this endpoint can now embed bibio, without needing to have
a custom piece of code.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Martin Renvoize 2023-07-03 15:27:14 UTC
Created attachment 152983 [details] [review]
Bug 33974: Add Koha::Biblios->api_query_fixer

This patch adds the `api_query_fixer` method to the class, and adds
tests to validate its behavior.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2023-07-03 15:27:17 UTC
Created attachment 152984 [details] [review]
Bug 33974: Make biblioitem columns searchable in a generic way

This patch makes biblioitem attributes be searchable on the biblios
endpoint. It does so by using the new method in Koha::Biblios, and by
adjusting objects.search(_rs) to accept a $query_fixer arrayref of
functions to be applied to each query or order_by parameters.

The result is cleaner code to write, but complex internals.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass! Searching for biblioitem attributes works on the
API!
3. Sign off :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2023-07-03 15:27:19 UTC
Created attachment 152985 [details] [review]
Bug 33974: (follow-up) Adapt the orders endpoint

In order to reduce the technical debt carried on the orders controller,
and to highlight the decisions made on the prior patch, I adapted the
list() orders controller using the new tools in town.

The result is this endpoint can now embed bibio, without needing to have
a custom piece of code.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass :-D

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2023-07-03 15:29:35 UTC
Created attachment 152986 [details] [review]
Bug 33974: (QA follow-up) Remove superflous import

We don't actually need the Clone import.. it's not used in Biblios.pmt

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Tomás Cohen Arazi 2023-07-03 16:34:08 UTC
Created attachment 152988 [details] [review]
Bug 33974: (follow-up) biblionumber column needs special handling

This patch makes a particular use case be handled correctly: i.e. no
context is passed (e.g. 'biblio' explicitly) and the query is done
against the `biblio_id` attribute. This results in the following DBIC
error:

[ERROR] GET /api/v1/biblios: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'biblionumber' in where clause is ambiguous at /kohadevbox/koha/Koha/Objects.pm line 394>>

With this patch, this is no longuer the case :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2023-07-03 17:51:01 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Jonathan Druart 2023-07-04 13:18:00 UTC
You broke t/db_dependent/Koha/REST/Plugin/Objects.t
Comment 19 Jonathan Druart 2023-07-04 13:18:17 UTC
(In reply to Jonathan Druart from comment #18)
> You broke t/db_dependent/Koha/REST/Plugin/Objects.t

Can't use string ("+name,-country") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/R
EST/Plugin/Objects.pm line 165.

And lot of others.
Comment 20 Tomás Cohen Arazi 2023-07-06 18:53:28 UTC
Created attachment 153104 [details] [review]
Bug 33974: (follow-up) _order_by is not always an arrayref

The loop through the `_order_by` query parameter occurences introduced
by this patchset was naive regarding the possible scenarios.

When there's only one parameter passed, it shouldn't be expecting an
arrayref, but a scalar. This patch deals with that in the simplest way.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Tests are failing
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Martin Renvoize 2023-07-17 15:21:03 UTC
API only addition.. choosing to backport to 23.05.x series.
Comment 22 Martin Renvoize 2023-07-17 15:35:52 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 23 Matt Blenkinsop 2023-07-18 14:45:18 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x