Bug 33974

Summary: Add ability to search biblios endpoint using any biblioitem attribute
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on:    
Bug Blocks: 33556    
Attachments: Bug 33974: Add Koha::Biblios->api_query_fixer
Bug 33974: Make biblioitem columns searchable in a generic way
Bug 33974: (follow-up) Adapt the orders endpoint
Bug 33974: Add Koha::Biblios->api_query_fixer
Bug 33974: Make biblioitem columns searchable in a generic way
Bug 33974: (follow-up) Adapt the orders endpoint
Bug 33974: Add Koha::Biblios->api_query_fixer
Bug 33974: Make biblioitem columns searchable in a generic way
Bug 33974: (follow-up) Adapt the orders endpoint
Bug 33974: Add Koha::Biblios->api_query_fixer
Bug 33974: Make biblioitem columns searchable in a generic way
Bug 33974: (follow-up) Adapt the orders endpoint
Bug 33974: (QA follow-up) Remove superflous import
Bug 33974: (follow-up) biblionumber column needs special handling
Bug 33974: (follow-up) _order_by is not always an arrayref

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