From 561c018be0e8bcf61ef196ee31427b37e9190412 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 18 May 2021 12:39:39 -0300 Subject: [PATCH] Bug 28369: (QA follow-up) Restore PUT with deprecation message This patch restores the PUT route, but adding a deprecation message. The controller method is the same, and relevant patch tests are duplicated but calling PUT, so it still gets tested. To test: 1. Apply this patch 2. Reload plack 3. Open https://<>/api/v1/.html and find the PUT /holds/{hold_id} route => SUCCESS: There's a deprecation wargning! 4. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: David Nind Signed-off-by: Martin Renvoize --- api/v1/swagger/paths/holds.json | 86 +++++++++++++++++++++++++++++++++ t/db_dependent/api/v1/holds.t | 39 ++++++++++++++- 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/api/v1/swagger/paths/holds.json b/api/v1/swagger/paths/holds.json index 2a863eefdf..66dd21c733 100644 --- a/api/v1/swagger/paths/holds.json +++ b/api/v1/swagger/paths/holds.json @@ -379,6 +379,92 @@ } } }, + "put": { + "x-mojo-to": "Holds#edit", + "operationId": "overwriteHold", + "tags": ["holds"], + "description": "This route is being deprecated and will be removed in future releases. Please migrate your project to use PATCH /holds/{hold_id} instead.", + "parameters": [{ + "$ref": "../parameters.json#/hold_id_pp" + }, { + "name": "body", + "in": "body", + "description": "A JSON object containing fields to modify", + "required": true, + "schema": { + "type": "object", + "properties": { + "priority": { + "description": "Position in waiting queue", + "type": "integer", + "minimum": 1 + }, + "pickup_library_id": { + "description": "Internal library identifier for the pickup library", + "type": "string" + }, + "suspended_until": { + "description": "Date until which the hold has been suspended", + "type": "string", + "format": "date-time" + } + }, + "additionalProperties": false + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "200": { + "description": "Updated hold", + "schema": { + "$ref": "../definitions.json#/hold" + } + }, + "400": { + "description": "Missing or wrong parameters", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Hold not allowed", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Hold not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "reserveforothers": "1" + } + } + }, "delete": { "x-mojo-to": "Holds#delete", "operationId": "deleteHold", diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 7d9cf295e4..e5a84bae3c 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -892,7 +892,7 @@ subtest 'pickup_locations() tests' => sub { subtest 'edit() tests' => sub { - plan tests => 20; + plan tests => 37; $schema->storage->txn_begin; @@ -963,6 +963,12 @@ subtest 'edit() tests' => sub { ->status_is(400) ->json_is({ error => 'The supplied pickup location is not valid' }); + $t->put_ok( "//$userid:$password@/api/v1/holds/" + . $biblio_hold->id + => json => $biblio_hold_data ) + ->status_is(400) + ->json_is({ error => 'The supplied pickup location is not valid' }); + $biblio_hold->discard_changes; is( $biblio_hold->branchcode, $library_3->branchcode, 'branchcode remains untouched' ); @@ -972,6 +978,12 @@ subtest 'edit() tests' => sub { ->status_is(200) ->json_has( '/pickup_library_id' => $library_1->id ); + $t->put_ok( "//$userid:$password@/api/v1/holds/" . $biblio_hold->id + => { 'x-koha-override' => 'any' } + => json => $biblio_hold_data ) + ->status_is(200) + ->json_has( '/pickup_library_id' => $library_1->id ); + $biblio_hold_data->{pickup_library_id} = $library_2->branchcode; $t->patch_ok( "//$userid:$password@/api/v1/holds/" . $biblio_hold->id @@ -981,6 +993,14 @@ subtest 'edit() tests' => sub { $biblio_hold->discard_changes; is( $biblio_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); + $t->put_ok( "//$userid:$password@/api/v1/holds/" + . $biblio_hold->id + => json => $biblio_hold_data ) + ->status_is(200); + + $biblio_hold->discard_changes; + is( $biblio_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); + # Test item-level holds my $item_hold = $builder->build_object( { @@ -1006,6 +1026,12 @@ subtest 'edit() tests' => sub { ->status_is(400) ->json_is({ error => 'The supplied pickup location is not valid' }); + $t->put_ok( "//$userid:$password@/api/v1/holds/" + . $item_hold->id + => json => $item_hold_data ) + ->status_is(400) + ->json_is({ error => 'The supplied pickup location is not valid' }); + $item_hold->discard_changes; is( $item_hold->branchcode, $library_3->branchcode, 'branchcode remains untouched' ); @@ -1015,12 +1041,23 @@ subtest 'edit() tests' => sub { ->status_is(200) ->json_has( '/pickup_library_id' => $library_1->id ); + $t->put_ok( "//$userid:$password@/api/v1/holds/" . $item_hold->id + => { 'x-koha-override' => 'any' } + => json => $item_hold_data ) + ->status_is(200) + ->json_has( '/pickup_library_id' => $library_1->id ); + $item_hold_data->{pickup_library_id} = $library_2->branchcode; $t->patch_ok( "//$userid:$password@/api/v1/holds/" . $item_hold->id => json => $item_hold_data ) ->status_is(200); + $t->put_ok( "//$userid:$password@/api/v1/holds/" + . $item_hold->id + => json => $item_hold_data ) + ->status_is(200); + $item_hold->discard_changes; is( $item_hold->branchcode, $library_2->id, 'Pickup location changed correctly' ); -- 2.20.1