From f6b5b5349f988451794eb9d6ac796f38033b0c79 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 --- C4/Circulation.pm | 22 +++--- Koha/Charges/Fees.pm | 12 ++-- Koha/Schema/Result/Itemtype.pm | 4 +- 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 ++-- 9 files changed, 98 insertions(+), 41 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 6c7ae420ee..3641aebc9f 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -994,7 +994,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(); if ( $rentalCharge > 0 ){ $needsconfirmation{RENTALCHARGE} = $rentalCharge; } @@ -1454,7 +1454,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 ) = GetIssuingCharges( $item->{'itemnumber'}, $borrower->{'borrowernumber'} ); if ( $charge > 0 ) { AddIssuingCharge( $issue, $charge ); @@ -1463,10 +1463,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; } } @@ -2954,15 +2954,15 @@ sub AddRenewal { )->store(); } - # 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/Koha/Schema/Result/Itemtype.pm b/Koha/Schema/Result/Itemtype.pm index a7c84f1046..6f022e986f 100644 --- a/Koha/Schema/Result/Itemtype.pm +++ b/Koha/Schema/Result/Itemtype.pm @@ -41,7 +41,7 @@ __PACKAGE__->table("itemtypes"); is_nullable: 1 size: [28,6] -=head2 rental_charge_daily +=head2 rentalcharge_daily data_type: 'decimal' is_nullable: 1 @@ -115,7 +115,7 @@ __PACKAGE__->add_columns( { data_type => "longtext", is_nullable => 1 }, "rentalcharge", { data_type => "decimal", is_nullable => 1, size => [28, 6] }, - "rental_charge_daily", + "rentalcharge_daily", { data_type => "decimal", is_nullable => 1, size => [28, 6] }, "defaultreplacecost", { data_type => "decimal", is_nullable => 1, size => [28, 6] }, 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 264d4b1bbe..dcb6447c25 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -988,7 +988,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 6c8b0e71ba..e692fc7950 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -235,8 +235,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.
  • @@ -381,7 +381,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 183e78f949..53225a2ce9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -34,7 +34,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 9f9b47230b..839e3fd818 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; @@ -73,7 +73,7 @@ my $itemtype = $builder->build( value => { notforloan => undef, rentalcharge => 0, - rental_charge_daily => 0, + rentalcharge_daily => 0, defaultreplacecost => undef, processfee => undef } @@ -1951,7 +1951,7 @@ subtest '_FixAccountForLostAndReturned' => sub { rentalcharge => 0, defaultreplacecost => undef, processfee => $processfee_amount, - rental_charge_daily => 0, + rentalcharge_daily => 0, } } ); @@ -2249,7 +2249,7 @@ subtest '_FixAccountForLostAndReturned' => sub { rentalcharge => 0, defaultreplacecost => undef, processfee => 0, - rental_charge_daily => 0, + rentalcharge_daily => 0, } } ); @@ -2853,7 +2853,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 } @@ -3016,7 +3016,7 @@ subtest 'Incremented fee tests' => sub { value => { notforloan => undef, rentalcharge => 0, - rental_charge_daily => 1.000000 + rentalcharge_daily => 1.000000 } } )->store; @@ -3038,7 +3038,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(); @@ -3095,3 +3095,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.20.1