Bug 31795 - Add POST endpoint for Authorities
Summary: Add POST endpoint for Authorities
Status: CLOSED 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:
URL:
Keywords:
Depends on: 31793
Blocks: 31790 31796
  Show dependency treegraph
 
Reported: 2022-10-14 14:35 UTC by Nick Clemens
Modified: 2023-12-28 20:43 UTC (History)
4 users (show)

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


Attachments
Bug 31795: Add REST endpoint to add an authority (9.81 KB, patch)
2022-12-13 16:03 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31795: Add REST endpoint to add an authority (9.94 KB, patch)
2022-12-20 21:06 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31795: (follow-up) fix header name (3.55 KB, patch)
2022-12-20 21:06 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31795: Add REST endpoint to add an authority (9.99 KB, patch)
2022-12-20 22:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: (follow-up) fix header name (3.60 KB, patch)
2022-12-20 22:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: Add REST endpoint to add an authority (9.68 KB, patch)
2023-04-10 12:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31795: (QA follow-up) Use x-koha-override header (10.15 KB, patch)
2023-04-10 17:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31795: Fix required permissions (1.84 KB, patch)
2023-04-10 17:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority (2.11 KB, patch)
2023-04-11 15:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31795: Add REST endpoint to add an authority (9.68 KB, patch)
2023-04-11 18:11 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: (QA follow-up) Use x-koha-override header (10.20 KB, patch)
2023-04-11 18:11 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: Fix required permissions (1.88 KB, patch)
2023-04-11 18:11 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority (2.15 KB, patch)
2023-04-11 18:11 UTC, David Nind
Details | Diff | Splinter Review
Bug 31795: Add REST endpoint to add an authority (9.73 KB, patch)
2023-04-28 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31795: (QA follow-up) Use x-koha-override header (10.25 KB, patch)
2023-04-28 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31795: Fix required permissions (1.93 KB, patch)
2023-04-28 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority (2.21 KB, patch)
2023-04-28 13:12 UTC, Nick Clemens
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:35:16 UTC

    
Comment 1 Agustín Moyano 2022-12-13 16:03:40 UTC Comment hidden (obsolete)
Comment 2 David Nind 2022-12-18 03:37:11 UTC
I had a go at testing using Postman, but I'm not familiar enough with APIs etc., to know how to work out what needs to be in the body for the format.

If you could provide an example of what to put in the body for one of the formats, I'll have another go.
Comment 3 Agustín Moyano 2022-12-20 18:35:34 UTC
(In reply to David Nind from comment #2)
> I had a go at testing using Postman, but I'm not familiar enough with APIs
> etc., to know how to work out what needs to be in the body for the format.
> 
> If you could provide an example of what to put in the body for one of the
> formats, I'll have another go.

Hi David, I got this from KTD, but an example of marcxml would be

<?xml version="1.0" encoding="UTF-8"?>
<record
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"
    xmlns="http://www.loc.gov/MARC21/slim">

  <leader>00390nz  a2200133o  4500</leader>
  <controlfield tag="001">1707</controlfield>
  <controlfield tag="003">csc</controlfield>
  <controlfield tag="005">20140507162102.0</controlfield>
  <controlfield tag="008">140507|| aca||aabn           | a|a     d</controlfield>
  <datafield tag="040" ind1=" " ind2=" ">
    <subfield code="a">csc</subfield>
    <subfield code="c">csc</subfield>
  </datafield>
  <datafield tag="150" ind1=" " ind2=" ">
    <subfield code="a">C++ (Computer program language)</subfield>
  </datafield>
  <datafield tag="667" ind1=" " ind2=" ">
    <subfield code="a">Machine generated authority record.</subfield>
  </datafield>
  <datafield tag="670" ind1=" " ind2=" ">
    <subfield code="a">Work cat.: (csc)0: Josuttis, Nicolai M. 5582, The C++ standard library :, c2012.</subfield>
  </datafield>
  <datafield tag="942" ind1=" " ind2=" ">
    <subfield code="a">TOPIC_TERM</subfield>
  </datafield>
</record>

Maybe you can change some data so that Koha won't think it's a duplicate

Cheers!
Comment 4 Agustín Moyano 2022-12-20 21:06:41 UTC Comment hidden (obsolete)
Comment 5 Agustín Moyano 2022-12-20 21:06:46 UTC Comment hidden (obsolete)
Comment 6 David Nind 2022-12-20 22:23:23 UTC
Created attachment 144749 [details] [review]
Bug 31795: Add REST endpoint to add an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Make a POST request to /api/v1/authorities with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>"
5. Check that the authority is created
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-12-20 22:23:28 UTC
Created attachment 144750 [details] [review]
Bug 31795: (follow-up) fix header name

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-12-20 22:36:22 UTC
(In reply to Agustín Moyano from comment #3)
> (In reply to David Nind from comment #2)
>> ...
> Hi David, I got this from KTD, but an example of marcxml would be
> ...

Thanks Agustín!

I removed the 001 tag and changed the name in the 150 tag.

In Postman I created a new HTTP request and added these details:
1. Changed to a POST with the URL as http://127.0.0.1:8081/api/v1/authorities
2. Body: changed to 'raw', pasted in the amended XML for the MARC authority record, and changed the type to XML
3. Added Headers: 
   . x-authority-type: used TOPIC_TERM as the value
   . Content-Type: used application/marcxml+xml as the value
4. For authorization, add the Basic Auth type with koha as the username and password

The new authority record was then created!

David
Comment 9 Marcel de Rooy 2023-03-07 12:50:29 UTC
Applying: Bug 31795: Add REST endpoint to add an authority
error: sha1 information is lacking or useless (api/v1/swagger/swagger.yaml).
error: could not build fake ancestor
Comment 10 Tomás Cohen Arazi 2023-04-10 12:29:11 UTC
Created attachment 149380 [details] [review]
Bug 31795: Add REST endpoint to add an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Make a POST request to /api/v1/authorities with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>"
5. Check that the authority is created
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 Tomás Cohen Arazi 2023-04-10 17:36:17 UTC
Created attachment 149407 [details] [review]
Bug 31795: (QA follow-up) Use x-koha-override header

Despite its title, this patch does a couple more things.

1. The tests are rewritten to cover more things, and also to avoid
   deleting all authorities inside the transaction. It is really not
   required.
2. It makes the endpoint rely on the already generically implemented
   x-koha-override header, which is intended for the same use case as
   x-confirm-not-duplicate is for.
3. It changes the return codes to match the coding guidelines [1]
4. Only checks for duplicates if no override passed.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> SUCCESS: Tests pass!
2. Apply this follow-up
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2023-04-10 17:54:44 UTC
Created attachment 149409 [details] [review]
Bug 31795: Fix required permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 David Nind 2023-04-10 19:41:03 UTC
The tests don't pass after applying the patches:

prove t/db_dependent/api/v1/authorities.t
t/db_dependent/api/v1/authorities.t .. 1/3 
    #   Failed test '409 Conflict'
    #   at t/db_dependent/api/v1/authorities.t line 204.
    #          got: '201'
    #     expected: '409'

    #   Failed test 'Location header is only set when the new resource is created'
    #   at t/db_dependent/api/v1/authorities.t line 204.

    #   Failed test 'similar match for JSON Pointer "/error"'
    #   at t/db_dependent/api/v1/authorities.t line 204.
    #                   undef
    #     doesn't match '(?^u:Duplicate record (\d*))'

    #   Failed test 'exact match for JSON Pointer "/error_code"'
    #   at t/db_dependent/api/v1/authorities.t line 204.
    #          got: undef
    #     expected: 'duplicate'
    # Looks like you failed 4 tests of 19.
t/db_dependent/api/v1/authorities.t .. 3/3 
#   Failed test 'post() tests'
#   at t/db_dependent/api/v1/authorities.t line 227.
# Looks like you failed 1 test of 3.
t/db_dependent/api/v1/authorities.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests 

Test Summary Report
-------------------
t/db_dependent/api/v1/authorities.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
Files=1, Tests=3,  4 wallclock secs ( 0.02 usr  0.00 sys +  3.67 cusr  0.14 csys =  3.83 CPU)
Result: FAIL
Comment 14 Tomás Cohen Arazi 2023-04-11 14:42:29 UTC
(In reply to David Nind from comment #13)
> The tests don't pass after applying the patches:
> 
> prove t/db_dependent/api/v1/authorities.t
> t/db_dependent/api/v1/authorities.t .. 1/3 
>     #   Failed test '409 Conflict'
>     #   at t/db_dependent/api/v1/authorities.t line 204.
>     #          got: '201'
>     #     expected: '409'

This is odd. It fails on Zebra, and passes on ES. It means the FindDuplicateAuthority method is not behaving the same on each.

I'll mock the call to the method and file a new bug for the method specifically.
Comment 15 Tomás Cohen Arazi 2023-04-11 15:38:27 UTC
Created attachment 149451 [details] [review]
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority

When running against Zebra, this tests fail because it requires the
record to be indexed timely. In theory, a 5 seconds delay should be
enough for the indexer to pick up. But that's not the point of the test,
and we should better just mock the query in this case.

To test:
1. Have ktd launch with zebra:
   $ ktd up -d
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Launch ktd with ES:
   $ ktd down
   $ ktd --es7 up -d
6. Repeat 2
=> SUCCESS: Tests pass!
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 David Nind 2023-04-11 18:11:37 UTC
Created attachment 149459 [details] [review]
Bug 31795: Add REST endpoint to add an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Make a POST request to /api/v1/authorities with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>"
5. Check that the authority is created
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 David Nind 2023-04-11 18:11:40 UTC
Created attachment 149460 [details] [review]
Bug 31795: (QA follow-up) Use x-koha-override header

Despite its title, this patch does a couple more things.

1. The tests are rewritten to cover more things, and also to avoid
   deleting all authorities inside the transaction. It is really not
   required.
2. It makes the endpoint rely on the already generically implemented
   x-koha-override header, which is intended for the same use case as
   x-confirm-not-duplicate is for.
3. It changes the return codes to match the coding guidelines [1]
4. Only checks for duplicates if no override passed.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> SUCCESS: Tests pass!
2. Apply this follow-up
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 18 David Nind 2023-04-11 18:11:43 UTC
Created attachment 149461 [details] [review]
Bug 31795: Fix required permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 19 David Nind 2023-04-11 18:11:46 UTC
Created attachment 149462 [details] [review]
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority

When running against Zebra, this tests fail because it requires the
record to be indexed timely. In theory, a 5 seconds delay should be
enough for the indexer to pick up. But that's not the point of the test,
and we should better just mock the query in this case.

To test:
1. Have ktd launch with zebra:
   $ ktd up -d
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Launch ktd with ES:
   $ ktd down
   $ ktd --es7 up -d
6. Repeat 2
=> SUCCESS: Tests pass!
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 20 Nick Clemens 2023-04-28 13:12:33 UTC
Created attachment 150370 [details] [review]
Bug 31795: Add REST endpoint to add an authority

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Make a POST request to /api/v1/authorities with one of the following content type header
  - application/json
  - application/marcxml+xml
  - application/marc-in-json
  - application/marc
4. If content type is other than application/json, then add the following header to the request: "x-authority-type: <auth type code>"
5. Check that the authority is created
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens 2023-04-28 13:12:35 UTC
Created attachment 150371 [details] [review]
Bug 31795: (QA follow-up) Use x-koha-override header

Despite its title, this patch does a couple more things.

1. The tests are rewritten to cover more things, and also to avoid
   deleting all authorities inside the transaction. It is really not
   required.
2. It makes the endpoint rely on the already generically implemented
   x-koha-override header, which is intended for the same use case as
   x-confirm-not-duplicate is for.
3. It changes the return codes to match the coding guidelines [1]
4. Only checks for duplicates if no override passed.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> SUCCESS: Tests pass!
2. Apply this follow-up
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

[1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.1_POST

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2023-04-28 13:12:37 UTC
Created attachment 150372 [details] [review]
Bug 31795: Fix required permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Nick Clemens 2023-04-28 13:12:39 UTC
Created attachment 150373 [details] [review]
Bug 31795: (follow-up) Mock call to FindDuplicateAuthority

When running against Zebra, this tests fail because it requires the
record to be indexed timely. In theory, a 5 seconds delay should be
enough for the indexer to pick up. But that's not the point of the test,
and we should better just mock the query in this case.

To test:
1. Have ktd launch with zebra:
   $ ktd up -d
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/authorities.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Launch ktd with ES:
   $ ktd down
   $ ktd --es7 up -d
6. Repeat 2
=> SUCCESS: Tests pass!
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Tomás Cohen Arazi 2023-05-05 13:15:02 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 25 Martin Renvoize 2023-05-12 14:40:16 UTC
Enhancement - not backporting to 22.11.x

Good work everyone!