From 674b8ab771f7986d5cd6a3e0c55eeb686c574f04 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 9 Nov 2018 13:34:05 -0500 Subject: [PATCH] Bug 20912: Move calculation to Koha::Fees Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 57 +++++++++- Koha/Fees.pm | 186 ++++++++++++++++++++++++++++++++ Koha/ItemType.pm | 35 ------ t/db_dependent/Circulation.t | 8 +- t/db_dependent/Koha/Fees.t | 126 ++++++++++++++++++++++ t/db_dependent/Koha/ItemTypes.t | 44 +------- 6 files changed, 369 insertions(+), 87 deletions(-) create mode 100644 Koha/Fees.pm create mode 100644 t/db_dependent/Koha/Fees.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 944d6c2876..d1865411eb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -59,6 +59,7 @@ use Koha::RefundLostItemFeeRules; use Koha::Account::Lines; use Koha::Account::Offsets; use Koha::Config::SysPrefs; +use Koha::Fees; use Carp; use List::MoreUtils qw( uniq any ); use Scalar::Util qw( looks_like_number ); @@ -670,13 +671,17 @@ sub CanBookBeIssued { my $onsite_checkout = $params->{onsite_checkout} || 0; my $override_high_holds = $params->{override_high_holds} || 0; - my $item = GetItem(undef, $barcode ); + my $item_object = Koha::Items->find( { barcode => $barcode } ); + # MANDATORY CHECKS - unless item exists, nothing else matters - unless ( $item ) { + unless ( $item_object ) { $issuingimpossible{UNKNOWN_BARCODE} = 1; } return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible; + my $item = $item_object->unblessed; + $item->{itype} = $item_object->effective_itemtype(); # set the correct itype + my $issue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } ); my $biblio = Koha::Biblios->find( $item->{biblionumber} ); my $biblioitem = $biblio->biblioitem; @@ -684,6 +689,7 @@ sub CanBookBeIssued { my $dbh = C4::Context->dbh; my $patron_unblessed = $patron->unblessed; + my $library = Koha::Libraries->find( _GetCircControlBranch($item, $patron_unblessed) ); # # DUE DATE is OK ? -- should already have checked. # @@ -694,12 +700,22 @@ sub CanBookBeIssued { unless ( $duedate ) { my $issuedate = $now->clone(); - my $branch = _GetCircControlBranch($item, $patron_unblessed); + my $branch = $library->id; $duedate = CalcDateDue( $issuedate, $effective_itemtype, $branch, $patron_unblessed ); # Offline circ calls AddIssue directly, doesn't run through here # So issuingimpossible should be ok. } + + my $fees = Koha::Fees->new( + { + patron => $patron, + library => $library, + item => $item_object, + to_date => $duedate, + } + ); + if ($duedate) { my $today = $now->clone(); $today->truncate( to => 'minute'); @@ -1312,6 +1328,24 @@ sub AddIssue { ); } else { + unless ($datedue) { + my $itype = $item_object->effective_itemtype; + $datedue = CalcDateDue( $issuedate, $itype, $branch, $borrower ); + + } + $datedue->truncate( to => 'minute' ); + + my $patron = Koha::Patrons->find( $borrower ); + my $library = Koha::Libraries->find( $branch ); + my $fees = Koha::Fees->new( + { + patron => $patron, + library => $library, + item => $item_object, + to_date => $datedue, + } + ); + # it's NOT a renewal if ( $actualissue and not $switch_onsite_checkout ) { # This book is currently on loan, but not to the person @@ -1429,7 +1463,7 @@ sub AddIssue { my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); if ( $itemtype ) { - my $daily_charge = $itemtype->calc_rental_charge_daily( { from => $issuedate, to => $datedue } ); + my $daily_charge = $fees->rental_charge_daily(); if ( $daily_charge > 0 ) { AddIssuingCharge( $issue, $daily_charge, 'Daily rental' ) if $daily_charge > 0; $charge += $daily_charge; @@ -2851,6 +2885,9 @@ sub AddRenewal { my $patron = Koha::Patrons->find( $borrowernumber ) or return; # FIXME Should do more than just return my $patron_unblessed = $patron->unblessed; + my $library = Koha::Libraries->find( $branch ); + + if ( C4::Context->preference('CalculateFinesOnReturn') && $issue->is_overdue ) { _CalculateAndUpdateFine( { issue => $issue, item => $item, borrower => $patron_unblessed } ); } @@ -2868,6 +2905,16 @@ sub AddRenewal { $datedue = CalcDateDue($datedue, $itemtype, _GetCircControlBranch($item, $patron_unblessed), $patron_unblessed, 'is a renewal'); } + + my $fees = Koha::Fees->new( + { + patron => $patron, + library => $library, + item => $item_object, + to_date => dt_from_string( $datedue ), + } + ); + # Update the issues record to have the new due date, and a new count # of how many times it has been renewed. my $renews = $issue->renewals + 1; @@ -2910,7 +2957,7 @@ sub AddRenewal { # Charge a new daily rental fee, if applicable my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); if ( $itemtype ) { - my $daily_charge = $itemtype->calc_rental_charge_daily( { from => dt_from_string($lastreneweddate), to => $datedue } ); + my $daily_charge = $fees->rental_charge_daily(); if ( $daily_charge > 0 ) { my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item->{'barcode'}"; AddIssuingCharge( $issue, $daily_charge, $type_desc ) diff --git a/Koha/Fees.pm b/Koha/Fees.pm new file mode 100644 index 0000000000..b2bfc4f0bd --- /dev/null +++ b/Koha/Fees.pm @@ -0,0 +1,186 @@ +package Koha::Fees; + +# Copyright 2018 ByWater Solutions +# +# 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 Carp qw( confess ); + +use Koha::Calendar; +use Koha::DateUtils qw( dt_from_string ); +use Koha::Exceptions; + +=head1 NAME + +Koha::Feess - Module calculating fees in Koha + +=head3 new + +Koha::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $to_dt, + [ from_date => $from_dt, ] + } +); + +=cut + +sub new { + my ( $class, $params ) = @_; + + Koha::Exceptions::MissingParameter->throw("Missing mandatory parameter: patron") + unless $patron; + Koha::Exceptions::MissingParameter->throw("Missing mandatory parameter: library") + unless $library; + Koha::Exceptions::MissingParameter->throw("Missing mandatory parameter: item") + unless $item; + Koha::Exceptions::MissingParameter->throw("Missing mandatory parameter: to_date") + unless $to_date; + + Carp::confess("Key 'patron' is not a Koha::Patron object!") + unless $params->{patron}->isa('Koha::Patron'); + Carp::confess("Key 'library' is not a Koha::Library object!") + unless $params->{library}->isa('Koha::Library'); + Carp::confess("Key 'item' is not a Koha::Item object!") + unless $params->{item}->isa('Koha::Item'); + Carp::confess("Key 'to_date' is not a DateTime object!") + unless $params->{to_date}->isa('DateTime'); + + if ( $params->{from_date} ) { + Carp::croak("Key 'from_date' is not a DateTime object!") + unless $params->{from_date}->isa('DateTime'); + } + else { + $params->{from_date} = dt_from_string(); + } + + return bless( $params, $class ); +} + +=head3 rental_charge_daily + + my $fee = $self->rental_charge_daily(); + + This method calculates the daily rental fee for a given itemtype for a given + period of time passed in as a pair of DateTime objects. + +=cut + +sub rental_charge_daily { + my ( $self, $params ) = @_; + + my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); + my $rental_charge_daily = $itemtype->rental_charge_daily; + + return undef unless $rental_charge_daily && $rental_charge_daily > 0; + + my $duration; + if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) { + my $calendar = Koha::Calendar->new( branchcode => $self->library->id ); + $duration = $calendar->days_between( $self->from_date, $self->to_date ); + } + else { + $duration = $self->to_date->delta_days($self->from_date); + } + my $days = $duration->in_units('days'); + + my $charge = $rental_charge_daily * $days; + + return $charge; +} + +=head3 patron + +my $patron = $fees->patron( $patron ); + +=cut + +sub patron { + my ( $self, $patron ) = @_; + + $self->{patron} = $patron if $patron && $patron->isa('Koha::Patron'); + + return $self->{patron}; +} + +=head3 library + +my $library = $fees->library( $library ); + +=cut + +sub library { + my ( $self, $library ) = @_; + + $self->{library} = $library if $library && $library->isa('Koha::Library'); + + return $self->{library}; +} + +=head3 item + +my $item = $fees->item( $item ); + +=cut + +sub item { + my ( $self, $item ) = @_; + + $self->{item} = $item if $item && $item->isa('Koha::Item'); + + return $self->{item}; +} + +=head3 to_date + +my $to_date = $fees->to_date( $to_date ); + +=cut + +sub to_date { + my ( $self, $to_date ) = @_; + + $self->{to_date} = $to_date if $to_date && $to_date->isa('DateTime'); + + return $self->{to_date}; +} + +=head3 from_date + +my $from_date = $fees->from_date( $from_date ); + +=cut + +sub from_date { + my ( $self, $from_date ) = @_; + + $self->{from_date} = $from_date if $from_date && $from_date->isa('DateTime'); + + return $self->{from_date}; +} + +=head1 AUTHOR + +Kyle M Hall + +=cut + +1; diff --git a/Koha/ItemType.pm b/Koha/ItemType.pm index 5faf499a69..31578f05d0 100644 --- a/Koha/ItemType.pm +++ b/Koha/ItemType.pm @@ -90,41 +90,6 @@ sub translated_descriptions { } @translated_descriptions ]; } -=head3 calc_rental_charge_daily - - my $fee = $itemtype->calc_rental_charge_daily( { from => $dt_from, to => $dt_to } ); - - This method calculates the daily rental fee for a given itemtype for a given - period of time passed in as a pair of DateTime objects. - -=cut - -sub calc_rental_charge_daily { - my ( $self, $params ) = @_; - - my $rental_charge_daily = $self->rental_charge_daily; - return 0 unless $rental_charge_daily; - - my $from_dt = $params->{from}; - my $to_dt = $params->{to}; - - my $duration; - if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) { - my $branchcode = C4::Context->userenv->{branch}; - my $calendar = Koha::Calendar->new( branchcode => $branchcode ); - $duration = $calendar->days_between( $from_dt, $to_dt ); - } - else { - $duration = $to_dt->delta_days($from_dt); - } - my $days = $duration->in_units('days'); - - my $charge = $rental_charge_daily * $days; - - return $charge; -} - - =head3 can_be_deleted my $can_be_deleted = Koha::ItemType->can_be_deleted(); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 2055def6c4..6bfe8b886a 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 125; +use Test::More tests => 126; use Test::MockModule; use Data::Dumper; @@ -3049,7 +3049,7 @@ subtest 'Koha::ItemType::calc_rental_charge_daily tests' => sub { $accountline->delete(); AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to ); $accountline = Koha::Account::Lines->find({ itemnumber => $item->id }); - is( $accountline->amount, '6.000000', "Daily rental charge calculated correctly with finesCalendar = ignoreCalendar, for renewal" ); + is( $accountline->amount, '13.000000', "Daily rental charge calculated correctly with finesCalendar = ignoreCalendar, for renewal" ); $accountline->delete(); $issue->delete(); @@ -3060,7 +3060,7 @@ subtest 'Koha::ItemType::calc_rental_charge_daily tests' => sub { $accountline->delete(); AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to ); $accountline = Koha::Account::Lines->find({ itemnumber => $item->id }); - is( $accountline->amount, '6.000000', "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed, for renewal" ); + is( $accountline->amount, '13.000000', "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed, for renewal" ); $accountline->delete(); $issue->delete(); @@ -3076,7 +3076,7 @@ subtest 'Koha::ItemType::calc_rental_charge_daily tests' => sub { $accountline->delete(); AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to ); $accountline = Koha::Account::Lines->find({ itemnumber => $item->id }); - is( $accountline->amount, '5.000000', "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays, for renewal" ); + is( $accountline->amount, '11.000000', "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays, for renewal" ); $accountline->delete(); $issue->delete(); diff --git a/t/db_dependent/Koha/Fees.t b/t/db_dependent/Koha/Fees.t new file mode 100644 index 0000000000..db16a8236c --- /dev/null +++ b/t/db_dependent/Koha/Fees.t @@ -0,0 +1,126 @@ +#!/usr/bin/perl +# +# Copyright 2018 ByWater Solutions +# +# 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, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Test::More tests => 2; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +use Data::Dumper; + +use C4::Calendar; +use Koha::DateUtils qw(dt_from_string); + +BEGIN { + use_ok('Koha::Fees'); +} + +my $builder = t::lib::TestBuilder->new(); + +my $patron_category = $builder->build_object( + { + class => 'Koha::Patron::Categories', + value => { + category_type => 'P', + enrolmentfee => 0, + } + } +); +my $library = $builder->build_object( + { + class => 'Koha::Libraries', + } +); +my $biblio = $builder->build_object( + { + class => 'Koha::Biblios', + } +); +my $itemtype = $builder->build_object( + { + class => 'Koha::ItemTypes', + value => { + rental_charge_daily => '0.00', + rentalcharge => '0.00', + processfee => '0.00', + defaultreplacecost => '0.00', + }, + } +); +my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { + biblionumber => $biblio->id, + homebranch => $library->id, + holdingbranch => $library->id, + itype => $itemtype->id, + } + } +); +my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + dateexpiry => '9999-12-31', + categorycode => $patron_category->id, + } + } +); + +my $dt_from = dt_from_string(); +my $dt_to = dt_from_string()->add( days => 6 ); + +my $fees = Koha::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + from_date => $dt_from, + } +); + +subtest 'Koha::ItemType::rental_charge_daily tests' => sub { + plan tests => 4; + + $itemtype->rental_charge_daily(1.00); + $itemtype->store(); + is( $itemtype->rental_charge_daily, + 1.00, 'Daily return charge stored correctly' ); + + t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' ); + my $charge = $fees->rental_charge_daily(); + is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' ); + + t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' ); + $charge = $fees->rental_charge_daily(); + is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' ); + + my $calendar = C4::Calendar->new( branchcode => $library->id ); + $calendar->insert_week_day_holiday( + weekday => 3, + title => 'Test holiday', + description => 'Test holiday' + ); + $charge = $fees->rental_charge_daily(); + is( $charge, 5.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' ); +}; diff --git a/t/db_dependent/Koha/ItemTypes.t b/t/db_dependent/Koha/ItemTypes.t index ebd3597931..2baef5a7ed 100755 --- a/t/db_dependent/Koha/ItemTypes.t +++ b/t/db_dependent/Koha/ItemTypes.t @@ -20,7 +20,7 @@ use Modern::Perl; use Data::Dumper; -use Test::More tests => 25; +use Test::More tests => 24; use t::lib::Mocks; use t::lib::TestBuilder; @@ -155,46 +155,4 @@ $biblioitem->delete; is ( $item_type->can_be_deleted, 1, 'The item type that was being used by the removed item and biblioitem can now be deleted' ); -subtest 'Koha::ItemType::calc_rental_charge_daily tests' => sub { - plan tests => 4; - - my $library = Koha::Libraries->search()->next(); - my $module = new Test::MockModule('C4::Context'); - $module->mock('userenv', sub { { branch => $library->id } }); - - my $itemtype = Koha::ItemType->new( - { - itemtype => 'type4', - description => 'description', - rental_charge_daily => 1.00, - rentalcharge => '0.00', - processfee => '0.00', - defaultreplacecost => '0.00', - } - )->store; - - is( $itemtype->rental_charge_daily, 1.00, 'Daily rental charge stored and retreived correctly' ); - - my $dt_from = dt_from_string(); - my $dt_to = dt_from_string()->add( days => 6 ); - - t::lib::Mocks::mock_preference('finesCalendar', 'ignoreCalendar'); - my $charge = $itemtype->calc_rental_charge_daily( { from => $dt_from, to => $dt_to } ); - is( $charge, 6.00, "Daily rental charge calculated correctly with finesCalendar = ignoreCalendar" ); - - t::lib::Mocks::mock_preference('finesCalendar', 'noFinesWhenClosed'); - $charge = $itemtype->calc_rental_charge_daily( { from => $dt_from, to => $dt_to } ); - is( $charge, 6.00, "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed" ); - - my $calendar = C4::Calendar->new( branchcode => $library->id ); - $calendar->insert_week_day_holiday( - weekday => 3, - title => 'Test holiday', - description => 'Test holiday' - ); - $charge = $itemtype->calc_rental_charge_daily( { from => $dt_from, to => $dt_to } ); - is( $charge, 5.00, "Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays" ); - -}; - $schema->txn_rollback; -- 2.20.1