From 3d6f21a33a9f7ea5b047ba93969b302b275fee9f Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 6 Aug 2019 14:38:28 +0100
Subject: [PATCH] Bug 23382: Improve test coverage

---
 Koha/Charges/Fees.pm               |  30 +++-
 t/db_dependent/Koha/Charges/Fees.t | 260 ++++++++++++++++++++++++++---
 2 files changed, 264 insertions(+), 26 deletions(-)

diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm
index 41299cee14..96a9897ac1 100644
--- a/Koha/Charges/Fees.pm
+++ b/Koha/Charges/Fees.pm
@@ -19,7 +19,7 @@ package Koha::Charges::Fees;
 
 use Modern::Perl;
 
-use Carp qw( confess );
+use Carp qw( carp confess );
 
 use Koha::Calendar;
 use Koha::IssuingRules;
@@ -82,7 +82,7 @@ sub new {
 
     my $fee = $self->accumulate_rentalcharge();
 
-    This method calculates the daily rental fee for a given itemtype for a given
+    This method calculates the daily/hourly rental fee for a given itemtype for a given
     period of time passed in as a pair of DateTime objects.
 
 =cut
@@ -138,7 +138,10 @@ my $patron = $fees->patron( $patron );
 sub patron {
     my ( $self, $patron ) = @_;
 
-    $self->{patron} = $patron if $patron && $patron->isa('Koha::Patron');
+    Carp::carp("Setting 'patron' to something other than a Koha::Patron is not supported!")
+      if ($patron && !$patron->isa('Koha::Patron'));
+
+    $self->{patron} = $patron if $patron;
 
     return $self->{patron};
 }
@@ -152,7 +155,10 @@ my $library = $fees->library( $library );
 sub library {
     my ( $self, $library ) = @_;
 
-    $self->{library} = $library if $library && $library->isa('Koha::Library');
+    Carp::carp("Setting 'library' to something other than a Koha::Library is not supported!")
+      if ($library && !$library->isa('Koha::Library'));
+
+    $self->{library} = $library if $library;
 
     return $self->{library};
 }
@@ -166,7 +172,10 @@ my $item = $fees->item( $item );
 sub item {
     my ( $self, $item ) = @_;
 
-    $self->{item} = $item if $item && $item->isa('Koha::Item');
+    Carp::carp("Setting 'item' to something other than a Koha::Item is not supported!")
+      if ($item && !$item->isa('Koha::Item'));
+
+    $self->{item} = $item if $item;
 
     return $self->{item};
 }
@@ -180,7 +189,10 @@ my $to_date = $fees->to_date( $to_date );
 sub to_date {
     my ( $self, $to_date ) = @_;
 
-    $self->{to_date} = $to_date if $to_date && $to_date->isa('DateTime');
+    Carp::carp("Setting 'to_date' to something other than a DateTime is not supported!")
+      if ($to_date && !$to_date->isa('DateTime'));
+
+    $self->{to_date} = $to_date if $to_date;
 
     return $self->{to_date};
 }
@@ -194,14 +206,18 @@ my $from_date = $fees->from_date( $from_date );
 sub from_date {
     my ( $self, $from_date ) = @_;
 
+    Carp::carp("Setting 'from_date' to something other than a DateTime is not supported!")
+      if ($from_date && !$from_date->isa('DateTime'));
+
     $self->{from_date} = $from_date if $from_date && $from_date->isa('DateTime');
 
     return $self->{from_date};
 }
 
-=head1 AUTHOR
+=head1 AUTHORS
 
 Kyle M Hall <kyle.m.hall@gmail.com>
+Martin Renvoize <martin.renvoize@ptfs-europe.com>
 
 =cut
 
diff --git a/t/db_dependent/Koha/Charges/Fees.t b/t/db_dependent/Koha/Charges/Fees.t
index cc555779b5..1722fc191c 100644
--- a/t/db_dependent/Koha/Charges/Fees.t
+++ b/t/db_dependent/Koha/Charges/Fees.t
@@ -19,13 +19,14 @@
 
 use Modern::Perl;
 
-use Test::More tests => 2;
+use Test::More tests => 8;
+use Test::Exception;
+use Test::Warn;
 
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
-use Data::Dumper;
-
+use Time::Fake;
 use C4::Calendar;
 use Koha::DateUtils qw(dt_from_string);
 
@@ -59,9 +60,9 @@ my $itemtype = $builder->build_object(
         class => 'Koha::ItemTypes',
         value => {
             rentalcharge_daily => '0.00',
-            rentalcharge        => '0.00',
-            processfee          => '0.00',
-            defaultreplacecost  => '0.00',
+            rentalcharge       => '0.00',
+            processfee         => '0.00',
+            defaultreplacecost => '0.00',
         },
     }
 );
@@ -86,22 +87,235 @@ my $patron = $builder->build_object(
     }
 );
 
-my $dt_from = dt_from_string();
-my $dt_to = dt_from_string()->add( days => 6 );
+my $dt = dt_from_string();
+Time::Fake->offset( $dt->epoch );
 
-my $fees = Koha::Charges::Fees->new(
-    {
-        patron    => $patron,
-        library   => $library,
-        item      => $item,
-        to_date   => $dt_to,
-        from_date => $dt_from,
+my $dt_from = dt_from_string()->subtract( days => 2 );
+my $dt_to = dt_from_string()->add( days => 4 );
+
+subtest 'new' => sub {
+    plan tests => 9;
+
+    # Mandatory parameters missing
+    throws_ok {
+        Koha::Charges::Fees->new(
+            {
+                library => $library,
+                item    => $item,
+                to_date => $dt_to,
+            }
+          )
     }
-);
+    'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for patron';
+    throws_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                item    => $item,
+                to_date => $dt_to,
+            }
+          )
+    }
+    'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for library';
+    throws_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                library => $library,
+                to_date => $dt_to,
+            }
+          )
+    }
+    'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for item';
+    throws_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                library => $library,
+                item    => $item,
+            }
+          )
+    }
+    'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for to_date';
+
+    # Mandatory parameter bad
+    dies_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => '12345',
+                library => $library,
+                item    => $item,
+                to_date => $dt_to,
+            }
+          )
+    }
+    'dies for bad patron';
+    dies_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                library => '12345',
+                item    => $item,
+                to_date => $dt_to,
+            }
+          )
+    }
+    'dies for bad library';
+    dies_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                library => $library,
+                item    => '12345',
+                to_date => $dt_to,
+            }
+          )
+    }
+    'dies for bad item';
+    dies_ok {
+        Koha::Charges::Fees->new(
+            {
+                patron  => $patron,
+                library => $library,
+                item    => $item,
+                to_date => 12345
+            }
+          )
+    }
+    'dies for bad to_date';
+
+    # Defaults
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+    is( $fees->from_date, dt_from_string(),
+        'from_date default set correctly to today' );
+};
+
+subtest 'patron accessor' => sub {
+    plan tests => 2;
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+
+    ok(
+        $fees->patron->isa('Koha::Patron'),
+        'patron accessor returns a Koha::Patron'
+    );
+    warning_is { $fees->patron('12345') }
+    { carped =>
+"Setting 'patron' to something other than a Koha::Patron is not supported!"
+    }, "Warning thrown when attempting to set patron to string";
+
+};
+
+subtest 'library accessor' => sub {
+    plan tests => 2;
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+
+    ok(
+        $fees->library->isa('Koha::Library'),
+        'library accessor returns a Koha::Library'
+    );
+    warning_is { $fees->library('12345') }
+    { carped =>
+"Setting 'library' to something other than a Koha::Library is not supported!"
+    }, "Warning thrown when attempting to set library to string";
+};
+
+subtest 'item accessor' => sub {
+    plan tests => 2;
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+
+    ok( $fees->item->isa('Koha::Item'), 'item accessor returns a Koha::Item' );
+    warning_is { $fees->item('12345') }
+    { carped =>
+"Setting 'item' to something other than a Koha::Item is not supported!"
+    }, "Warning thrown when attempting to set item to string";
+};
+
+subtest 'to_date accessor' => sub {
+    plan tests => 2;
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+
+    ok( $fees->to_date->isa('DateTime'),
+        'to_date accessor returns a DateTime' );
+    warning_is { $fees->to_date(12345) }
+    { carped =>
+"Setting 'to_date' to something other than a DateTime is not supported!"
+    }, "Warning thrown when attempting to set to_date to integer";
+};
+
+subtest 'from_date accessor' => sub {
+    plan tests => 2;
+
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron  => $patron,
+            library => $library,
+            item    => $item,
+            to_date => $dt_to,
+        }
+    );
+
+    ok(
+        $fees->from_date->isa('DateTime'),
+        'from_date accessor returns a DateTime'
+    );
+    warning_is { $fees->from_date(12345) }
+    { carped =>
+"Setting 'from_date' to something other than a DateTime is not supported!"
+    }, "Warning thrown when attempting to set from_date to integer";
+};
 
 subtest 'accumulate_rentalcharge tests' => sub {
     plan tests => 4;
 
+    my $fees = Koha::Charges::Fees->new(
+        {
+            patron    => $patron,
+            library   => $library,
+            item      => $item,
+            to_date   => $dt_to,
+            from_date => $dt_from,
+        }
+    );
+
     $itemtype->rentalcharge_daily(1.00);
     $itemtype->store();
     is( $itemtype->rentalcharge_daily,
@@ -109,11 +323,15 @@ subtest 'accumulate_rentalcharge tests' => sub {
 
     t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' );
     my $charge = $fees->accumulate_rentalcharge();
-    is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' );
+    is( $charge, 6.00,
+'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar'
+    );
 
     t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' );
     $charge = $fees->accumulate_rentalcharge();
-    is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' );
+    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(
@@ -122,5 +340,9 @@ subtest 'accumulate_rentalcharge tests' => sub {
         description => 'Test holiday'
     );
     $charge = $fees->accumulate_rentalcharge();
-    is( $charge, 5.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' );
+    is( $charge, 5.00,
+'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays'
+    );
 };
+
+Time::Fake->reset;
-- 
2.20.1