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 <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

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;
 }
 

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;

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;
 };

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;
+};