Attachment 173123 Details for Bug 30661 – Bug 30661: (follow-up) Add separate endpoints

[patch] Bug 30661: (follow-up) Add separate endpoints

Bug-30661-follow-up-Add-separate-endpoints.patch (text/plain), 12.51 KB, created by David Nind on 2024-10-22 10:01:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2024-10-22 10:01:59 UTC

Size: 12.51 KB

patch

obsolete

>From 0b9532ab59e1e6b28d8709361082c1b17011e731 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Johanna=20R=C3=A4is=C3=A4?= <johanna.raisa@gmail.com>
>Date: Tue, 22 Oct 2024 08:53:09 +0300
>Subject: [PATCH] Bug 30661: (follow-up) Add separate endpoints
>
>This patch adds separate endpoints for updating hold_date and expiration_date.
>
>Test plan:
>1) prove t/db_dependent/api/v1/holds.t
>
>Sponsored-by: Koha-Suomi Oy
>Signed-off-by: David Nind <david@davidnind.com>
>---
> Koha/REST/V1/Holds.pm           |  77 +++++++++++++++++
> api/v1/swagger/paths/holds.yaml | 142 ++++++++++++++++++++++++++++++++
> api/v1/swagger/swagger.yaml     |   4 +
> t/db_dependent/api/v1/holds.t   | 124 +++++++++++++++++++++++++++-
> 4 files changed, 346 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm
>index c4398bb59d..0cfb8da89b 100644
>--- a/Koha/REST/V1/Holds.pm
>+++ b/Koha/REST/V1/Holds.pm
>@@ -560,4 +560,81 @@ sub update_pickup_location {
>     };
> }
> 
>+=head3 update_hold_date
>+
>+Method that handles modifying the hold date of a Koha::Hold object
>+
>+=cut
>+
>+sub update_hold_date {
>+    my $c = shift->openapi->valid_input or return;
>+
>+    my $hold_id   = $c->param('hold_id');
>+    my $body      = $c->req->json;
>+    my $hold_date = $body->{hold_date};
>+
>+    my $hold = Koha::Holds->find($hold_id);
>+
>+    unless ( C4::Context->preference('AllowHoldDateInFuture') ) {
>+        return $c->render(
>+            status  => 403,
>+            openapi => { error => "Update not allowed" }
>+        );
>+    }
>+
>+    unless ($hold) {
>+        return $c->render(
>+            status  => 404,
>+            openapi => { error => "Hold not found" }
>+        );
>+    }
>+
>+    return try {
>+
>+        $hold->set( { reservedate => $hold_date } )->store;
>+
>+        return $c->render(
>+            status  => 200,
>+            openapi => { hold_date => $hold_date }
>+        );
>+    } catch {
>+        $c->unhandled_exception($_);
>+    };
>+}
>+
>+=head3 update_expiration_date
>+
>+Method that handles modifying the expiration date of a Koha::Hold object
>+
>+=cut
>+
>+sub update_expiration_date {
>+    my $c = shift->openapi->valid_input or return;
>+
>+    my $hold_id         = $c->param('hold_id');
>+    my $body            = $c->req->json;
>+    my $expiration_date = $body->{expiration_date};
>+
>+    my $hold = Koha::Holds->find($hold_id);
>+
>+    unless ($hold) {
>+        return $c->render(
>+            status  => 404,
>+            openapi => { error => "Hold not found" }
>+        );
>+    }
>+
>+    return try {
>+
>+        $hold->set( { expirationdate => $expiration_date } )->store;
>+
>+        return $c->render(
>+            status  => 200,
>+            openapi => { expiration_date => $expiration_date }
>+        );
>+    } catch {
>+        $c->unhandled_exception($_);
>+    };
>+}
>+
> 1;
>diff --git a/api/v1/swagger/paths/holds.yaml b/api/v1/swagger/paths/holds.yaml
>index 6b5c792095..55a5d5fde5 100644
>--- a/api/v1/swagger/paths/holds.yaml
>+++ b/api/v1/swagger/paths/holds.yaml
>@@ -755,3 +755,145 @@
>     x-koha-authorization:
>       permissions:
>         reserveforothers: place_holds
>+"/holds/{hold_id}/hold_date":
>+  put:
>+    x-mojo-to: Holds#update_hold_date
>+    operationId: updateHoldDate
>+    tags:
>+      - holds
>+    summary: Update hold date
>+    parameters:
>+      - $ref: "../swagger.yaml#/parameters/hold_id_pp"
>+      - name: body
>+        in: body
>+        description: A JSON object containing the new hold date
>+        required: true
>+        schema:
>+          type: object
>+          properties:
>+            hold_date:
>+              description: The date the hold was placed
>+              type: string
>+              format: date
>+          additionalProperties: false
>+    produces:
>+      - application/json
>+    responses:
>+      "200":
>+        description: The new hold date value for the hold
>+        schema:
>+          type: object
>+          properties:
>+            hold_date:
>+              description: The date the hold was placed
>+              type: string
>+              format: date
>+          additionalProperties: false
>+      "400":
>+        description: Bad request
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "401":
>+        description: Authentication required
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "403":
>+        description: Access forbidden
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "404":
>+        description: Hold not found
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "409":
>+        description: |
>+          Unable to perform action on hold. Possible `error_code` attribute values:
>+
>+          * `hold_waiting`
>+          * `hold_in_transit`
>+          * `hold_in_processing`
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "500":
>+        description: |
>+          Internal server error. Possible `error_code` attribute values:
>+
>+          * `internal_server_error`
>+      "503":
>+        description: Under maintenance
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+    x-koha-authorization:
>+      permissions:
>+        reserveforothers: place_holds
>+"/holds/{hold_id}/expiration_date":
>+  put:
>+    x-mojo-to: Holds#update_expiration_date
>+    operationId: updateHoldExpirationDate
>+    tags:
>+      - holds
>+    summary: Update hold expiration date
>+    parameters:
>+      - $ref: "../swagger.yaml#/parameters/hold_id_pp"
>+      - name: body
>+        in: body
>+        description: A JSON object containing the new expiration date
>+        required: true
>+        schema:
>+          type: object
>+          properties:
>+            expiration_date:
>+              description: The date the hold expires
>+              type: string
>+              format: date
>+          additionalProperties: false
>+    produces:
>+      - application/json
>+    responses:
>+      "200":
>+        description: The new expiration date value for the hold
>+        schema:
>+          type: object
>+          properties:
>+            expiration_date:
>+              description: The date the hold expires
>+              type: string
>+              format: date
>+          additionalProperties: false
>+      "400":
>+        description: Bad request
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "401":
>+        description: Authentication required
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "403":
>+        description: Access forbidden
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "404":
>+        description: Hold not found
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "409":
>+        description: |
>+          Unable to perform action on hold. Possible `error_code` attribute values:
>+
>+          * `hold_waiting`
>+          * `hold_in_transit`
>+          * `hold_in_processing`
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+      "500":
>+        description: |
>+          Internal server error. Possible `error_code` attribute values:
>+
>+          * `internal_server_error`
>+      "503":
>+        description: Under maintenance
>+        schema:
>+          $ref: "../swagger.yaml#/definitions/error"
>+    x-koha-authorization:
>+      permissions:
>+        reserveforothers: place_holds
>diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml
>index dd72361380..83ffa3f365 100644
>--- a/api/v1/swagger/swagger.yaml
>+++ b/api/v1/swagger/swagger.yaml
>@@ -391,6 +391,10 @@ paths:
>     $ref: "./paths/holds.yaml#/~1holds~1{hold_id}~1priority"
>   "/holds/{hold_id}/suspension":
>     $ref: "./paths/holds.yaml#/~1holds~1{hold_id}~1suspension"
>+  "/holds/{hold_id}/hold_date":
>+    $ref: "./paths/holds.yaml#/~1holds~1{hold_id}~1hold_date"
>+  "/holds/{hold_id}/expiration_date":
>+    $ref: "./paths/holds.yaml#/~1holds~1{hold_id}~1expiration_date"
>   /ill/backends:
>     $ref: ./paths/ill_backends.yaml#/~1ill~1backends
>   "/ill/backends/{ill_backend_id}":
>diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t
>index 63be8e9d18..db83044e63 100755
>--- a/t/db_dependent/api/v1/holds.t
>+++ b/t/db_dependent/api/v1/holds.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 14;
>+use Test::More tests => 16;
> use Test::MockModule;
> use Test::Mojo;
> use t::lib::TestBuilder;
>@@ -1575,3 +1575,125 @@ subtest 'delete() tests' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'PUT /holds/{hold_id}/hold_date tests' => sub {
>+
>+    plan tests => 10;
>+
>+    $schema->storage->txn_begin;
>+
>+    my $password = 'AbcdEFG123';
>+
>+    my $library_1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } );
>+
>+    my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } );
>+    $patron->set_password( { password => $password, skip_validation => 1 } );
>+    my $userid = $patron->userid;
>+    $builder->build(
>+        {
>+            source => 'UserPermission',
>+            value  => {
>+                borrowernumber => $patron->borrowernumber,
>+                module_bit     => 6,
>+                code           => 'place_holds',
>+            },
>+        }
>+    );
>+
>+    # Disable logging
>+    t::lib::Mocks::mock_preference( 'HoldsLog',      0 );
>+    t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
>+
>+    my $biblio = $builder->build_sample_biblio;
>+
>+    # biblio-level hold
>+    my $hold = Koha::Holds->find(
>+        AddReserve(
>+            {
>+                branchcode     => $library_1->branchcode,
>+                borrowernumber => $patron->borrowernumber,
>+                biblionumber   => $biblio->biblionumber,
>+                priority       => 1,
>+                itemnumber     => undef,
>+            }
>+        )
>+    );
>+
>+    t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 0 );
>+
>+    $t->put_ok(
>+        "//$userid:$password@/api/v1/holds/" . $hold->id . "/hold_date" => json => { hold_date => '2022-01-01' } )
>+        ->status_is(403)->json_is( { error => 'Update not allowed' } );
>+
>+    t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 );
>+
>+    # Attempt to use an invalid pickup locations ends in 400
>+    $t->put_ok( "//$userid:$password@/api/v1/holds/0" . "/hold_date" => json => { hold_date => '2022-01-01' } )
>+        ->status_is(404)->json_is( { error => 'Hold not found' } );
>+
>+    $t->put_ok(
>+        "//$userid:$password@/api/v1/holds/" . $hold->id . "/hold_date" => json => { hold_date => '2022-01-01' } )
>+        ->status_is(200)->json_is( { hold_date => '2022-01-01' } );
>+
>+    is( $hold->discard_changes->reservedate, '2022-01-01', 'hold date changed correctly' );
>+
>+    $schema->storage->txn_rollback;
>+};
>+
>+subtest 'PUT /holds/{hold_id}/expiration_date tests' => sub {
>+
>+    plan tests => 7;
>+
>+    $schema->storage->txn_begin;
>+
>+    my $password = 'AbcdEFG123';
>+
>+    my $library_1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } );
>+
>+    my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } );
>+    $patron->set_password( { password => $password, skip_validation => 1 } );
>+    my $userid = $patron->userid;
>+    $builder->build(
>+        {
>+            source => 'UserPermission',
>+            value  => {
>+                borrowernumber => $patron->borrowernumber,
>+                module_bit     => 6,
>+                code           => 'place_holds',
>+            },
>+        }
>+    );
>+
>+    # Disable logging
>+    t::lib::Mocks::mock_preference( 'HoldsLog',      0 );
>+    t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
>+
>+    my $biblio = $builder->build_sample_biblio;
>+
>+    # biblio-level hold
>+    my $hold = Koha::Holds->find(
>+        AddReserve(
>+            {
>+                branchcode     => $library_1->branchcode,
>+                borrowernumber => $patron->borrowernumber,
>+                biblionumber   => $biblio->biblionumber,
>+                priority       => 1,
>+                itemnumber     => undef,
>+            }
>+        )
>+    );
>+
>+    # Attempt to use an invalid pickup locations ends in 400
>+    $t->put_ok(
>+        "//$userid:$password@/api/v1/holds/0" . "/expiration_date" => json => { expiration_date => '2022-01-01' } )
>+        ->status_is(404)->json_is( { error => 'Hold not found' } );
>+
>+    $t->put_ok( "//$userid:$password@/api/v1/holds/"
>+            . $hold->id
>+            . "/expiration_date" => json => { expiration_date => '2022-01-01' } )->status_is(200)
>+        ->json_is( { expiration_date => '2022-01-01' } );
>+
>+    is( $hold->discard_changes->expirationdate, '2022-01-01', 'expiration date changed correctly' );
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.39.5

Actions: View | Diff | Splinter Review

Attachments on bug 30661: 139973 | 139974 | 140221 | 140359 | 173116 | 173117 | 173122 | 173123 | 173209