Bug 31794

Summary: Add GET endpoint for Authorities
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: REST APIAssignee: Agustín Moyano <agustinmoyano>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: new feature    
Priority: P5 - low CC: david, dcook, m.de.rooy, martin.renvoize, matt.blenkinsop, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on:    
Bug Blocks: 26611, 33124, 31790, 31793    
Attachments: Bug 31974: Regression: Bug 31813 incorrectly affected labels in the header search
Bug 31794: Add REST endpoint to get an authority
Bug 31794: Add REST endpoint to get an authority
Bug 31794: Add REST endpoint to get an authority
Bug 31794: (follow-up) Add description to authorities tag
Bug 31794: Add REST endpoint to get an authority
Bug 31794: (follow-up) Add description to authorities tag
Bug 31794: Add REST endpoint to get an authority
Bug 31794: (follow-up) Add description to authorities tag

Description Nick Clemens (kidclamp) 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)