Bug 23677

Summary: Add API route to get a bibliographic record
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: new feature    
Priority: P5 - low CC: arthur.suzuki, jonathan.druart, joonas.kylmala, josef.moravec, katrin.fischer, kyle, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21232
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 24237, 17371, 21232, 24238, 24908, 24909    
Attachments: Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}
Bug 23677: Schema fix
Bug 23677: Controller method and dependencies tweak
Bug 23677: Unit tests
Bug 23677: Controller method and dependencies tweak
Bug 23677: Unit tests
Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}
Bug 23677: Schema fix
Bug 23677: Controller method and dependencies tweak
Bug 23677: Unit tests
Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}
Bug 23677: Schema fix
Bug 23677: Controller method and dependencies tweak
Bug 23677: Unit tests

Description Tomás Cohen Arazi 2019-09-25 19:58:42 UTC

    
Comment 1 Tomás Cohen Arazi 2019-09-26 19:26:44 UTC
Created attachment 93172 [details] [review]
Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}
Comment 2 Tomás Cohen Arazi 2019-09-26 19:26:48 UTC
Created attachment 93173 [details] [review]
Bug 23677: Schema fix
Comment 3 Tomás Cohen Arazi 2019-09-26 19:26:51 UTC
Created attachment 93174 [details] [review]
Bug 23677: Controller method and dependencies tweak
Comment 4 Tomás Cohen Arazi 2019-09-26 19:26:55 UTC
Created attachment 93175 [details] [review]
Bug 23677: Unit tests

This patch introduces tests for the GET /biblios/{biblio_id} endpoint.
It tries to cover all the use cases.

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
- Use Postman (or your favourite tool) to test the API
=> SUCCESS: Works as expected!
- Sign off :-D
Comment 5 Tomás Cohen Arazi 2019-09-26 19:27:59 UTC
I submit this work to gather opinions. I've already thought about extending this with plugins for handling special serialization formats and schemas. This is just the start
Comment 6 Joonas Kylmälä 2019-09-30 10:27:35 UTC
I think it would be wise to submit support for MARC in JSON format in a separate patch after this is merged. This is because MARC::Record::MiJ needs to be first packaged for Debian and that might take a while.
Comment 7 Tomás Cohen Arazi 2019-09-30 10:57:54 UTC
(In reply to Joonas Kylmälä from comment #6)
> I think it would be wise to submit support for MARC in JSON format in a
> separate patch after this is merged. This is because MARC::Record::MiJ needs
> to be first packaged for Debian and that might take a while.

It is already packaged.

https://packages.debian.org/stretch/libmarc-file-mij-perl
Comment 8 Joonas Kylmälä 2019-09-30 11:34:06 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> It is already packaged.
> 
> https://packages.debian.org/stretch/libmarc-file-mij-perl

Sorry about that, I just searched for MARC::Record::MiJ and couldn't find it.
Comment 9 Tomás Cohen Arazi 2019-09-30 11:45:33 UTC
It might not be pulled by default. As Catmandu is pulled when you install koha-elasticsearch I recall.
But this patch adds as a core dependency so next koha-common builds would pull it!

Thanks for looking at my patches
Comment 10 Arthur Suzuki 2019-10-03 14:04:45 UTC
Hi Tomas,
Here are the results for the qa-tools, over than that everything (unit test and manual testing) working perfectly thanks!

curl http://localhost:8080/api/v1/biblios/4 \ -H 'Accept: application/json'

> Gives back expected results :)


testing 4 commit(s) (applied to 98e4b5c '4a Bug 23537: Overdrive won't show co')

Processing files before patches
|========================>| 4 / 4 (100.00%)
Processing files after patches
|========================>| 4 / 4 (100.00%)

 OK	C4/Installer/PerlDependencies.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   OK	  valid

 FAIL	Koha/REST/V1/Biblios.pm
   FAIL	  critic
		# Variables::ProhibitConditionalDeclarations: Got 1 violation(s).  
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   OK	  valid

 OK	Koha/Schema/Result/Biblio.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  pod coverage
   OK	  spelling
   OK	  valid

 OK	t/db_dependent/api/v1/biblios.t
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  spelling
   OK	  valid
Comment 11 Tomás Cohen Arazi 2019-10-03 14:30:48 UTC
Created attachment 93622 [details] [review]
Bug 23677: Controller method and dependencies tweak
Comment 12 Tomás Cohen Arazi 2019-10-03 14:30:55 UTC
Created attachment 93623 [details] [review]
Bug 23677: Unit tests

This patch introduces tests for the GET /biblios/{biblio_id} endpoint.
It tries to cover all the use cases.

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
- Use Postman (or your favourite tool) to test the API
=> SUCCESS: Works as expected!
- Sign off :-D
Comment 13 Tomás Cohen Arazi 2019-10-03 14:31:38 UTC
(In reply to Arthur Suzuki from comment #10)
> Hi Tomas,
> Here are the results for the qa-tools, over than that everything (unit test
> and manual testing) working perfectly thanks!
> 
> curl http://localhost:8080/api/v1/biblios/4 \ -H 'Accept: application/json'
> 
> > Gives back expected results :)

Awesome! Thanks!

>  FAIL	Koha/REST/V1/Biblios.pm
>    FAIL	  critic
> 		# Variables::ProhibitConditionalDeclarations: Got 1 violation(s).  

Fixed!
Comment 14 Arthur Suzuki 2019-10-04 08:38:28 UTC
Created attachment 93679 [details] [review]
Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 15 Arthur Suzuki 2019-10-04 08:38:44 UTC
Created attachment 93680 [details] [review]
Bug 23677: Schema fix

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 16 Arthur Suzuki 2019-10-04 08:38:47 UTC
Created attachment 93681 [details] [review]
Bug 23677: Controller method and dependencies tweak

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 17 Arthur Suzuki 2019-10-04 08:38:51 UTC
Created attachment 93682 [details] [review]
Bug 23677: Unit tests

This patch introduces tests for the GET /biblios/{biblio_id} endpoint.
It tries to cover all the use cases.

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
- Use Postman (or your favourite tool) to test the API
=> SUCCESS: Works as expected!
- Sign off :-D

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 18 Kyle M Hall 2019-10-04 16:13:46 UTC
Created attachment 93750 [details] [review]
Bug 23677: OpenAPI spec for GET /biblios/{biblio_id}

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Kyle M Hall 2019-10-04 16:13:56 UTC
Created attachment 93751 [details] [review]
Bug 23677: Schema fix

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Kyle M Hall 2019-10-04 16:13:59 UTC
Created attachment 93752 [details] [review]
Bug 23677: Controller method and dependencies tweak

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2019-10-04 16:14:02 UTC
Created attachment 93753 [details] [review]
Bug 23677: Unit tests

This patch introduces tests for the GET /biblios/{biblio_id} endpoint.
It tries to cover all the use cases.

To test:
- Apply this patchset
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
- Use Postman (or your favourite tool) to test the API
=> SUCCESS: Works as expected!
- Sign off :-D

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Martin Renvoize 2019-10-08 13:31:42 UTC
Nice work!

Pushed to master for 19.11.00
Comment 23 Jonathan Druart 2019-12-13 11:46:41 UTC
I used this new route on bug 21232. I am if it is ok to request the accept in the header (by default it is */*).
Should not we default to application/json?
Comment 24 Martin Renvoize 2019-12-13 11:47:54 UTC
+1
Comment 25 Tomás Cohen Arazi 2019-12-13 12:30:54 UTC
(In reply to Jonathan Druart from comment #23)
> I used this new route on bug 21232. I am if it is ok to request the accept
> in the header (by default it is */*).
> Should not we default to application/json?

I didn't implement it like that, because I had plans of making HTML the default (i.e. the normal, tt based view).

But it's a fair request. I can do it if you file a bug.
Comment 26 Jonathan Druart 2019-12-13 12:55:49 UTC
It seems that other routes (all? I did not check) are already defaulting to json. Is it correct?
If so it would make sense to continue that way.