Attachment 131929 Details for Bug 17983 – fixed patch tests and minor adjustments

[patch] fixed patch tests and minor adjustments

fixed-patch-tests-and-minor-adjustments.patch (text/plain), 18.13 KB, created by Shi Yao Wang on 2022-03-18 18:18:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Shi Yao Wang

Created: 2022-03-18 18:18:41 UTC

Size: 18.13 KB

patch

obsolete

>From b74069ff1b54a23dfac421a108cb9d13b0e4f7cb Mon Sep 17 00:00:00 2001
>From: shiyao <shiyao@inlibro.com>
>Date: Fri, 18 Mar 2022 14:15:12 -0400
>Subject: [PATCH] fixed patch tests and minor adjustments
>
>https://bugs.koha-community.org/show_bug.cgi?id=17983
>---
> C4/Circulation.pm                             |  21 +-
> C4/Overdues.pm                                |  12 +
> Koha/DiscreteCalendar.pm                      |  96 ++++++
> .../prog/en/modules/admin/smart-rules.tt      |   3 +
> t/db_dependent/MinuteLoans.t                  | 283 ++++++++++++++++++
> 5 files changed, 414 insertions(+), 1 deletion(-)
> create mode 100644 t/db_dependent/MinuteLoans.t
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index a8199564af..c96bb5126b 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -3710,11 +3710,14 @@ sub CalcDateDue {
>         }
>     );
> 
>+    my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branch });
>     # calculate the datedue as normal
>     if ( $daysmode eq 'Days' )
>     {    # ignoring calendar
>         if ( $loanlength->{lengthunit} eq 'hours' ) {
>             $datedue->add( hours => $loanlength->{$length_key} );
>+	    } elsif ( $loanlength->{lengthunit} eq 'minutes' ) {
>+	        $datedue->add( minutes => $loanlength->{$length_key} );
>         } else {    # days
>             $datedue->add( days => $loanlength->{$length_key} );
>             $datedue->set_hour(23);
>@@ -3725,10 +3728,13 @@ sub CalcDateDue {
>         if ($loanlength->{lengthunit} eq 'hours') {
>             $dur = DateTime::Duration->new( hours => $loanlength->{$length_key});
>         }
>+	    elsif ($loanlength->{lengthunit} eq 'minutes') {
>+	        $dur = DateTime::Duration->new( minutes => $loanlength->{$length_key});
>+	    }
>         else { # days
>             $dur = DateTime::Duration->new( days => $loanlength->{$length_key});
>         }
>-        my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branch, days_mode => $daysmode });
>+	##my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branch, days_mode => $daysmode });
>         $datedue = $calendar->addDuration( $datedue, $dur, $loanlength->{lengthunit} );
>         if ($loanlength->{lengthunit} eq 'days') {
>             $datedue->set_hour(23);
>@@ -3774,6 +3780,19 @@ sub CalcDateDue {
>           }
>         }
>     }
>+
>+    #check if it's minutes or hourly loan and set due date to close hour, if the current due date is passed closing hours.
>+    if ($loanlength->{lengthunit} eq 'hours' || $loanlength->{lengthunit} eq 'minutes') {
>+	    my $dateInfo = $calendar->get_date_info($datedue);
>+	    my $close = dt_from_string($dateInfo->{date} ." ". $dateInfo->{close_hour}, 'iso', C4::Context->tz());
>+
>+	    my $close_datetime = $datedue->clone()->set(hour => $close->hour(), minute=> $close->minute());
>+
>+	    if(DateTime->compare($datedue, $close_datetime) > 0) {
>+	        $datedue = $close_datetime->clone();
>+	    }
>+    }
>+
>     return $datedue;
> }
> 
>diff --git a/C4/Overdues.pm b/C4/Overdues.pm
>index 357037a834..5d83ab0cd8 100644
>--- a/C4/Overdues.pm
>+++ b/C4/Overdues.pm
>@@ -331,6 +331,18 @@ sub get_chargeable_units {
>         }
>         return $charge_duration->in_units('hours');
>     }
>+    elsif($unit eq 'minutes'){
>+	    if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
>+	        my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branchcode });
>+	        $charge_duration = $calendar->open_minutes_between( $date_due, $date_returned );
>+	    } else {
>+	        $charge_duration = $date_returned->delta_ms( $date_due );
>+        }
>+	    if($charge_duration == 0 && $charge_duration * 60 > 0) {
>+	        return 1;
>+    	}
>+	    return $charge_duration;
>+    }
>     else { # days
>         if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') {
>             my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branchcode });
>diff --git a/Koha/DiscreteCalendar.pm b/Koha/DiscreteCalendar.pm
>index 9ee432e6ad..8ce4907000 100644
>--- a/Koha/DiscreteCalendar.pm
>+++ b/Koha/DiscreteCalendar.pm
>@@ -1246,6 +1246,75 @@ sub open_hours_between {
>     return ($working_hours_between->next()->get_column('hours_between') - $not_used_hours->next()->get_column('not_used_hours'));
> }
> 
>+=head2 open_minutes_between
>+
>+  $minutes = $calendar->open_minutes_between($start_date, $end_date)
>+
>+Returns the number of minutes between C<$start_date> and C<$end_date>, taking into
>+account the opening hours of the library.
>+
>+=cut
>+
>+sub open_minutes_between {
>+    my ($self, $start_date, $end_date) = @_;
>+    my $branchcode = $self->{branchcode};
>+    my $schema = Koha::Database->new->schema;
>+    my $dtf = DateTime::Format::Strptime->new(pattern => "%F %T");
>+    $start_date = $dtf->format_datetime($start_date);
>+    $end_date = $dtf->format_datetime($end_date);
>+    
>+    my $working_minutes_between = $schema->resultset('DiscreteCalendar')->search(
>+	    {
>+	        branchcode  => $branchcode,
>+	        is_opened   => 1,
>+        },
>+	    {
>+	        select  => \['sum(time_to_sec(timediff(close_hour, open_hour)) / 60)'],
>+	        as      => [qw /minutes_between/],
>+	        where   => \['date BETWEEN DATE(?) AND DATE(?)', $start_date, $end_date]
>+        }
>+    );
>+
>+    my $loan_day = $schema->resultset('DiscreteCalendar')->search(
>+	    {
>+	        branchcode  => $branchcode,
>+	    },
>+	    {
>+	        order_by => \[ 'ABS(DATEDIFF(date, ?))', $start_date ],
>+	        rows => 1,
>+	    }
>+    );
>+
>+    my $return_day = $schema->resultset('DiscreteCalendar')->search(
>+	    {
>+	        branchcode  => $branchcode,
>+	    },
>+	    {
>+	        order_by => \[ 'ABS(DATEDIFF(date, ?))', $end_date ],
>+	        rows => 1,
>+        }
>+    );
>+
>+    #Capture the time portion of the date
>+    $start_date =~ /\s(.*)/;
>+    my $loan_date_time = $1;
>+    $end_date =~ /\s(.*)/;
>+    my $return_date_time = $1;
>+
>+    my $not_used_minutes = $schema->resultset('DiscreteCalendar')->search(
>+	    {
>+	        branchcode  => $branchcode,
>+	        is_opened    => 1,
>+	    },
>+	    {
>+	        select  => \[ '(time_to_sec(timediff(?, ?)) + time_to_sec(timediff(?, ?)) ) / 60', $return_day->next()->close_hour(), $return_date_time, $loan_date_time, $loan_day->next()->open_hour()],
>+	        as      => [qw /not_used_minutes/],
>+	    }
>+    );
>+
>+    return ($working_minutes_between->next()->get_column('minutes_between') - $not_used_minutes->next()->get_column('not_used_minutes'));
>+}
>+
> =head2 addDuration
> 
>   my $dt = $calendar->addDuration($date, $dur, $unit)
>@@ -1272,6 +1341,8 @@ sub addDuration {
>         my $return_by_hour = 10;
> 
>         $dt = $self->addHours($startdate, $add_duration, $return_by_hour);
>+    } elsif($unit eq 'minutes') {
>+	    $dt = $self->addMinutes($startdate, $add_duration);
>     } else {
>         # days
>         $dt = $self->addDays($startdate, $add_duration);
>@@ -1314,6 +1385,31 @@ sub addHours {
>     return $base_date;
> }
> 
>+=head2 addMinutes
>+
>+  $end = $calendar->addMinutes($date, $dur)
>+
>+Add C<$dur> minutes to C<$start> date.
>+
>+=cut
>+
>+sub addMinutes {
>+    my ( $self, $startdate, $minutes_duration) = @_;
>+    my $base_date = $startdate->clone();
>+
>+    $base_date->add_duration($minutes_duration);
>+
>+    my $dateInfo = $self->get_date_info($base_date);
>+
>+    my $close_datetime = dt_from_string($dateInfo->{date} ." ". $dateInfo->{close_hour}, 'iso');
>+
>+    if(DateTime->compare($base_date, $close_datetime) > 0) {
>+	    return $close_datetime;
>+    }
>+
>+    return $base_date;
>+}
>+
> =head2 addDays
> 
>   $date = $calendar->addDays($start, $duration)
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index 0ee352fcb0..76325afa83 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -256,6 +256,8 @@
>                                             Days
>                                         [% ELSIF ( lengthunit == 'hours') %]
>                                             Hours
>+					[% ELSIF ( lengthunit == 'minutes') %]
>+					    Minutes
>                                         [% ELSE %]
>                                             Undefined
>                                         [% END %]
>@@ -432,6 +434,7 @@
>                       <select name="lengthunit" id="lengthunit">
>                         <option value="days" selected="selected">Days</option>
>                         <option value="hours">Hours</option>
>+			<option value="minutes">Minutes</option>
>                       </select>
>                     </td>
>                     <td>
>diff --git a/t/db_dependent/MinuteLoans.t b/t/db_dependent/MinuteLoans.t
>new file mode 100644
>index 0000000000..78257debc4
>--- /dev/null
>+++ b/t/db_dependent/MinuteLoans.t
>@@ -0,0 +1,283 @@
>+#!/usr/bin/env perl
>+
>+use strict;
>+use warnings;
>+
>+use DateTime;
>+use DateTime::Duration;
>+use Test::More tests => 10;
>+use Test::MockModule;
>+
>+use C4::Circulation;
>+use C4::Overdues;
>+use Koha::DateUtils qw( dt_from_string );
>+use Koha::DiscreteCalendar;
>+
>+use t::lib::Mocks;
>+use t::lib::TestBuilder;
>+
>+my $dbh = C4::Context->dbh();
>+my $today = DateTime->today;
>+my $schema = Koha::Database->new->schema;
>+$schema->storage->txn_begin;
>+
>+my $branch1 = 'Library1';
>+
>+# This test assumes branchcode 'UPL' exist inside discrete_calendar table of your database
>+# Should probably find a better alternative
>+$schema->resultset('DiscreteCalendar')->search({
>+    branchcode  => 'UPL'
>+})->update_all({
>+    is_opened    => 1,
>+    holiday_type => '',
>+    note         => '',
>+    open_hour    => '08:00:00',
>+    close_hour   => '16:00:00'
>+});
>+
>+Koha::DiscreteCalendar->add_new_branch('UPL', $branch1);
>+
>+my $calendar = Koha::DiscreteCalendar->new( { branchcode => $branch1 } );
>+
>+#######################################
>+# Add minutes and open minutes between
>+#######################################
>+
>+my $start_date = dt_from_string->set( hour => 14, minute => 15, second => 0);
>+my $end_date = dt_from_string->set( hour => 14, minute => 20, second => 0);
>+
>+is($calendar->open_minutes_between($start_date, $end_date), 5, "Item returned 5 minutes late");
>+
>+#Adding 10 minutes
>+my $ten_mins_duration = DateTime::Duration->new( minutes => 10);
>+is($calendar->addDuration($start_date, $ten_mins_duration, 'minutes' ),
>+    dt_from_string->set( hour => 14, minute => 25, second => 0),
>+        'Added 10 minutes to loan' );
>+
>+#Adding 10 minutes, passed closing hour
>+$start_date = dt_from_string->set( hour => 15, minute => 51, second => 0 );
>+is($calendar->addDuration($start_date, $ten_mins_duration, 'minutes' ),
>+    dt_from_string->set( hour => 16, minute => 0, second => 0),
>+        'Added 10 minutes, due date passed closing hours, set due date to same day at close hour' );
>+
>+#Item returned next open day after a holiday.
>+my $open_minutes_today = DateTime->today->add(hours =>10, minutes => 10);
>+my $open_minutes_tomorrow = DateTime->today->add(days =>1);
>+my $open_minutes_day_after_tomorrow = DateTime->today->add(days => 2, hours =>11);
>+
>+$calendar->edit_holiday({
>+    title => "Single holiday Today",
>+    holiday_type => $Koha::DiscreteCalendar::HOLIDAYS->{EXCEPTION},
>+    start_date => $open_minutes_tomorrow,
>+    end_date =>$open_minutes_tomorrow
>+});
>+
>+is($calendar->open_minutes_between($open_minutes_today, $open_minutes_day_after_tomorrow),
>+    530,
>+    "Correct open minutes, with a holiday in between");
>+
>+######################
>+# DueDate calculation
>+######################
>+
>+#Set syspref ReturnBeforeExpiry = 1 and useDaysMode = 'Days'
>+t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 1);
>+t::lib::Mocks::mock_preference('useDaysMode', 'Days');
>+
>+my $issuelength = 25;
>+my $renewalperiod = 5;
>+my $lengthunit = 'minutes';
>+my $dateexpiry = DateTime->today->add(years => 1);
>+
>+my $mock_loan_length = [
>+    ['issuelength', 'renewalperiod', 'lengthunit'],
>+    [$issuelength, $renewalperiod, $lengthunit]
>+];
>+
>+my $categorycode = 'X';
>+my $itemtype = 'MINUTES';
>+my $borrower = {categorycode => 'X', dateexpiry => $dateexpiry};
>+$dbh->do("INSERT INTO itemtypes (itemtype, parent_type) VALUES('$itemtype', NULL)");
>+$dbh->do("INSERT INTO categories (categorycode) VALUES('$categorycode')");
>+$dbh->do("INSERT INTO branches (branchcode, branchname) VALUES('$branch1', '$branch1')");
>+$dbh->do("INSERT INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES('$branch1', '$categorycode', '$itemtype', 'issuelength', '$issuelength')");
>+$dbh->do("INSERT INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES('$branch1', '$categorycode', '$itemtype', 'lengthunit', '$lengthunit')");
>+
>+
>+my $date = C4::Circulation::CalcDateDue( $today, $itemtype, $branch1, $borrower );
>+is($date, DateTime->today->add(minutes => $issuelength), "Due date calculated correctly");
>+
>+#passed closing hour
>+$issuelength = 1300;
>+$dbh->do("UPDATE circulation_rules SET rule_value=$issuelength where branchcode='$branch1' and itemtype='$itemtype' and rule_name='issuelength'");
>+
>+$date = C4::Circulation::CalcDateDue( $today, $itemtype, $branch1, $borrower );
>+is($date, DateTime->today->add(hours =>16), "Due date passed close hour, item due at close hour");
>+
>+#############################
>+# Chargeable minutes between
>+############################
>+
>+$issuelength = 25;
>+$dbh->do("UPDATE circulation_rules SET rule_value=$issuelength where branchcode='$branch1' and itemtype='$itemtype' and rule_name='issuelength'");
>+
>+my $date_due = DateTime->today;
>+my $date_returned = DateTime->today->add(minutes => 40);
>+
>+my $chargeable_units = C4::Overdues::get_chargeable_units('minutes', $date_due, $date_returned, $branch1);
>+is($chargeable_units, 40, "40 minutes of use");
>+
>+######################
>+# Fines calculation
>+#####################
>+
>+my $builder = t::lib::TestBuilder->new();
>+
>+my $category = $builder->build(
>+    {
>+        source => 'Category',
>+    }
>+);
>+
>+my $patron = $builder->build(
>+    {
>+        source => 'Borrower',
>+        value  => {
>+            categorycode => $category->{categorycode},
>+            branchcode   => $branch1,
>+        },
>+    }
>+);
>+
>+my $biblio = $builder->build(
>+    {
>+        source => 'Biblio',
>+        value  => {
>+            branchcode => $branch1,
>+        },
>+    }
>+);
>+
>+my $item = $builder->build(
>+    {
>+        source => 'Item',
>+        value  => {
>+            biblionumber     => $biblio->{biblionumber},
>+            homebranch       => $branch1,
>+            holdingbranch    => $branch1,
>+            replacementprice => '5.00',
>+        },
>+    }
>+);
>+my $rule = $builder->schema->resultset('CirculationRule')->search({
>+    branchcode                    => undef,
>+    categorycode                  => undef,
>+    itemtype                      => undef,
>+});
>+$rule->delete_all if $rule;
>+# FinesIncludeGracePeriod included
>+t::lib::Mocks::mock_preference('FinesIncludeGracePeriod', 1);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'fine',
>+            rule_value                    => '1',
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'lengthunit',
>+            rule_value                    => 'minutes',
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'finedays',
>+            rule_value                    => 0,
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'firstremind',
>+            rule_value                    => 5,
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'chargeperiod',
>+            rule_value                    => 1,
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'overduefinescap',
>+            rule_value                    => 9000,
>+        },
>+    }
>+);
>+$builder->build(
>+    {
>+        source => 'CirculationRule',
>+        value  => {
>+            branchcode                    => undef,
>+            categorycode                  => undef,
>+            itemtype                      => undef,
>+            rule_name                     => 'cap_fine_to_replacement_price',
>+            rule_value                    => 0,
>+        },
>+    }
>+);
>+
>+my $start_dt = dt_from_string->set( hour => 15, minute => 0, second => 0);
>+my $end_dt = dt_from_string->set( hour => 15, minute => 4, second => 0);
>+
>+my @amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch1, $start_dt, $end_dt );
>+is($amount[0], 0, "No fine when under the fine grace period");
>+
>+$end_dt = dt_from_string->set( hour => 15, minute => 6, second => 0);
>+@amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch1, $start_dt, $end_dt );
>+is($amount[0], 6, "6\$ fine for 6 minutes delay, fine grace period included");
>+
>+# FinesIncludeGracePeriod not included
>+t::lib::Mocks::mock_preference('FinesIncludeGracePeriod', 0);
>+
>+@amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch1, $start_dt, $end_dt );
>+is($amount[0], 1, "1\$ fine for 6 minutes delay, fine grace period not included");
>+
>+$schema->storage->txn_rollback;
>+
>+1;
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 17983: 59521 | 59522 | 59523 | 67880 | 67881 | 67882 | 67969 | 68016 | 68017 | 73353 | 73354 | 73355 | 75480 | 131929 | 131930 | 131931 | 131932 | 136414 | 136415 | 139519 | 144267 | 151452 | 167846