Bugzilla – Attachment 135323 Details for
Bug 22456
Allow patrons to cancel their waiting holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22456: Add cancellation request methods to Koha::Hold
Bug-22456-Add-cancellation-request-methods-to-Koha.patch (text/plain), 12.98 KB, created by
Tomás Cohen Arazi (tcohen)
on 2022-05-24 17:18:01 UTC
(
hide
)
Description:
Bug 22456: Add cancellation request methods to Koha::Hold
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2022-05-24 17:18:01 UTC
Size:
12.98 KB
patch
obsolete
>From c5cd53e4187d27b710f26b2503c8c38290adbc56 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Wed, 18 May 2022 14:05:04 -0300 >Subject: [PATCH] Bug 22456: Add cancellation request methods to Koha::Hold > >This patch adds the following helper methods for dealing with hold >cancellation requests to the Koha::Hold class: > > * cancellation_requests > * add_cancellation_request > * cancellation_requestable_from_opac > * cancellation_requested_by_owner > >To test: >1. Apply this patch >2. Run: > $ kshell > k$ prove t/db_dependent/Koha/Hold.t >=> SUCCESS: Tests pass! >3. Sign off :-D >--- > Koha/Exceptions.pm | 5 + > Koha/Hold.pm | 116 ++++++++++++++++++ > t/db_dependent/Koha/Hold.t | 237 ++++++++++++++++++++++++++++++++++++- > 3 files changed, 356 insertions(+), 2 deletions(-) > >diff --git a/Koha/Exceptions.pm b/Koha/Exceptions.pm >index 5cb8d5c38b..e3c8e2d223 100644 >--- a/Koha/Exceptions.pm >+++ b/Koha/Exceptions.pm >@@ -15,6 +15,11 @@ use Exception::Class ( > isa => 'Koha::Exception', > description => 'Same object already exists', > }, >+ 'Koha::Exceptions::InvalidStatus' => { >+ isa => 'Koha::Exception', >+ description => 'The current status is not valid in context', >+ fields => ['invalid_status'], >+ }, > 'Koha::Exceptions::ObjectNotFound' => { > isa => 'Koha::Exception', > description => 'The required object doesn\'t exist', >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index be5970cbd4..9e953837f7 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -30,6 +30,7 @@ use Koha::AuthorisedValues; > use Koha::DateUtils qw( dt_from_string ); > use Koha::Patrons; > use Koha::Biblios; >+use Koha::Hold::CancellationRequests; > use Koha::Items; > use Koha::Libraries; > use Koha::Old::Holds; >@@ -38,6 +39,7 @@ use Koha::Plugins; > > use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; > >+use Koha::Exceptions; > use Koha::Exceptions::Hold; > > use base qw(Koha::Object); >@@ -416,6 +418,74 @@ sub is_cancelable_from_opac { > return 0; # if ->is_in_transit or if ->is_waiting or ->is_in_processing > } > >+=head3 cancellation_requestable_from_opac >+ >+ if ( $hold->cancellation_requestable_from_opac ) { ... } >+ >+Returns a I<boolean> representing if a cancellation request can be placed on the hold >+from the OPAC. It targets holds that cannot be cancelled from the OPAC (see the >+B<is_cancelable_from_opac> method above), but for which circulation rules allow >+requesting cancellation. >+ >+Throws a B<Koha::Exceptions::InvalidStatus> exception with the following I<invalid_status> >+values: >+ >+=over 4 >+ >+=item B<'hold_not_waiting'>: the hold is expected to be waiting and it is not. >+ >+=item B<'no_item_linked'>: the waiting hold doesn't have an item properly linked. >+ >+=back >+ >+=cut >+ >+sub cancellation_requestable_from_opac { >+ my ( $self ) = @_; >+ >+ Koha::Exceptions::InvalidStatus->throw( invalid_status => 'hold_not_waiting' ) >+ unless $self->is_waiting; >+ >+ my $item = $self->item; >+ >+ Koha::Exceptions::InvalidStatus->throw( invalid_status => 'no_item_linked' ) >+ unless $item; >+ >+ my $patron = $self->patron; >+ >+ my $controlbranch = $patron->branchcode; >+ >+ if ( C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary' ) { >+ $controlbranch = $item->homebranch; >+ } >+ >+ return Koha::CirculationRules->get_effective_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->itype, >+ branchcode => $controlbranch, >+ rule_name => 'waiting_hold_cancellation', >+ } >+ )->rule_value ? 1 : 0; >+} >+ >+=head3 owner_cancellation_request >+ >+ if ( $hold->owner_cancellation_request ) { ... } >+ >+Returns a I<Koha::Hold::CancellationRequest> if the owner has placed one. >+ >+=cut >+ >+sub owner_cancellation_request { >+ my ($self) = @_; >+ >+ my ($owner_request) = $self->cancellation_requests->search( >+ { requester_id => $self->borrowernumber } )->as_list; >+ >+ return $owner_request; >+} >+ > =head3 is_at_destination > > Returns true if hold is waiting >@@ -525,6 +595,50 @@ sub is_suspended { > return $self->suspend(); > } > >+=head3 add_cancellation_request >+ >+ my $cancellation_request = $hold->add_cancellation_request( >+ { >+ [ requester_id => $patron->id, >+ creation_date => $creation_date, >+ source => 'owner', ] >+ } >+ ); >+ >+Adds a cancellation request to the hold. Returns the generated >+I<Koha::Hold::CancellationRequest> object. >+ >+=cut >+ >+sub add_cancellation_request { >+ my ( $self, $params ) = @_; >+ >+ my $request = Koha::Hold::CancellationRequest->new( >+ { hold_id => $self->id, >+ requester_id => $params->{requester_id}, >+ ( $params->{creation_date} ? ( creation_date => $params->{creation_date} ) : () ), >+ source => $params->{source}, >+ } >+ )->store; >+ >+ $request->discard_changes; >+ >+ return $request; >+} >+ >+=head3 cancellation_requests >+ >+ my $cancellation_requests = $hold->cancellation_requests; >+ >+Returns related a I<Koha::Hold::CancellationRequests> resultset. >+ >+=cut >+ >+sub cancellation_requests { >+ my ($self) = @_; >+ >+ return Koha::Hold::CancellationRequests->search( { hold_id => $self->id } ); >+} > > =head3 cancel > >@@ -584,6 +698,8 @@ sub cancel { > } > } > >+ $self->cancellation_requests->filter_by_current->accept; >+ > my $old_me = $self->_move_to_old; > > Koha::Plugins->call( >diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t >index 8c32bda3f7..4b25602759 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 => 6; >+use Test::More tests => 8; > > use Test::Exception; > use Test::MockModule; >@@ -495,7 +495,7 @@ subtest 'is_pickup_location_valid() tests' => sub { > > subtest 'cancel() tests' => sub { > >- plan tests => 6; >+ plan tests => 7; > > $schema->storage->txn_begin; > >@@ -614,6 +614,41 @@ subtest 'cancel() tests' => sub { > )->cancel({ skip_holds_queue => 0 }); > }; > >+ subtest 'cancellation_requests handling' => sub { >+ >+ plan tests => 2; >+ >+ my $item = $builder->build_sample_item; >+ my $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ value => { >+ biblionumber => $item->biblionumber, >+ itemnumber => $item->itemnumber, >+ found => 'W', >+ } >+ } >+ ); >+ >+ my $request = $hold->add_cancellation_request( >+ { >+ requester_id => $hold->borrowernumber, >+ } >+ ); >+ >+ # limit noise >+ t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelayCharge', undef ); >+ t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 0 ); >+ >+ is( $request->status, 'requested' ); >+ >+ $hold->cancel; >+ >+ $request->discard_changes; >+ >+ is( $request->status, 'accepted', 'When holds are cancellaed, cancellation requests are implicitly accepted' ); >+ }; >+ > $schema->storage->txn_rollback; > }; > >@@ -656,3 +691,201 @@ subtest 'suspend_hold() and resume() tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'cancellation_requests() and add_cancellation_request() tests' => sub { >+ >+ plan tests => 8; >+ >+ $schema->storage->txn_begin; >+ >+ t::lib::Mocks::mock_preference( 'RealTimeHoldsQueue', 0 ); >+ >+ my $hold = $builder->build_object( { class => 'Koha::Holds', } ); >+ >+ is( $hold->cancellation_requests->count, 0 ); >+ >+ # Add two cancellation requests >+ my $request_1 = $hold->add_cancellation_request; >+ >+ is( $request_1->requester_id, undef ); >+ is( $request_1->status, 'requested', 'Default value set' ); >+ isnt( $request_1->creation_date, undef ); >+ >+ my $requester = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $creation_date = '2021-06-25 14:05:35'; >+ >+ my $request_2 = $hold->add_cancellation_request( >+ { >+ requester_id => $requester->id, >+ creation_date => $creation_date, >+ } >+ ); >+ >+ is( $request_2->requester_id, $requester->id, 'Passed requester_id set' ); >+ is( $request_2->creation_date, $creation_date, 'Passed creation_date set' ); >+ >+ is( $hold->cancellation_requests->count, 2 ); >+ >+ subtest 'owner_cancellation_request() tests' => sub { >+ >+ plan tests => 3; >+ >+ my $owner_request = $hold->owner_cancellation_request; >+ >+ ok( !$owner_request ); >+ >+ $hold->add_cancellation_request({ requester_id => $hold->borrowernumber }); >+ >+ $owner_request = $hold->owner_cancellation_request; >+ >+ is( ref($owner_request), 'Koha::Hold::CancellationRequest' ); >+ is( $owner_request->requester_id, $hold->borrowernumber ); >+ }; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'cancellation_requestable_from_opac() tests' => sub { >+ >+ plan tests => 5; >+ >+ $schema->storage->txn_begin; >+ >+ my $category = >+ $builder->build_object( { class => 'Koha::Patron::Categories' } ); >+ my $item_home_library = >+ $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron_home_library = >+ $builder->build_object( { class => 'Koha::Libraries' } ); >+ >+ my $item = >+ $builder->build_sample_item( { library => $item_home_library->id } ); >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { branchcode => $patron_home_library->id } >+ } >+ ); >+ >+ subtest 'Exception cases' => sub { >+ >+ plan tests => 4; >+ >+ my $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ value => { >+ itemnumber => undef, >+ found => undef, >+ borrowernumber => $patron->id >+ } >+ } >+ ); >+ >+ throws_ok { $hold->cancellation_requestable_from_opac; } >+ 'Koha::Exceptions::InvalidStatus', >+ 'Exception thrown because hold is not waiting'; >+ >+ is( $@->invalid_status, 'hold_not_waiting' ); >+ >+ $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ value => { >+ itemnumber => undef, >+ found => 'W', >+ borrowernumber => $patron->id >+ } >+ } >+ ); >+ >+ throws_ok { $hold->cancellation_requestable_from_opac; } >+ 'Koha::Exceptions::InvalidStatus', >+ 'Exception thrown because waiting hold has no item linked'; >+ >+ is( $@->invalid_status, 'no_item_linked' ); >+ }; >+ >+ # set default rule to enabled >+ Koha::CirculationRules->set_rule( >+ { >+ categorycode => '*', >+ itemtype => '*', >+ branchcode => '*', >+ rule_name => 'waiting_hold_cancellation', >+ rule_value => 1, >+ } >+ ); >+ >+ my $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ value => { >+ itemnumber => $item->id, >+ found => 'W', >+ borrowernumber => $patron->id >+ } >+ } >+ ); >+ >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', >+ 'ItemHomeLibrary' ); >+ >+ Koha::CirculationRules->set_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->itype, >+ branchcode => $item->homebranch, >+ rule_name => 'waiting_hold_cancellation', >+ rule_value => 0, >+ } >+ ); >+ >+ ok( !$hold->cancellation_requestable_from_opac ); >+ >+ Koha::CirculationRules->set_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->itype, >+ branchcode => $item->homebranch, >+ rule_name => 'waiting_hold_cancellation', >+ rule_value => 1, >+ } >+ ); >+ >+ ok( >+ $hold->cancellation_requestable_from_opac, >+ 'Make sure it is picking the right circulation rule' >+ ); >+ >+ t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); >+ >+ Koha::CirculationRules->set_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->itype, >+ branchcode => $patron->branchcode, >+ rule_name => 'waiting_hold_cancellation', >+ rule_value => 0, >+ } >+ ); >+ >+ ok( !$hold->cancellation_requestable_from_opac ); >+ >+ Koha::CirculationRules->set_rule( >+ { >+ categorycode => $patron->categorycode, >+ itemtype => $item->itype, >+ branchcode => $patron->branchcode, >+ rule_name => 'waiting_hold_cancellation', >+ rule_value => 1, >+ } >+ ); >+ >+ ok( >+ $hold->cancellation_requestable_from_opac, >+ 'Make sure it is picking the right circulation rule' >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >-- >2.34.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 22456
:
135165
|
135166
|
135167
|
135168
|
135169
|
135170
|
135171
|
135319
|
135320
|
135321
|
135322
|
135323
|
135324
|
135325
|
135326
|
135349
|
135350
|
135351
|
135352
|
135396
|
135397
|
135398
|
135399
|
135400
|
135401
|
135402
|
135403
|
135404
|
135405
|
135406
|
135407
|
135408
|
135409
|
135410
|
135411
|
135430
|
135830
|
135831
|
135832
|
135838
|
135839
|
135840
|
135841
|
135842
|
135843
|
135844
|
135845
|
135846
|
137580
|
137581
|
137582
|
137583
|
137584
|
137585
|
137586
|
137587
|
137588
|
137589
|
137591
|
138209
|
138210
|
138211
|
138212
|
138213
|
138214
|
138215
|
138216
|
138217
|
138218