From c5cd53e4187d27b710f26b2503c8c38290adbc56 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 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 method above), but for which circulation rules allow +requesting cancellation. + +Throws a B exception with the following I +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 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 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 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