Bugzilla – Attachment 163834 Details for
Bug 35353
Add API endpoint to fetch patron's previous holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35353: Add REST API endpoint to retrieve old holds
Bug-35353-Add-REST-API-endpoint-to-retrieve-old-ho.patch (text/plain), 11.84 KB, created by
Matt Blenkinsop
on 2024-03-25 16:31:16 UTC
(
hide
)
Description:
Bug 35353: Add REST API endpoint to retrieve old holds
Filename:
MIME Type:
Creator:
Matt Blenkinsop
Created:
2024-03-25 16:31:16 UTC
Size:
11.84 KB
patch
obsolete
>From 850d633d89363fb688f8f10b941604d484e90700 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 25 Mar 2024 16:42:18 +0100 >Subject: [PATCH] Bug 35353: Add REST API endpoint to retrieve old holds > >Same as checkout but for holds, we need to provide a way to retrieve old >holds for a patron. > >Test plan: >Create some holds for a patron, cancel and fulfill some, then use the >REST API endpoint with the new 'old' flag set to 1 > /api/v1/patrons/42/holds?old=1 > >Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> >--- > Koha/Hold.pm | 29 ++++++++++++++++++++-- > Koha/Old/Hold.pm | 26 +++++++++++++++++++ > Koha/REST/V1/Holds.pm | 14 ++++++++--- > Koha/REST/V1/Patrons/Holds.pm | 18 ++++++++------ > api/v1/swagger/definitions/hold.yaml | 15 +++++++++++ > api/v1/swagger/paths/holds.yaml | 7 ++++++ > api/v1/swagger/paths/patrons_holds.yaml | 9 +++++++ > t/db_dependent/Koha/Hold.t | 19 +++++++++++++- > t/db_dependent/Koha/Old/Hold.t | 27 +++++++++++++++++++- > t/db_dependent/api/v1/patrons_holds.t | 33 ++++++++++++++++++++----- > 10 files changed, 176 insertions(+), 21 deletions(-) > >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index a74ed95b58..6ea8fc5756 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -574,13 +574,25 @@ sub item_group { > > =head3 branch > >-Returns the related Koha::Library object for this Hold >+Returns the related Koha::Library object for this hold >+ >+DEPRECATED > > =cut > > sub branch { >+ return shift->pickup_library(@_); >+} >+ >+=head3 pickup_library >+ >+Returns the related Koha::Library object for this hold >+ >+=cut >+ >+sub pickup_library { > my ($self) = @_; >- my $rs = $self->_result->branchcode; >+ my $rs = $self->_result->pickup_library; > return Koha::Library->_new_from_dbic($rs); > } > >@@ -1041,6 +1053,19 @@ sub can_update_pickup_location_opac { > return 0; > } > >+=head3 strings_map >+ >+Returns a map of column name to string representations including the string. >+ >+=cut >+ >+sub strings_map { >+ my ($self) = @_; >+ return { >+ pickup_library_id => { str => $self->branch->branchname, type => 'library' }, >+ }; >+} >+ > =head2 Internal methods > > =head3 _type >diff --git a/Koha/Old/Hold.pm b/Koha/Old/Hold.pm >index 91f2adb39c..8f97163c6e 100644 >--- a/Koha/Old/Hold.pm >+++ b/Koha/Old/Hold.pm >@@ -47,6 +47,32 @@ sub biblio { > return Koha::Biblio->_new_from_dbic($rs); > } > >+=head3 item >+ >+Returns the related Koha::Item object for this old Hold >+ >+=cut >+ >+sub item { >+ my ($self) = @_; >+ my $rs = $self->_result->itemnumber; >+ return unless $rs; >+ return Koha::Item->_new_from_dbic($rs); >+} >+ >+=head3 pickup_library >+ >+Returns the related Koha::Biblio object for this old hold >+ >+=cut >+ >+sub pickup_library { >+ my ($self) = @_; >+ my $rs = $self->_result->pickup_library; >+ return unless $rs; >+ return Koha::Library->_new_from_dbic($rs); >+} >+ > =head3 anonymize > > $old_hold->anonymize(); >diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm >index ff162cd945..e7f0d64341 100644 >--- a/Koha/REST/V1/Holds.pm >+++ b/Koha/REST/V1/Holds.pm >@@ -26,6 +26,7 @@ use C4::Reserves; > use Koha::Items; > use Koha::Patrons; > use Koha::Holds; >+use Koha::Old::Holds; > use Koha::DateUtils qw( dt_from_string ); > > use List::MoreUtils qw( any ); >@@ -44,11 +45,18 @@ Method that handles listing Koha::Hold objects > sub list { > my $c = shift->openapi->valid_input or return; > >+ my $old = $c->param('old'); >+ $c->req->params->remove('old'); >+ > return try { >- my $holds = $c->objects->search( Koha::Holds->new ); >+ my $holds_set = >+ $old >+ ? Koha::Old::Holds->new >+ : Koha::Holds->new; >+ >+ my $holds = $c->objects->search($holds_set); > return $c->render( status => 200, openapi => $holds ); >- } >- catch { >+ } catch { > $c->unhandled_exception($_); > }; > } >diff --git a/Koha/REST/V1/Patrons/Holds.pm b/Koha/REST/V1/Patrons/Holds.pm >index 88eea15b53..d3aa97ca91 100644 >--- a/Koha/REST/V1/Patrons/Holds.pm >+++ b/Koha/REST/V1/Patrons/Holds.pm >@@ -49,16 +49,18 @@ sub list { > ); > } > >- return try { >+ my $old = $c->param('old'); >+ $c->req->params->remove('old'); > >- my $holds = $c->objects->search( $patron->holds ); >+ return try { >+ my $holds_set = >+ $old >+ ? $patron->old_holds >+ : $patron->holds; > >- return $c->render( >- status => 200, >- openapi => $holds >- ); >- } >- catch { >+ my $holds = $c->objects->search( $holds_set ); >+ return $c->render( status => 200, openapi => $holds ); >+ } catch { > $c->unhandled_exception($_); > }; > } >diff --git a/api/v1/swagger/definitions/hold.yaml b/api/v1/swagger/definitions/hold.yaml >index f97c3e216b..ae2c055a30 100644 >--- a/api/v1/swagger/definitions/hold.yaml >+++ b/api/v1/swagger/definitions/hold.yaml >@@ -108,5 +108,20 @@ properties: > - boolean > - "null" > description: Cancellation requests count for the hold (x-koha-embed) >+ biblio: >+ type: >+ - object >+ - "null" >+ description: Bibliographic record >+ item: >+ type: >+ - object >+ - "null" >+ description: The item >+ pickup_library: >+ type: >+ - object >+ - "null" >+ description: Pickup library > > additionalProperties: false >diff --git a/api/v1/swagger/paths/holds.yaml b/api/v1/swagger/paths/holds.yaml >index 7a1055e391..6458ff72ef 100644 >--- a/api/v1/swagger/paths/holds.yaml >+++ b/api/v1/swagger/paths/holds.yaml >@@ -88,6 +88,11 @@ > - $ref: "../swagger.yaml#/parameters/q_param" > - $ref: "../swagger.yaml#/parameters/q_body" > - $ref: "../swagger.yaml#/parameters/request_id_header" >+ - name: old >+ in: query >+ description: By default, current holds are returned, when this is true then >+ old holds are returned as result. >+ type: boolean > - name: x-koha-embed > in: header > required: false >@@ -97,6 +102,8 @@ > type: string > enum: > - cancellation_requested >+ - biblio >+ - pickup_library > collectionFormat: csv > produces: > - application/json >diff --git a/api/v1/swagger/paths/patrons_holds.yaml b/api/v1/swagger/paths/patrons_holds.yaml >index 7204be98cf..c8a4750b91 100644 >--- a/api/v1/swagger/paths/patrons_holds.yaml >+++ b/api/v1/swagger/paths/patrons_holds.yaml >@@ -15,6 +15,11 @@ > - $ref: "../swagger.yaml#/parameters/q_param" > - $ref: "../swagger.yaml#/parameters/q_body" > - $ref: "../swagger.yaml#/parameters/request_id_header" >+ - name: old >+ in: query >+ description: By default, current holds are returned, when this is true then >+ old holds are returned as result. >+ type: boolean > - name: x-koha-embed > in: header > required: false >@@ -24,6 +29,10 @@ > type: string > enum: > - cancellation_requested >+ - biblio >+ - item >+ - pickup_library >+ - pickup_library.branchname > collectionFormat: csv > produces: > - application/json >diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t >index aa7f677f64..99dae133a2 100755 >--- a/t/db_dependent/Koha/Hold.t >+++ b/t/db_dependent/Koha/Hold.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 13; >+use Test::More tests => 14; > > use Test::Exception; > use Test::MockModule; >@@ -87,6 +87,23 @@ subtest 'biblio() tests' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'pickup_library/branch tests' => sub { >+ >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ my $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ } >+ ); >+ >+ is( ref( $hold->pickup_library ), 'Koha::Library', '->pickup_library should return a Koha::Library object' ); >+ >+ $schema->storage->txn_rollback; >+}; >+ > subtest 'fill() tests' => sub { > > plan tests => 14; >diff --git a/t/db_dependent/Koha/Old/Hold.t b/t/db_dependent/Koha/Old/Hold.t >index d62a3136fc..3b52b5ee27 100755 >--- a/t/db_dependent/Koha/Old/Hold.t >+++ b/t/db_dependent/Koha/Old/Hold.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 2; >+use Test::More tests => 3; > use Test::Exception; > > use Koha::Database; >@@ -118,3 +118,28 @@ subtest 'biblio() tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'item/pickup_library() tests' => sub { >+ >+ plan tests => 4; >+ >+ $schema->storage->txn_begin; >+ >+ my $old_hold = $builder->build_object( >+ { >+ class => 'Koha::Old::Holds', >+ } >+ ); >+ is( ref( $old_hold->item ), 'Koha::Item', '->item should return a Koha::Item object' ); >+ is( ref( $old_hold->pickup_library ), 'Koha::Library', '->pickup_library should return a Koha::Library object' ); >+ >+ $old_hold->item->delete; >+ $old_hold->pickup_library->delete; >+ >+ $old_hold = $old_hold->get_from_storage; # Be on the safe side >+ >+ is( $old_hold->item, undef, 'Item has been deleted, ->item should return undef' ); >+ is( $old_hold->pickup_library, undef, 'Library has been deleted, ->pickup_library should return undef' ); >+ >+ $schema->storage->txn_rollback; >+}; >diff --git a/t/db_dependent/api/v1/patrons_holds.t b/t/db_dependent/api/v1/patrons_holds.t >index 98cffc375b..ffe6f56f0d 100755 >--- a/t/db_dependent/api/v1/patrons_holds.t >+++ b/t/db_dependent/api/v1/patrons_holds.t >@@ -35,7 +35,7 @@ t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); > > subtest 'list() tests' => sub { > >- plan tests => 9; >+ plan tests => 18; > > $schema->storage->txn_begin; > >@@ -51,12 +51,33 @@ subtest 'list() tests' => sub { > ->status_is( 200, 'SWAGGER3.2.2' ) > ->json_is( [] ); > >- my $hold_1 = $builder->build_object({ class => 'Koha::Holds', value => { borrowernumber => $patron->id } }); >- my $hold_2 = $builder->build_object({ class => 'Koha::Holds', value => { borrowernumber => $patron->id } }); >+ my $hold_1 = $builder->build_object( { class => 'Koha::Holds', value => { borrowernumber => $patron->id } } ); >+ my $hold_2 = $builder->build_object( { class => 'Koha::Holds', value => { borrowernumber => $patron->id } } ); >+ my $hold_3 = $builder->build_object( { class => 'Koha::Holds', value => { borrowernumber => $patron->id } } ); > >- $t->get_ok("//$userid:$password@/api/v1/patrons/" . $patron->id . '/holds?_order_by=+me.hold_id') >- ->status_is( 200, 'SWAGGER3.2.2' ) >- ->json_is( '' => [ $hold_1->to_api, $hold_2->to_api ], 'Holds retrieved' ); >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->id . '/holds?_order_by=+me.hold_id' ) >+ ->status_is( 200, 'SWAGGER3.2.2' ) >+ ->json_is( '' => [ $hold_1->to_api, $hold_2->to_api, $hold_3->to_api ], 'Holds retrieved' ); >+ >+ $hold_1->fill; >+ $hold_3->fill; >+ >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->id . '/holds?_order_by=+me.hold_id' ) >+ ->status_is( 200, 'SWAGGER3.2.2' )->json_is( '' => [ $hold_2->to_api ], 'Only current holds retrieved' ); >+ >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->id . '/holds?old=1&_order_by=+me.hold_id' ) >+ ->status_is( 200, 'SWAGGER3.2.2' ) >+ ->json_is( '' => [ $hold_1->to_api, $hold_3->to_api ], 'Only old holds retrieved' ); >+ >+ my $old_hold_1 = Koha::Old::Holds->find( $hold_1->id ); >+ $old_hold_1->item->delete; >+ $old_hold_1->pickup_library->delete; >+ >+ $t->get_ok( "//$userid:$password@/api/v1/patrons/" . $patron->id . '/holds?old=1&_order_by=+me.hold_id' ) >+ ->status_is( 200, 'SWAGGER3.2.2' )->json_is( >+ '' => [ $old_hold_1->get_from_storage->to_api, $hold_3->to_api ], >+ 'Old holds even after item and library removed' >+ ); > > my $non_existent_patron = $builder->build_object({ class => 'Koha::Patrons' }); > my $non_existent_patron_id = $non_existent_patron->id; >-- >2.37.1 (Apple Git-137.1)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35353
:
159055
|
159067
|
163828
|
163829
|
163833
|
163834
|
164298
|
164299