From a68926d9cb1b485ae302a8c72f7e637fc9310fe0 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 30 Jan 2019 16:20:23 +0000 Subject: [PATCH] Bug 20912: (follow-up) Improve test coverage Increase test coverage for CanBookBeIssued and fix a introduced during the refactoring to Koha::Fees. Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: Nick Clemens --- C4/Circulation.pm | 22 +++--- Koha/Charges/Fees.pm | 12 ++-- admin/itemtypes.pl | 6 +- installer/data/mysql/kohastructure.sql | 2 +- .../prog/en/modules/admin/itemtypes.tt | 6 +- .../prog/en/modules/catalogue/moredetail.tt | 2 +- t/db_dependent/Circulation.t | 71 +++++++++++++++++-- t/db_dependent/Koha/Charges/Fees.t | 14 ++-- 8 files changed, 96 insertions(+), 39 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 23fafdd5b0..0fa6ea7390 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1004,7 +1004,7 @@ sub CanBookBeIssued { if ( $rentalConfirmation ){ my ($rentalCharge) = GetIssuingCharges( $item->itemnumber, $patron->borrowernumber ); my $itemtype = Koha::ItemTypes->find( $item->itype ); # GetItem sets effective itemtype - $rentalCharge += $itemtype->calc_rental_charge_daily( { from => dt_from_string(), to => $duedate } ); + $rentalCharge += $fees->accumulate_rentalcharge({ from => dt_from_string(), to => $duedate }); if ( $rentalCharge > 0 ){ $needsconfirmation{RENTALCHARGE} = $rentalCharge; } @@ -1464,7 +1464,7 @@ sub AddIssue { ); ModDateLastSeen( $item->itemnumber ); - # If it costs to borrow this book, charge it to the patron's account. + # If it costs to borrow this book, charge it to the patron's account. my ( $charge, $itemtype ) = GetIssuingCharges( $item->itemnumber, $borrower->{'borrowernumber'} ); if ( $charge > 0 ) { my $description = "Rental"; @@ -1473,10 +1473,10 @@ sub AddIssue { my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); if ( $itemtype ) { - my $daily_charge = $fees->rental_charge_daily(); - if ( $daily_charge > 0 ) { - AddIssuingCharge( $issue, $daily_charge, 'Daily rental' ) if $daily_charge > 0; - $charge += $daily_charge; + my $accumulate_charge = $fees->accumulate_rentalcharge(); + if ( $accumulate_charge > 0 ) { + AddIssuingCharge( $issue, $accumulate_charge, 'Daily rental' ) if $accumulate_charge > 0; + $charge += $accumulate_charge; $item->{charge} = $charge; } } @@ -2923,15 +2923,15 @@ sub AddRenewal { AddIssuingCharge($issue, $charge, $description); } - # Charge a new daily rental fee, if applicable + # Charge a new accumulate rental fee, if applicable my $itemtype = Koha::ItemTypes->find( $item_object->effective_itemtype ); if ( $itemtype ) { - my $daily_charge = $fees->rental_charge_daily(); - if ( $daily_charge > 0 ) { + my $accumulate_charge = $fees->accumulate_rentalcharge(); + if ( $accumulate_charge > 0 ) { my $type_desc = "Renewal of Daily Rental Item " . $biblio->title . " $item->{'barcode'}"; - AddIssuingCharge( $issue, $daily_charge, $type_desc ) + AddIssuingCharge( $issue, $accumulate_charge, $type_desc ) } - $charge += $daily_charge; + $charge += $accumulate_charge; } # Send a renewal slip according to checkout alert preferencei diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm index 62f534147c..0d948338b3 100644 --- a/Koha/Charges/Fees.pm +++ b/Koha/Charges/Fees.pm @@ -75,22 +75,22 @@ sub new { return bless( $params, $class ); } -=head3 rental_charge_daily +=head3 accumulate_rentalcharge - my $fee = $self->rental_charge_daily(); + my $fee = $self->accumulate_rentalcharge(); 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 { +sub accumulate_rentalcharge { my ( $self, $params ) = @_; my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); - my $rental_charge_daily = $itemtype->rental_charge_daily; + my $rentalcharge_daily = $itemtype->rentalcharge_daily; - return undef unless $rental_charge_daily && $rental_charge_daily > 0; + return undef unless $rentalcharge_daily && $rentalcharge_daily > 0; my $duration; if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) { @@ -102,7 +102,7 @@ sub rental_charge_daily { } my $days = $duration->in_units('days'); - my $charge = $rental_charge_daily * $days; + my $charge = $rentalcharge_daily * $days; return $charge; } diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl index 723dfe9c93..3381e21ec3 100755 --- a/admin/itemtypes.pl +++ b/admin/itemtypes.pl @@ -72,7 +72,7 @@ if ( $op eq 'add_form' ) { my $itemtype = Koha::ItemTypes->find($itemtype_code); my $description = $input->param('description'); my $rentalcharge = $input->param('rentalcharge'); - my $rental_charge_daily = $input->param('rental_charge_daily'); + my $rentalcharge_daily = $input->param('rentalcharge_daily'); my $defaultreplacecost = $input->param('defaultreplacecost'); my $processfee = $input->param('processfee'); my $image = $input->param('image') || q||; @@ -93,7 +93,7 @@ if ( $op eq 'add_form' ) { if ( $itemtype and $is_a_modif ) { # it's a modification $itemtype->description($description); $itemtype->rentalcharge($rentalcharge); - $itemtype->rental_charge_daily($rental_charge_daily); + $itemtype->rentalcharge_daily($rentalcharge_daily); $itemtype->defaultreplacecost($defaultreplacecost); $itemtype->processfee($processfee); $itemtype->notforloan($notforloan); @@ -118,7 +118,7 @@ if ( $op eq 'add_form' ) { itemtype => $itemtype_code, description => $description, rentalcharge => $rentalcharge, - rental_charge_daily => $rental_charge_daily, + rentalcharge_daily => $rentalcharge_daily, defaultreplacecost => $defaultreplacecost, processfee => $processfee, notforloan => $notforloan, diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 9671c0ecf9..8fad803739 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -951,7 +951,7 @@ CREATE TABLE `itemtypes` ( -- defines the item types itemtype varchar(10) NOT NULL default '', -- unique key, a code associated with the item type description LONGTEXT, -- a plain text explanation of the item type rentalcharge decimal(28,6) default NULL, -- the amount charged when this item is checked out/issued - rental_charge_daily decimal(28,6) default NULL, -- the amount charged for each day between checkout date and due date + rentalcharge_daily decimal(28,6) default NULL, -- the amount charged for each increment (day/hour) between checkout date and due date defaultreplacecost decimal(28,6) default NULL, -- default replacement cost processfee decimal(28,6) default NULL, -- default text be recorded in the column note when the processing fee is applied notforloan smallint(6) default NULL, -- 1 if the item is not for loan, 0 if the item is available for loan diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt index 6c47f91a38..0f48e11a86 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -234,8 +234,8 @@ Item types administration This fee is charged once per checkout/renewal per item
  • - - + + This fee is charged a checkout/renewal time for each day between the checkout/renewal date and due date.
  • @@ -380,7 +380,7 @@ Item types administration [% UNLESS ( itemtype.notforloan ) %] - [% itemtype.rental_charge_daily | $Price %] + [% itemtype.rentalcharge_daily | $Price %] [% END %] [% itemtype.defaultreplacecost | $Price %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt index cd83edc8ad..afe6f0a2f1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -35,7 +35,7 @@
  • Item type: [% itemtypename | html %] 
    • [% END %] [% IF ( rentalcharge ) %]
  • Rental charge:[% rentalcharge | $Price %] 
    • [% END %] - [% IF ( rental_charge_daily ) %]
  • Daily rental charge:[% rental_charge_daily | $Price %] 
    • [% END %] + [% IF ( rentalcharge_daily ) %]
  • Daily rental charge:[% rentalcharge_daily | $Price %] 
    • [% END %]
  • ISBN: [% isbn | html %] 
  • Publisher:[% place | html %] [% publishercode | html %] [% publicationyear | html %] 
    • [% IF ( volumeddesc ) %]
  • Volume: [% volumeddesc | html %]
    • [% END %] diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 345a50eea2..7ef6019a7a 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; @@ -75,7 +75,7 @@ my $itemtype = $builder->build( value => { notforloan => undef, rentalcharge => 0, - rental_charge_daily => 0, + rentalcharge_daily => 0, defaultreplacecost => undef, processfee => undef } @@ -1962,7 +1962,7 @@ subtest '_FixAccountForLostAndReturned' => sub { rentalcharge => 0, defaultreplacecost => undef, processfee => $processfee_amount, - rental_charge_daily => 0, + rentalcharge_daily => 0, } } ); @@ -2260,7 +2260,7 @@ subtest '_FixAccountForLostAndReturned' => sub { rentalcharge => 0, defaultreplacecost => undef, processfee => 0, - rental_charge_daily => 0, + rentalcharge_daily => 0, } } ); @@ -2864,7 +2864,7 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub { ); my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes', value => { rental_charge_daily => 0.00 }}); + my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes', value => { rentalcharge_daily => 0.00 }}); my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $library->id } @@ -3029,7 +3029,7 @@ subtest 'Incremented fee tests' => sub { value => { notforloan => undef, rentalcharge => 0, - rental_charge_daily => 1.000000 + rentalcharge_daily => 1.000000 } } )->store; @@ -3051,7 +3051,7 @@ subtest 'Incremented fee tests' => sub { } )->store; - is( $itemtype->rental_charge_daily, '1.000000', 'Daily rental charge stored and retreived correctly' ); + is( $itemtype->rentalcharge_daily, '1.000000', 'Daily rental charge stored and retreived correctly' ); is( $item->effective_itemtype, $itemtype->id, "Itemtype set correctly for item"); my $dt_from = dt_from_string(); @@ -3108,3 +3108,60 @@ subtest 'Incremented fee tests' => sub { $accountlines->delete(); $issue->delete(); }; + +subtest 'CanBookBeIssued & RentalFeesCheckoutConfirmation' => sub { + plan tests => 2; + + t::lib::Mocks::mock_preference('RentalFeesCheckoutConfirmation', 1); + t::lib::Mocks::mock_preference('item-level_itypes', 1); + + my $library = + $builder->build_object( { class => 'Koha::Libraries' } )->store; + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + } + )->store; + + my $itemtype = $builder->build_object( + { + class => 'Koha::ItemTypes', + value => { + notforloan => 0, + rentalcharge => 0, + rentalcharge_daily => 0 + } + } + ); + + my $biblioitem = $builder->build( { source => 'Biblioitem' } ); + my $item = $builder->build_object( + { + class => 'Koha::Items', + value => { + homebranch => $library->id, + holdingbranch => $library->id, + notforloan => 0, + itemlost => 0, + withdrawn => 0, + itype => $itemtype->id, + biblionumber => $biblioitem->{biblionumber}, + biblioitemnumber => $biblioitem->{biblioitemnumber}, + } + } + )->store; + + my ( $issuingimpossible, $needsconfirmation ); + my $dt_from = dt_from_string(); + my $dt_due = dt_from_string()->add( days => 3 ); + + $itemtype->rentalcharge('1.000000')->store; + ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode, $dt_due, undef, undef, undef ); + is_deeply( $needsconfirmation, { RENTALCHARGE => '1' }, 'Item needs rentalcharge confirmation to be issued' ); + $itemtype->rentalcharge('0')->store; + $itemtype->rentalcharge_daily('1.000000')->store; + ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode, $dt_due, undef, undef, undef ); + is_deeply( $needsconfirmation, { RENTALCHARGE => '3' }, 'Item needs rentalcharge confirmation to be issued, increment' ); + $itemtype->rentalcharge_daily('0')->store; +}; diff --git a/t/db_dependent/Koha/Charges/Fees.t b/t/db_dependent/Koha/Charges/Fees.t index 55c324216b..cc555779b5 100644 --- a/t/db_dependent/Koha/Charges/Fees.t +++ b/t/db_dependent/Koha/Charges/Fees.t @@ -58,7 +58,7 @@ my $itemtype = $builder->build_object( { class => 'Koha::ItemTypes', value => { - rental_charge_daily => '0.00', + rentalcharge_daily => '0.00', rentalcharge => '0.00', processfee => '0.00', defaultreplacecost => '0.00', @@ -99,20 +99,20 @@ my $fees = Koha::Charges::Fees->new( } ); -subtest 'Koha::ItemType::rental_charge_daily tests' => sub { +subtest 'accumulate_rentalcharge tests' => sub { plan tests => 4; - $itemtype->rental_charge_daily(1.00); + $itemtype->rentalcharge_daily(1.00); $itemtype->store(); - is( $itemtype->rental_charge_daily, + is( $itemtype->rentalcharge_daily, 1.00, 'Daily return charge stored correctly' ); t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' ); - my $charge = $fees->rental_charge_daily(); + my $charge = $fees->accumulate_rentalcharge(); is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' ); t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' ); - $charge = $fees->rental_charge_daily(); + $charge = $fees->accumulate_rentalcharge(); is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' ); my $calendar = C4::Calendar->new( branchcode => $library->id ); @@ -121,6 +121,6 @@ subtest 'Koha::ItemType::rental_charge_daily tests' => sub { title => 'Test holiday', description => 'Test holiday' ); - $charge = $fees->rental_charge_daily(); + $charge = $fees->accumulate_rentalcharge(); is( $charge, 5.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' ); }; -- 2.39.5