Bug 15516: Relevant controller changes and tests

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Reserves.pm

@@ -192,6 +192,7 @@ sub AddReserve {
     my $itemtype               = $params->{itemtype};
     my $non_priority           = $params->{non_priority};
     my $item_group_id          = $params->{item_group_id};
+    my $hold_group_id          = $params->{hold_group_id};
 
     $resdate ||= dt_from_string;
 
@@ -254,6 +255,7 @@ sub AddReserve {
             itemtype               => $itemtype,
             item_level_hold        => $checkitem    ? 1 : 0,
             non_priority           => $non_priority ? 1 : 0,
+            hold_group_id          => $hold_group_id,
         }
     )->store();
     $hold->set_waiting() if $found && $found eq 'W';
@@ -567,21 +569,21 @@ sub CanItemBeReserved {
                 borrowernumber => $patron->borrowernumber,
                 biblionumber   => $item->biblionumber,
             };
-            my $holds = Koha::Holds->search($search_params);
+            my $holds_count = Koha::Holds->count_holds($search_params);
             return _cache { status => "tooManyHoldsForThisRecord", limit => $holds_per_record }
-                if $holds->count() >= $holds_per_record;
+                if $holds_count >= $holds_per_record;
         }
     }
 
     if ( !$params->{ignore_hold_counts} && defined $holds_per_day && $holds_per_day ne '' ) {
-        my $today_holds = Koha::Holds->search(
+        my $today_holds_count = Koha::Holds->count_holds(
             {
                 borrowernumber => $patron->borrowernumber,
                 reservedate    => dt_from_string->date
             }
         );
         return _cache { status => 'tooManyReservesToday', limit => $holds_per_day }
-            if $today_holds->count() >= $holds_per_day;
+            if $today_holds_count >= $holds_per_day;
     }
 
     # we check if it's ok or not
@@ -1237,6 +1239,9 @@ sub ModReserveAffect {
 
     $hold->itemnumber($itemnumber);
 
+    my @reserve_ids;
+    push @reserve_ids, $hold->reserve_id;
+
     if ($transferToDo) {
         $hold->set_transfer();
     } elsif ( C4::Context->preference('HoldsNeedProcessingSIP')
@@ -1251,6 +1256,15 @@ sub ModReserveAffect {
         # Complete transfer if one exists
         my $transfer = $hold->item->get_transfer;
         $transfer->receive if $transfer;
+
+        # if this hold was part of a group, cancel other holds in the group
+        if ( $hold->hold_group_id ) {
+            my @holds = $hold->hold_group->holds->as_list;
+            foreach my $h (@holds) {
+                push @reserve_ids, $h->reserve_id;
+                $h->cancel unless $h->reserve_id == $hold->reserve_id;
+            }
+        }
     }
 
     _koha_notify_hold_changed($hold) if $notify_library;
@@ -1264,8 +1278,9 @@ sub ModReserveAffect {
         CartToShelf($itemnumber);
     }
 
-    my $std = $dbh->prepare(
-        q{
+    foreach my $id (@reserve_ids) {
+        my $std = $dbh->prepare(
+            q{
         DELETE  q, t
         FROM    tmp_holdsqueue q
         INNER JOIN hold_fill_targets t
@@ -1275,9 +1290,10 @@ sub ModReserveAffect {
             AND q.item_level_request = t.item_level_request
             AND q.holdingbranch = t.source_branchcode
         WHERE t.reserve_id = ?
+            }
+        );
+        $std->execute($id);
     }
-    );
-    $std->execute( $hold->reserve_id );
 
     logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $original )
         if C4::Context->preference('HoldsLog');

Koha/Hold.pm

@@ -40,6 +40,7 @@ use Koha::Plugins;
 use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue;
 
 use Koha::Exceptions;
+use Koha::HoldGroups;
 use Koha::Exceptions::Hold;
 
 use base qw(Koha::Object);
@@ -881,6 +882,14 @@ sub fill {
             C4::Log::logaction( 'HOLDS', 'FILL', $self->id, $self, undef, $original )
                 if C4::Context->preference('HoldsLog');
 
+            # if this hold was part of a group, cancel other holds in the group
+            if ( $self->hold_group_id ) {
+                my @holds = $self->hold_group->holds->as_list;
+                foreach my $h (@holds) {
+                    $h->cancel unless $h->reserve_id == $self->id;
+                }
+            }
+
             Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue(
                 { biblio_ids => [ $old_me->biblionumber ] } )
                 if C4::Context->preference('RealTimeHoldsQueue');
@@ -1052,6 +1061,25 @@ sub _set_default_expirationdate {
     }
 }
 
+=head3 hold_group
+
+    my $hold_group = $hold->hold_group;
+    my $hold_group_id = $hold_group->id if $hold_group;
+
+Return the Koha::HoldGroup object of a hold if part of a hold group
+
+=cut
+
+sub hold_group {
+    my ($self) = @_;
+
+    if ( $self->hold_group_id ) {
+        return Koha::HoldGroups->find( $self->hold_group_id );
+    }
+
+    return;
+}
+
 =head3 _move_to_old
 
 my $is_moved = $hold->_move_to_old;

Koha/HoldGroup.pm

@@ -0,0 +1,63 @@
+package Koha::HoldGroup;
+
+# Copyright 2020 Koha Development team
+#
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+
+use base qw(Koha::Object);
+
+=head1 NAME
+
+Koha::HoldGroup - Koha Hold Group object class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 holds
+
+    $holds = $hold_group->holds
+
+Return all holds associated with this group
+
+=cut
+
+sub holds {
+    my ($self) = @_;
+
+    my $holds_rs = $self->_result->reserves->search;
+    return Koha::Holds->_new_from_dbic($holds_rs);
+}
+
+=head3 _type
+
+=cut
+
+sub _type {
+    return 'HoldGroup';
+}
+
+=head1 AUTHORS
+
+Josef Moravec <josef.moravec@gmail.com>
+
+=cut
+
+1;

Koha/HoldGroups.pm

@@ -0,0 +1,60 @@
+package Koha::HoldGroups;
+
+# Copyright Koha Development team 2020
+#
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+
+use Koha::Database;
+
+use Koha::HoldGroup;
+
+use base qw(Koha::Objects);
+
+=head1 NAME
+
+Koha::HoldGroups - Koha Hold Group object set class
+
+=head1 API
+
+=head2 Class Methods
+
+=cut
+
+=head3 _type
+
+=cut
+
+sub _type {
+    return 'HoldGroup';
+}
+
+=head3 object_class
+
+=cut
+
+sub object_class {
+    return 'Koha::HoldGroup';
+}
+
+=head1 AUTHOR
+
+Josef Moravec <josef.moravec@gmail.com>
+
+=cut
+
+1;

Koha/Holds.pm

@@ -206,6 +206,32 @@ sub filter_out_has_cancellation_requests {
     );
 }
 
+=head3 count_holds
+
+    $holds->count_holds( $search_params );
+
+This overwrites the default count().
+
+Return the number of holds, where a hold group is counted as one hold.
+
+=cut
+
+sub count_holds {
+    my ( $self, $search_params ) = @_;
+
+    $search_params = {
+        hold_group_id => undef,
+    };
+    my $holds_without_group_count = $self->search($search_params)->count();
+
+    $search_params = {
+        hold_group_id => { '!=', undef },
+    };
+    my $hold_groups_count = $self->search( $search_params, { group_by => 'me.hold_group_id' } )->count();
+
+    return $holds_without_group_count + $hold_groups_count;
+}
+
 =head2 Internal methods
 
 =head3 _type

t/db_dependent/Koha/Holds.t

@@ -1068,6 +1068,95 @@ subtest 'Test Koha::Hold::item_group' => sub {
     );
 };
 
+subtest 'count_holds' => sub {
+    plan tests => 3;
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+        }
+    );
+    my $patron_id = $patron->borrowernumber;
+
+    my $hold1 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => undef,
+            },
+        }
+    );
+
+    is( $patron->holds->count_holds, 1, 'Test patron has 1 hold.' );
+
+    my $hold_group = $builder->build_object(
+        {
+            class => 'Koha::HoldGroups',
+        }
+    );
+
+    my $hold2 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => $hold_group->hold_group_id,
+            }
+        }
+    );
+    my $hold3 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => $hold_group->hold_group_id,
+            }
+        }
+    );
+
+    is( $patron->holds->count_holds, 2, 'Test patron has 2 holds.' );
+
+    my $hold_group2 = $builder->build_object(
+        {
+            class => 'Koha::HoldGroups',
+        }
+    );
+
+    my $hold4 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => $hold_group2->hold_group_id,
+            }
+        }
+    );
+    my $hold5 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => $hold_group2->hold_group_id,
+            }
+        }
+    );
+    my $hold6 = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron_id,
+                hold_group_id  => $hold_group2->hold_group_id,
+            }
+        }
+    );
+
+    is( $patron->holds->count_holds, 3, 'Test patron has 3 holds.' );
+
+    $schema->storage->txn_rollback;
+};
+
 $schema->storage->txn_rollback;
 
 subtest 'filter_by_found() tests' => sub {

t/db_dependent/Reserves/HoldGroup.t

@@ -0,0 +1,53 @@
+#!/usr/bin/perl
+
+# Copyright 2020 Koha Development team
+#
+# This file is part of Koha
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+
+use Test::More tests => 1;
+
+use t::lib::TestBuilder;
+
+my $schema  = Koha::Database->new->schema;
+my $builder = t::lib::TestBuilder->new;
+
+subtest 'holds tests' => sub {
+
+    plan tests => 2;
+
+    $schema->storage->txn_begin;
+
+    my $patron     = $builder->build_object( { class => 'Koha::Patrons' } );
+    my $hold_group = $builder->build_object( { class => 'Koha::HoldGroups' } );
+    my $hold       = $builder->build_object(
+        {
+            class => 'Koha::Holds',
+            value => {
+                borrowernumber => $patron->borrowernumber,
+                hold_group_id  => $hold_group->hold_group_id,
+            }
+        }
+    );
+
+    my $holds = $hold_group->holds;
+    is( ref($holds), 'Koha::Holds', 'Right type' );
+    my $hold_from_group = $holds->next;
+    is( $hold_from_group->id, $hold->id, 'Right object' );
+
+    $schema->storage->txn_rollback;
+};