Bug 31793 - Add DELETE endpoint for Authorities
Summary: Add DELETE 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: Marcel de Rooy
URL:
Keywords:
Depends on: 31794
Blocks: 31790 31795
  Show dependency treegraph
 
Reported: 2022-10-14 14:33 UTC by Nick Clemens
Modified: 2023-12-28 20:43 UTC (History)
6 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 31793: Add REST endpoint to delete authorities (5.75 KB, patch)
2022-12-07 15:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 31793: Add REST endpoint to delete authorities (5.80 KB, patch)
2022-12-18 00:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 31793: Add REST endpoint to delete authorities (5.80 KB, patch)
2023-03-06 20:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31793: (QA follow-up) DelAuthority doesn't return errors (1.44 KB, patch)
2023-03-06 20:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31793: (QA follow-up) Require editauthorities flag (2.55 KB, patch)
2023-03-06 20:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31793: Add REST endpoint to delete authorities (5.89 KB, patch)
2023-03-07 12:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31793: (QA follow-up) DelAuthority doesn't return errors (1.54 KB, patch)
2023-03-07 12:45 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 31793: (QA follow-up) Require editauthorities flag (2.64 KB, patch)
2023-03-07 12:45 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:33:35 UTC

    
Comment 1 Agustín Moyano 2022-12-07 15:21:46 UTC
Created attachment 144472 [details] [review]
Bug 31793: Add REST endpoint to delete authorities

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a DELETE request to /api/v1/authorities/{authid}
5. Check that the authority was deleted
6. Sign off
Comment 2 David Nind 2022-12-18 00:50:36 UTC
Created attachment 144705 [details] [review]
Bug 31793: Add REST endpoint to delete authorities

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a DELETE request to /api/v1/authorities/{authid}
5. Check that the authority was deleted
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2022-12-18 01:01:08 UTC
Testing notes (using KTD):

1. I used this command to delete an existing authority: curl -u koha:koha --request DELETE http://127.0.0.1:8080/api/v1/authorities/419

2. This authority (419) had associated records - the authority deleted was removed from the record.
Comment 4 Marcel de Rooy 2023-03-06 10:27:11 UTC
+    return try {
+        my $error = DelAuthority( { authid => $authority->authid } );
+
+        if ($error) {
+            return $c->render(
+                status  => 409,

I would probably go for 500 as long as we dont know what has been going on in DelAuthority.
Comment 5 Marcel de Rooy 2023-03-06 10:29:24 UTC
+      "404":
+        description: Biblio not found
+        schema:
+          $ref: "../swagger.yaml#/definitions/error"

Copy and paste. Correct !

+      "409":
+        description: Unable to perform action on biblio
Comment 6 Marcel de Rooy 2023-03-06 10:30:12 UTC
+    x-koha-authorization:
+      permissions:
+        editcatalogue: edit_catalogue

I would probably expect:
flagsrequired => {editauthorities => 1},
Comment 7 Tomás Cohen Arazi 2023-03-06 20:43:50 UTC
Created attachment 147808 [details] [review]
Bug 31793: Add REST endpoint to delete authorities

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a DELETE request to /api/v1/authorities/{authid}
5. Check that the authority was deleted
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Tomás Cohen Arazi 2023-03-06 20:43:53 UTC
Created attachment 147809 [details] [review]
Bug 31793: (QA follow-up) DelAuthority doesn't return errors

DelAuthority doesn't return an error, so the only possible error
conditions are exceptions, that are unknown for now. Removing the error
checking control structure and the 409 error returning, which is
inherited from DelBiblio's call on the biblios endpoint.

It will now just return a 500 in the event of an unhandled internal
error.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2023-03-06 20:43:56 UTC
Created attachment 147810 [details] [review]
Bug 31793: (QA follow-up) Require editauthorities flag

This patch makes the route require the right permissions. Tests are
adjusted to reflect this new situation.

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

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Marcel de Rooy 2023-03-07 12:45:14 UTC
Created attachment 147831 [details] [review]
Bug 31793: Add REST endpoint to delete authorities

To test:
1. Apply patch
2. Set RESTBasicAuth preference to true
3. Get the id of an authority
4. Make a DELETE request to /api/v1/authorities/{authid}
5. Check that the authority was deleted
6. Sign off

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2023-03-07 12:45:16 UTC
Created attachment 147832 [details] [review]
Bug 31793: (QA follow-up) DelAuthority doesn't return errors

DelAuthority doesn't return an error, so the only possible error
conditions are exceptions, that are unknown for now. Removing the error
checking control structure and the 409 error returning, which is
inherited from DelBiblio's call on the biblios endpoint.

It will now just return a 500 in the event of an unhandled internal
error.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2023-03-07 12:45:18 UTC
Created attachment 147833 [details] [review]
Bug 31793: (QA follow-up) Require editauthorities flag

This patch makes the route require the right permissions. Tests are
adjusted to reflect this new situation.

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

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Tomás Cohen Arazi 2023-03-10 13:23:56 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Matt Blenkinsop 2023-03-10 16:31:56 UTC
Enhancement - not backporting to 22.11.x

Good work everyone!