From b09d93b4e9bf2a3f44080c25d34431f4ca2ed8bf Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 16 Mar 2022 17:22:28 -0300 Subject: [PATCH] Bug 29346: Hold actions triggers This patch makes several holds related actions schedule the background job for real-time update of the holds queue. This actions are: - place (C4::Reserves::AddReserve) - fill (Koha::Hold->fill) - cancel (Koha::Hold->cancel) - suspend (Koha::Hold->suspend) - resume (Koha::Hold->resume) The cancel() action is added a *skip_holds_queue* parameter to skip triggering the background job entirely. It targets cases like C4::Biblio::DelBiblio in which all biblio holds are cancelled in a loop. In that case, we just want to cancel them and let a single backgroung job take care of the holds queue, once the biblio is deleted. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Hold.t \ t/db_dependent/Reserves.t => SUCCESS: Tests pass! Triggers are triggered 3. Sign off :-D Sponsored-by: Montgomery County Public Libraries Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi Signed-off-by: Nick Clemens Signed-off-by: Fridolin Somers --- C4/Reserves.pm | 7 +++ Koha/Hold.pm | 29 +++++++++- t/db_dependent/Koha/Hold.t | 105 +++++++++++++++++++++++++++++++++++-- t/db_dependent/Reserves.t | 33 +++++++++++- 4 files changed, 169 insertions(+), 5 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index d6121a7616..caf210b1d0 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -33,6 +33,7 @@ use C4::Log qw( logaction ); use C4::Members::Messaging; use C4::Members; use Koha::Account::Lines; +use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; use Koha::Biblios; use Koha::Calendar; use Koha::CirculationRules; @@ -314,6 +315,12 @@ sub AddReserve { } ); + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { + biblio_ids => [ $biblionumber ] + } + ); + return $reserve_id; } diff --git a/Koha/Hold.pm b/Koha/Hold.pm index d4fb0e4c52..97c6b4c37a 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -36,6 +36,8 @@ use Koha::Old::Holds; use Koha::Calendar; use Koha::Plugins; +use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; + use Koha::Exceptions::Hold; use base qw(Koha::Object); @@ -124,6 +126,12 @@ sub suspend_hold { logaction( 'HOLDS', 'SUSPEND', $self->reserve_id, $self ) if C4::Context->preference('HoldsLog'); + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { + biblio_ids => [ $self->biblionumber ] + } + ); + return $self; } @@ -152,6 +160,12 @@ sub resume { logaction( 'HOLDS', 'RESUME', $self->reserve_id, $self ) if C4::Context->preference('HoldsLog'); + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { + biblio_ids => [ $self->biblionumber ] + } + ); + return $self; } @@ -516,8 +530,9 @@ sub is_suspended { my $cancel_hold = $hold->cancel( { - [ charge_cancel_fee => 1||0, ] + [ charge_cancel_fee => 1||0, ] [ cancellation_reason => $cancellation_reason, ] + [ skip_holds_queue => 1||0 ] } ); @@ -607,6 +622,12 @@ sub cancel { C4::Log::logaction( 'HOLDS', 'CANCEL', $self->reserve_id, $self ) if C4::Context->preference('HoldsLog'); + + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { + biblio_ids => [ $old_me->biblionumber ] + } + ) unless $params->{skip_holds_queue}; } ); return $self; @@ -671,6 +692,12 @@ sub fill { C4::Log::logaction( 'HOLDS', 'FILL', $self->id, $self ) if C4::Context->preference('HoldsLog'); + + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { + biblio_ids => [ $old_me->biblionumber ] + } + ); } ); return $self; diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t index 0c1776daa9..9547e353a2 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 => 5; +use Test::More tests => 6; use Test::Exception; use Test::MockModule; @@ -38,7 +38,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'fill() tests' => sub { - plan tests => 12; + plan tests => 13; $schema->storage->txn_begin; @@ -233,6 +233,32 @@ subtest 'fill() tests' => sub { ); }; + subtest 'holds_queue update tests' => sub { + + plan tests => 1; + + my $biblio = $builder->build_sample_biblio; + + my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); + $mock->mock( 'enqueue', sub { + my ( $self, $args ) = @_; + is_deeply( + $args->{biblio_ids}, + [ $biblio->id ], + '->fill triggers a holds queue update for the related biblio' + ); + } ); + + $builder->build_object( + { + class => 'Koha::Holds', + value => { + biblionumber => $biblio->id, + } + } + )->fill; + }; + $schema->storage->txn_rollback; }; @@ -456,7 +482,7 @@ subtest 'is_pickup_location_valid() tests' => sub { subtest 'cancel() tests' => sub { - plan tests => 5; + plan tests => 6; $schema->storage->txn_begin; @@ -526,5 +552,78 @@ subtest 'cancel() tests' => sub { 'Patron link is set to the configured anonymous patron immediately' ); + subtest 'holds_queue update tests' => sub { + + plan tests => 1; + + my $biblio = $builder->build_sample_biblio; + + my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); + $mock->mock( 'enqueue', sub { + my ( $self, $args ) = @_; + is_deeply( + $args->{biblio_ids}, + [ $biblio->id ], + '->cancel triggers a holds queue update for the related biblio' + ); + } ); + + $builder->build_object( + { + class => 'Koha::Holds', + value => { + biblionumber => $biblio->id, + } + } + )->cancel; + + # If the skip_holds_queue param is not honoured, then test count will fail. + $builder->build_object( + { + class => 'Koha::Holds', + value => { + biblionumber => $biblio->id, + } + } + )->cancel({ skip_holds_queue => 1 }); + }; + + $schema->storage->txn_rollback; +}; + +subtest 'suspend_hold() and resume() tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + my $biblio = $builder->build_sample_biblio; + my $action; + + my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); + $mock->mock( 'enqueue', sub { + my ( $self, $args ) = @_; + is_deeply( + $args->{biblio_ids}, + [ $biblio->id ], + "->$action triggers a holds queue update for the related biblio" + ); + } ); + + my $hold = $builder->build_object( + { + class => 'Koha::Holds', + value => { + biblionumber => $biblio->id, + } + } + ); + + $action = 'suspend_hold'; + $hold->suspend_hold; + + $action = 'resume'; + $hold->resume; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 9616d8f160..c93987e17a 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 69; +use Test::More tests => 70; use Test::MockModule; use Test::Warn; @@ -1397,3 +1397,34 @@ subtest 'IsAvailableForItemLevelRequest() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'AddReserve() tests' => sub { + + plan tests => 1; + + $schema->storage->txn_begin; + + my $library = $builder->build_object({ class => 'Koha::Libraries' }); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $biblio = $builder->build_sample_biblio; + + my $mock = Test::MockModule->new('Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue'); + $mock->mock( 'enqueue', sub { + my ( $self, $args ) = @_; + is_deeply( + $args->{biblio_ids}, + [ $biblio->id ], + "AddReserve triggers a holds queue update for the related biblio" + ); + } ); + + AddReserve( + { + branchcode => $library->branchcode, + borrowernumber => $patron->id, + biblionumber => $biblio->id, + } + ); + + $schema->storage->txn_rollback; +}; -- 2.39.5