From 39aa2d391d442a24ca6e86e8458432aeacdacade Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 1 Mar 2021 15:25:52 +1300 Subject: [PATCH] Bug 15516: Relevant controller changes and tests Signed-off-by: Michal Denar Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Michal Denar --- C4/Reserves.pm | 32 ++++++++--- Koha/Hold.pm | 28 +++++++++ Koha/HoldGroup.pm | 63 ++++++++++++++++++++ Koha/HoldGroups.pm | 60 +++++++++++++++++++ Koha/Holds.pm | 26 +++++++++ t/db_dependent/Koha/Holds.t | 89 +++++++++++++++++++++++++++++ t/db_dependent/Reserves/HoldGroup.t | 53 +++++++++++++++++ 7 files changed, 343 insertions(+), 8 deletions(-) create mode 100644 Koha/HoldGroup.pm create mode 100644 Koha/HoldGroups.pm create mode 100755 t/db_dependent/Reserves/HoldGroup.t diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 1ffddd5b9f..a1740017d1 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -194,6 +194,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; @@ -256,6 +257,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'; @@ -569,21 +571,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 @@ -1239,6 +1241,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') @@ -1253,6 +1258,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; @@ -1266,8 +1280,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 @@ -1277,9 +1292,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'); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 59b498a7d2..998e5beda2 100644 --- a/Koha/Hold.pm +++ b/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'); @@ -993,6 +1002,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; diff --git a/Koha/HoldGroup.pm b/Koha/HoldGroup.pm new file mode 100644 index 0000000000..08675c48eb --- /dev/null +++ b/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 . + +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 + +=cut + +1; diff --git a/Koha/HoldGroups.pm b/Koha/HoldGroups.pm new file mode 100644 index 0000000000..ad96b60704 --- /dev/null +++ b/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 . + +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 + +=cut + +1; diff --git a/Koha/Holds.pm b/Koha/Holds.pm index 8336be0cd4..dcdd08584d 100644 --- a/Koha/Holds.pm +++ b/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 diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t index a27dd02ed8..30e84f3dbb 100755 --- a/t/db_dependent/Koha/Holds.t +++ b/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 { diff --git a/t/db_dependent/Reserves/HoldGroup.t b/t/db_dependent/Reserves/HoldGroup.t new file mode 100755 index 0000000000..5f92edaa44 --- /dev/null +++ b/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 . + +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; +}; -- 2.39.5