Bug 31794 - Add GET endpoint for Authorities
Summary: Add GET endpoint for Authorities
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature (vote)
Assignee: Agustín Moyano
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 26611 33124 31790 31793
  Show dependency treegraph
 
Reported: 2022-10-14 14:34 UTC by Nick Clemens
Modified: 2023-11-15 15:58 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 31974: Regression: Bug 31813 incorrectly affected labels in the header search (1.28 KB, patch)
2022-10-25 16:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31794: Add REST endpoint to get an authority (13.61 KB, patch)
2022-12-06 20:26 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31794: Add REST endpoint to get an authority (13.66 KB, patch)
2022-12-06 23:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 31794: Add REST endpoint to get an authority (13.66 KB, patch)
2022-12-07 18:33 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31794: (follow-up) Add description to authorities tag (828 bytes, patch)
2022-12-07 18:33 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31794: Add REST endpoint to get an authority (13.72 KB, patch)
2023-02-07 16:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 31794: (follow-up) Add description to authorities tag (887 bytes, patch)
2023-02-07 16:02 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 31794: Add REST endpoint to get an authority (13.81 KB, patch)
2023-03-06 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31794: (follow-up) Add description to authorities tag (985 bytes, patch)
2023-03-06 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-10-14 14:34:26 UTC

    
Comment 1 Owen Leonard 2022-10-25 16:15:50 UTC Comment hidden (obsolete)
Comment 2 Agustín Moyano 2022-12-06 20:26:17 UTC
Created attachment 144458 [details] [review]
Bug 31794: Add REST endpoint to get an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a GET request to /api/v1/authorities/{authid} with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
  - text/plain
5. Check that you get the authority in the corresponding format
6. Sign off
Comment 3 David Nind 2022-12-06 23:56:45 UTC
Created attachment 144460 [details] [review]
Bug 31794: Add REST endpoint to get an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a GET request to /api/v1/authorities/{authid} with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
  - text/plain
5. Check that you get the authority in the corresponding format
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2022-12-07 00:06:22 UTC
Testing notes (using KTD):

1. I used the curl command to test the different content header types:
   curl -u koha:koha -i -H 'Accept:text/plain' http://127.0.0.1:8080/api/v1/authorities/419
Comment 5 Agustín Moyano 2022-12-07 18:33:43 UTC
Created attachment 144484 [details] [review]
Bug 31794: Add REST endpoint to get an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a GET request to /api/v1/authorities/{authid} with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
  - text/plain
5. Check that you get the authority in the corresponding format
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Agustín Moyano 2022-12-07 18:33:48 UTC
Created attachment 144485 [details] [review]
Bug 31794: (follow-up) Add description to authorities tag
Comment 7 Agustín Moyano 2022-12-12 13:09:18 UTC
(In reply to David Nind from comment #4)
> Testing notes (using KTD):
> 
> 1. I used the curl command to test the different content header types:
>    curl -u koha:koha -i -H 'Accept:text/plain'
> http://127.0.0.1:8080/api/v1/authorities/419

Hi David, thanks for testing.. sorry for the confusion, it was not the "Content-Type" header, it was "Accept" as you correctly used it
Comment 8 Pedro Amorim 2023-02-07 16:02:24 UTC
Created attachment 146338 [details] [review]
Bug 31794: Add REST endpoint to get an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a GET request to /api/v1/authorities/{authid} with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
  - text/plain
5. Check that you get the authority in the corresponding format
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 9 Pedro Amorim 2023-02-07 16:02:29 UTC
Created attachment 146339 [details] [review]
Bug 31794: (follow-up) Add description to authorities tag

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 10 Pedro Amorim 2023-02-07 16:05:32 UTC
Works as described, tests pass, wondering if we should DRY this in Authority.pm, or not worth:
152	      C4::Context->preference('marcflavour') eq 'UNIMARC'
153	      ? 'UNIMARCAUTH'
154	      : 'MARC21';
Comment 11 Marcel de Rooy 2023-02-24 09:04:24 UTC
Looks good to me. But I think we should add the public path there too just as we do for displaying a biblio record. Please add it. Thanks.
Comment 12 Tomás Cohen Arazi 2023-02-24 11:11:20 UTC
(In reply to Marcel de Rooy from comment #11)
> Looks good to me. But I think we should add the public path there too just
> as we do for displaying a biblio record. Please add it. Thanks.

I'm sorry, but wanting another more feature should not be failing this endpoint dev.
Comment 13 Marcel de Rooy 2023-03-03 07:43:41 UTC
(In reply to Tomás Cohen Arazi from comment #12)
> I'm sorry, but wanting another more feature should not be failing this
> endpoint dev.

Please note that the reason is not asking more functionality but lack of consistency. I agree that it is arguable if we should fail here, but we have been adding incomplete developments so often.. Opening a new report and expressing commitment to finalize might be helpful to convince me.
Comment 14 Marcel de Rooy 2023-03-03 10:54:43 UTC
See IRC. Resuming here now
Comment 15 Marcel de Rooy 2023-03-03 11:01:36 UTC
Come back here monday. Issues with the api test, but maybe related to a local dev change
Comment 16 Marcel de Rooy 2023-03-06 10:02:33 UTC
(In reply to Marcel de Rooy from comment #15)
> Come back here monday. Issues with the api test, but maybe related to a
> local dev change

There is trouble in the current API with authorised values somehow. See 32981 and 32983 that currently prevent me from testing this properly.
Comment 17 Marcel de Rooy 2023-03-06 10:07:35 UTC
Created attachment 147778 [details] [review]
Bug 31794: Add REST endpoint to get an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a GET request to /api/v1/authorities/{authid} with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
  - text/plain
5. Check that you get the authority in the corresponding format
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2023-03-06 10:07:38 UTC
Created attachment 147779 [details] [review]
Bug 31794: (follow-up) Add description to authorities tag

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2023-03-06 10:09:41 UTC
Well, i tested this now on top of commit 

commit fe872b792037ee2f8cce0d25f95c1e4f739ffe49
Author: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Date:   Thu Feb 2 12:45:04 2023 -0500

    Bug 32803: EnableItemGroups and EnableItemGroupHolds options are wrong

And that it works fine.
After 32981 things break.
Comment 20 Tomás Cohen Arazi 2023-03-06 17:46:54 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 21 Matt Blenkinsop 2023-03-09 09:42:41 UTC
Enhancement - not backporting to 22.11.x. 
Setting to Needs documenting. 
Nice work everyone!
Comment 22 Martin Renvoize 2023-03-09 09:43:24 UTC
The API's are self documenting (i.e. the swagger spec is the documentation and gets pushed to api.koha-community.org on pushes)