Bug 30790 - Add REST API route to list biblios
Summary: Add REST API route to list biblios
Status: RESOLVED DUPLICATE of bug 32734
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-05-17 13:17 UTC by Julian Maurice
Modified: 2023-02-21 13:42 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:


Attachments
Bug 30790: Add REST API route to list biblios (7.13 KB, patch)
2022-05-17 13:18 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 30790: Add REST API route to list biblios (7.10 KB, patch)
2022-05-18 06:34 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 30790: Add REST API route to list biblios (7.15 KB, patch)
2022-05-19 11:11 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2022-05-17 13:17:37 UTC

    
Comment 1 Julian Maurice 2022-05-17 13:18:42 UTC
Created attachment 135072 [details] [review]
Bug 30790: Add REST API route to list biblios

Examples:

GET /api/v1/biblios
GET /api/v1/biblios?q={"me.title": "foo"}
GET /api/v1/biblios?q={"biblioitem.isbn": "1-23-456789-0"}
GET /api/v1/biblios?_order_by=title

Test plan:
1. Try requesting this endpoint with your favorite API tool
2. Run `prove t/db_dependent/api/v1/biblios/list.t`
Comment 2 Tomás Cohen Arazi 2022-05-17 13:23:07 UTC
We have hidden the biblioitem table from the API. I regret not having spent time on a more generic solution yet...

Take a look at... Orders.pm when it comes to filtering by biblio.*
Comment 3 Julian Maurice 2022-05-17 13:30:25 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> We have hidden the biblioitem table from the API. I regret not having spent
> time on a more generic solution yet...
> 
> Take a look at... Orders.pm when it comes to filtering by biblio.*

It looks like Koha/REST/Plugin/Objects.pm was duplicated inside Koha::REST::V1::Orders::list
... is it what we are supposed to do ? :/
Comment 4 Tomás Cohen Arazi 2022-05-17 13:34:12 UTC
(In reply to Julian Maurice from comment #3)
> (In reply to Tomás Cohen Arazi from comment #2)
> > We have hidden the biblioitem table from the API. I regret not having spent
> > time on a more generic solution yet...
> > 
> > Take a look at... Orders.pm when it comes to filtering by biblio.*
> 
> It looks like Koha/REST/Plugin/Objects.pm was duplicated inside
> Koha::REST::V1::Orders::list
> ... is it what we are supposed to do ? :/

No, that's why I say I regret not finding the time to provide a generic solution... Namely, a pair of Koha::Objects methods for tweaking the 'embed' structure and queries....
Comment 5 Julian Maurice 2022-05-17 14:18:17 UTC
(In reply to Tomás Cohen Arazi from comment #4)
> No, that's why I say I regret not finding the time to provide a generic
> solution... Namely, a pair of Koha::Objects methods for tweaking the 'embed'
> structure and queries....

I just noticed that I can do

GET /api/v1/biblios?q={"isbn": "1-23-456789-0"}

without the "biblioitem." prefix and it works too. So... is there something else needed ? (except updating the tests and commit messages to reflect that)
Comment 6 Julian Maurice 2022-05-18 06:34:34 UTC
Created attachment 135089 [details] [review]
Bug 30790: Add REST API route to list biblios

Examples:

GET /api/v1/biblios
GET /api/v1/biblios?q={"title": "foo"}
GET /api/v1/biblios?q={"isbn": "1-23-456789-0"}
GET /api/v1/biblios?_order_by=title

Test plan:
1. Try requesting this endpoint with your favorite API tool
2. Run `prove t/db_dependent/api/v1/biblios/list.t`
Comment 7 David Nind 2022-05-19 11:11:37 UTC
Created attachment 135199 [details] [review]
Bug 30790: Add REST API route to list biblios

Examples:

GET /api/v1/biblios
GET /api/v1/biblios?q={"title": "foo"}
GET /api/v1/biblios?q={"isbn": "1-23-456789-0"}
GET /api/v1/biblios?_order_by=title

Test plan:
1. Try requesting this endpoint with your favorite API tool
2. Run `prove t/db_dependent/api/v1/biblios/list.t`

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Marcel de Rooy 2022-05-20 07:06:49 UTC
+    $schema->resultset('Biblio')->delete();

Hmm
Comment 9 Marcel de Rooy 2022-05-20 07:11:00 UTC
my $biblios_rs = Koha::Biblios->new->search(undef, { join => 'biblioitem' });

Not a normal pattern to use new here
Comment 10 Julian Maurice 2022-05-20 07:37:02 UTC
(In reply to Marcel de Rooy from comment #8)
> +    $schema->resultset('Biblio')->delete();
> 
> Hmm

What is wrong with this ? It's in the tests, inside a transaction.

(In reply to Marcel de Rooy from comment #9)
> my $biblios_rs = Koha::Biblios->new->search(undef, { join => 'biblioitem' });
> 
> Not a normal pattern to use new here

Bad copy/paste/edit here ... I'll fix it
Comment 11 Marcel de Rooy 2022-05-20 07:50:01 UTC
(In reply to Julian Maurice from comment #10)
> (In reply to Marcel de Rooy from comment #8)
> > +    $schema->resultset('Biblio')->delete();
> What is wrong with this ? It's in the tests, inside a transaction.

We try to avoid direct DBIx stuff when having an Koha object.
Yes, this may be faster. But Koha objects abstract the DBIx.
So here you could simply do Koha::Biblios->delete.

Like:
t/db_dependent/HoldsQueue.t:    Koha::Biblios->delete();

But found:
t/db_dependent/Koha/BiblioUtils/Iterator.t:$schema->resultset('Biblio')->delete();
Comment 12 Marcel de Rooy 2022-05-20 07:58:58 UTC
Just trying to get a bit more grip on what could be done with the query string:

objects.search
            my $args = $c->validation->output;
[..]
            my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($args);

Query
extract_reserved_params
$c->req->params->to_hash is compared with $c->validation->output
The first iiuc is all get/post parameters. But what exactly is the second?
Where is this output method exactly? Thus far couldnt find it yet in the JSON Validator and OpenAPI stuff..
Comment 13 Marcel de Rooy 2022-05-20 08:22:00 UTC
Seeing this too:

#   Failed test 'No tests run for subtest "get() tests"'
#   at t/db_dependent/api/v1/biblios/list.t line 89.
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_myclone`.`issues`, CONSTRAINT `issues_ibfk_2` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON UPDATE CASCADE) at t/db_dependent/api/v1/biblios/list.t line 48
Comment 14 Marcel de Rooy 2022-05-20 08:23:32 UTC
(In reply to Marcel de Rooy from comment #13)
> Seeing this too:
> 
> #   Failed test 'No tests run for subtest "get() tests"'
> #   at t/db_dependent/api/v1/biblios/list.t line 89.
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Cannot delete or update a parent row: a foreign key
> constraint fails (`koha_myclone`.`issues`, CONSTRAINT `issues_ibfk_2`
> FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON UPDATE
> CASCADE) at t/db_dependent/api/v1/biblios/list.t line 48

Yes, when you delete biblios there could still be issues !
Comment 15 Marcel de Rooy 2022-05-20 08:26:48 UTC
(In reply to Marcel de Rooy from comment #14)
> (In reply to Marcel de Rooy from comment #13)
> > Seeing this too:
> > 
> > #   Failed test 'No tests run for subtest "get() tests"'
> > #   at t/db_dependent/api/v1/biblios/list.t line 89.
> > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> > execute failed: Cannot delete or update a parent row: a foreign key
> > constraint fails (`koha_myclone`.`issues`, CONSTRAINT `issues_ibfk_2`
> > FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON UPDATE
> > CASCADE) at t/db_dependent/api/v1/biblios/list.t line 48
> 
> Yes, when you delete biblios there could still be issues !

You might better consider adding a few biblios without deleting them all btw. And adjusting your queries based on that situation?
Comment 16 Tomás Cohen Arazi 2023-02-21 13:42:13 UTC
Picking bug 32734 which includes more use cases (response formats based on the Accept header) and is pretty similar and active.

*** This bug has been marked as a duplicate of bug 32734 ***