From 0a52881c150911971955ba92f581a4ec7920f3ce 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 | 48 ++++++++++++------- Koha/Hold.pm | 28 +++++++++++ Koha/HoldGroup.pm | 63 +++++++++++++++++++++++++ Koha/HoldGroups.pm | 60 ++++++++++++++++++++++++ Koha/Holds.pm | 26 +++++++++++ t/db_dependent/Koha/Holds.t | 72 ++++++++++++++++++++++++++++- t/db_dependent/Reserves/HoldGroup.t | 53 +++++++++++++++++++++ 7 files changed, 333 insertions(+), 17 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 d5e01f283d..6a5b688776 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -193,6 +193,7 @@ sub AddReserve { my $found = $params->{found}; my $itemtype = $params->{itemtype}; my $non_priority = $params->{non_priority}; + my $hold_group_id = $params->{hold_group_id}; $resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' }) or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' }); @@ -255,6 +256,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'; @@ -520,18 +522,18 @@ sub CanItemBeReserved { biblionumber => $item->biblionumber, }; $search_params->{found} = undef if $params->{ignore_found_holds}; - my $holds = Koha::Holds->search($search_params); - return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record } if $holds->count() >= $holds_per_record; + my $holds_count = Koha::Holds->count_holds($search_params); + return { status => "tooManyHoldsForThisRecord", limit => $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 { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds->count() >= $holds_per_day; + return { status => 'tooManyReservesToday', limit => $holds_per_day } if $today_holds_count >= $holds_per_day; } # we check if it's ok or not @@ -1235,6 +1237,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') @@ -1247,6 +1252,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; @@ -1258,18 +1272,20 @@ sub ModReserveAffect { CartToShelf( $itemnumber ); } - my $std = $dbh->prepare(q{ - DELETE q, t - FROM tmp_holdsqueue q - INNER JOIN hold_fill_targets t - ON q.borrowernumber = t.borrowernumber - AND q.biblionumber = t.biblionumber - AND q.itemnumber = t.itemnumber - AND q.item_level_request = t.item_level_request - AND q.holdingbranch = t.source_branchcode - WHERE t.reserve_id = ? - }); - $std->execute($hold->reserve_id); + foreach my $id ( @reserve_ids ) { + my $std = $dbh->prepare(q{ + DELETE q, t + FROM tmp_holdsqueue q + INNER JOIN hold_fill_targets t + ON q.borrowernumber = t.borrowernumber + AND q.biblionumber = t.biblionumber + AND q.itemnumber = t.itemnumber + AND q.item_level_request = t.item_level_request + AND q.holdingbranch = t.source_branchcode + WHERE t.reserve_id = ? + }); + $std->execute($id); + } logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold ) if C4::Context->preference('HoldsLog'); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 63e30970ca..80866e02c4 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -41,6 +41,7 @@ use Koha::Plugins; use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; use Koha::Exceptions; +use Koha::HoldGroups; use Koha::Exceptions::Hold; use base qw(Koha::Object); @@ -796,6 +797,14 @@ sub fill { C4::Log::logaction( 'HOLDS', 'FILL', $self->id, $self ) 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 ] @@ -861,6 +870,25 @@ sub _set_default_expirationdate { dt_from_string( $self->reservedate )->add( $timeunit => $period ) ); } +=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 4ff90e679e..4450c21fc4 100644 --- a/Koha/Holds.pm +++ b/Koha/Holds.pm @@ -178,6 +178,32 @@ sub filter_out_has_cancellation_requests { { join => 'cancellation_requests' } ); } +=head3 count + + $holds->count( $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 aec261dd1f..4706ba22f6 100755 --- a/t/db_dependent/Koha/Holds.t +++ b/t/db_dependent/Koha/Holds.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 8; +use Test::More tests => 9; use Test::Warn; use C4::Circulation qw( AddIssue ); @@ -641,6 +641,76 @@ subtest 'set_waiting+patron_expiration_date' => 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; diff --git a/t/db_dependent/Reserves/HoldGroup.t b/t/db_dependent/Reserves/HoldGroup.t new file mode 100755 index 0000000000..4bc4687c79 --- /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.30.2