From d1e13102523f6af161a3a5b2944d1aa60746ee4d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 28 Apr 2022 09:28:50 +0100 Subject: [PATCH] Bug 30275: (follow-up) Drop renewer_id constraint This patch fixes some unit tests by ensureing we set a valid userid for mock userenv setting so that the foreign key constraint doesn't fail and it also removes the exception class and check for renewer_id from the store method as, for example with autorenewals, the renewal may not have been triggered by a actual user. Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 2 +- Koha/Checkouts/Renewal.pm | 7 +--- Koha/Exceptions/Checkouts/Renewals.pm | 49 ------------------------- t/db_dependent/Circulation.t | 52 ++++++++++++++++----------- t/db_dependent/Koha/Account/Line.t | 4 +-- 5 files changed, 36 insertions(+), 78 deletions(-) delete mode 100644 Koha/Exceptions/Checkouts/Renewals.pm diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 76ef6add27..a10503385f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3221,7 +3221,7 @@ sub AddRenewal { my $renewal = Koha::Checkouts::Renewal->new( { checkout_id => $issue->issue_id, - renewer_id => C4::Context->userenv->{'number'}, + renewer_id => C4::Context->userenv ? C4::Context->userenv->{'number'} : undef, seen => $seen, interface => C4::Context->interface } diff --git a/Koha/Checkouts/Renewal.pm b/Koha/Checkouts/Renewal.pm index bb8c54992a..b90072a028 100644 --- a/Koha/Checkouts/Renewal.pm +++ b/Koha/Checkouts/Renewal.pm @@ -22,7 +22,6 @@ use Modern::Perl; use base qw(Koha::Object); use Koha::Checkouts; -use Koha::Exceptions::Checkouts::Renewals; use Koha::Exceptions::Object; use Koha::Old::Checkouts; use Koha::Patrons; @@ -39,7 +38,7 @@ Koha::Checkouts::Renewal - Koha Renewal object class =head3 store - my $return_claim = Koha::Checkout::Renewal->new($args)->store; + my $renewal = Koha::Checkout::Renewal->new($args)->store; Overloaded I method that validates the attributes and raises relevant exceptions as needed. @@ -49,10 +48,6 @@ exceptions as needed. sub store { my ( $self ) = @_; - unless ( $self->in_storage || $self->renewer_id ) { - Koha::Exceptions::Checkouts::Renewals::NoRenewerID->throw(); - } - unless ( ( !$self->checkout_id && $self->in_storage ) || Koha::Checkouts->find( $self->checkout_id ) || Koha::Old::Checkouts->find( $self->checkout_id ) ) diff --git a/Koha/Exceptions/Checkouts/Renewals.pm b/Koha/Exceptions/Checkouts/Renewals.pm deleted file mode 100644 index 93ca7373d6..0000000000 --- a/Koha/Exceptions/Checkouts/Renewals.pm +++ /dev/null @@ -1,49 +0,0 @@ -package Koha::Exceptions::Checkouts::Renewals; - -# 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::Exception; - -use Exception::Class ( - 'Koha::Exceptions::Checkouts::Renewals' => { - isa => 'Koha::Exception', - }, - 'Koha::Exceptions::Checkouts::Renewals::NoRenewerID' => { - isa => 'Koha::Exceptions::Checkouts::Renewals', - description => 'renewer_id is mandatory' - }, -); - -=head1 NAME - -Koha::Exceptions::Checkouts - Base class for Checkouts exceptions - -=head1 Exceptions - -=head2 Koha::Exceptions::Checkouts::Renewals - -Generic return claim exception - -=head2 Koha::Exceptions::Checkouts::Renewals::NoRenewerID - -Exception to be used when a renewal is requested to be store but -the 'renewer_id' param is not passed. - -=cut - -1; diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e2a1d5dbf9..a7d5eaa104 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -56,9 +56,11 @@ use Koha::ActionLogs; use Koha::Notice::Messages; use Koha::Cache::Memory::Lite; +my $builder = t::lib::TestBuilder->new; sub set_userenv { my ( $library ) = @_; - t::lib::Mocks::mock_userenv({ branchcode => $library->{branchcode} }); + my $staff = $builder->build_object({ class => "Koha::Patrons" }); + t::lib::Mocks::mock_userenv({ patron => $staff, branchcode => $library->{branchcode} }); } sub str { @@ -105,7 +107,6 @@ sub test_debarment_on_checkout { my $schema = Koha::Database->schema; $schema->storage->txn_begin; -my $builder = t::lib::TestBuilder->new; my $dbh = C4::Context->dbh; # Prevent random failures by mocking ->now @@ -712,6 +713,9 @@ subtest "CanBookBeRenewed tests" => sub { # Make sure fine calculation isn't skipped when adding renewal t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1); + my $staff = $builder->build_object({ class => "Koha::Patrons" }); + t::lib::Mocks::mock_userenv({ patron => $staff }); + t::lib::Mocks::mock_preference('RenewalLog', 0); my $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } ); my %params_renewal = ( @@ -3940,26 +3944,34 @@ subtest 'ItemsDeniedRenewal preference' => sub { } }); my $future = dt_from_string->add( days => 1 ); - my $deny_issue = $builder->build_object({ class => 'Koha::Checkouts', value => { - returndate => undef, - renewals => 0, - auto_renew => 0, - borrowernumber => $idr_borrower->borrowernumber, - itemnumber => $deny_book->itemnumber, - onsite_checkout => 0, - date_due => $future, + my $deny_issue = $builder->build_object( + { + class => 'Koha::Checkouts', + value => { + returndate => undef, + renewals_count => 0, + auto_renew => 0, + borrowernumber => $idr_borrower->borrowernumber, + itemnumber => $deny_book->itemnumber, + onsite_checkout => 0, + date_due => $future, + } } - }); - my $allow_issue = $builder->build_object({ class => 'Koha::Checkouts', value => { - returndate => undef, - renewals => 0, - auto_renew => 0, - borrowernumber => $idr_borrower->borrowernumber, - itemnumber => $allow_book->itemnumber, - onsite_checkout => 0, - date_due => $future, + ); + my $allow_issue = $builder->build_object( + { + class => 'Koha::Checkouts', + value => { + returndate => undef, + renewals_count => 0, + auto_renew => 0, + borrowernumber => $idr_borrower->borrowernumber, + itemnumber => $allow_book->itemnumber, + onsite_checkout => 0, + date_due => $future, + } } - }); + ); my $idr_rules; diff --git a/t/db_dependent/Koha/Account/Line.t b/t/db_dependent/Koha/Account/Line.t index fe593ae17b..098d8e8bec 100755 --- a/t/db_dependent/Koha/Account/Line.t +++ b/t/db_dependent/Koha/Account/Line.t @@ -509,7 +509,7 @@ subtest 'Renewal related tests' => sub { value => { itemnumber => $item->itemnumber, onsite_checkout => 0, - renewals => 99, + renewals_count => 99, auto_renew => 0 } } @@ -558,7 +558,7 @@ subtest 'Renewal related tests' => sub { value => { itemnumber => $item->itemnumber, onsite_checkout => 0, - renewals => 0, + renewals_count => 0, auto_renew => 0 } } -- 2.20.1