From 87d579fc6061c724dc5f20fe10839cbccb10b404 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Wed, 17 Jun 2020 11:14:30 +0100
Subject: [PATCH] Bug 25723: [ALTERNATIVE] Use the same code for single and
 exception holidays

This patch refactors is_exception_holiday and is_single_holiday to
utilise a single _holidays method which combines the logic of the
previous single_holidays and exception_holidays methods.

As Koha::Calendar is instantiated at a branch level, we also move split
the cache into branches too.
---
 Koha/Calendar.pm | 116 ++++++++++++++---------------------------------
 1 file changed, 33 insertions(+), 83 deletions(-)

diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm
index f15f804e8c..fffc854afc 100644
--- a/Koha/Calendar.pm
+++ b/Koha/Calendar.pm
@@ -51,94 +51,59 @@ sub _init {
     return;
 }
 
-sub exception_holidays {
+sub _holidays {
     my ($self) = @_;
 
-    my $cache              = Koha::Caches->get_instance();
-    my $exception_holidays = $cache->get_from_cache('exception_holidays');
+    my $key      = $self->{branchcode} . "_holidays";
+    my $cache    = Koha::Caches->get_instance();
+    my $holidays = $cache->get_from_cache($key);
 
-    # Populate the cache is necessary
-    unless ($exception_holidays) {
-        my $dbh = C4::Context->dbh;
-        $exception_holidays = {};
-
-        # Push holidays for each branch
-        my $exception_holidays_sth = $dbh->prepare(
-'SELECT day, month, year, branchcode FROM special_holidays WHERE isexception = 1'
-        );
-        $exception_holidays_sth->execute();
-        my $dates = [];
-        while ( my ( $day, $month, $year, $branch ) =
-            $exception_holidays_sth->fetchrow )
-        {
-            my $datestring =
-                sprintf( "%04d", $year )
-              . sprintf( "%02d", $month )
-              . sprintf( "%02d", $day );
-
-            $exception_holidays->{$branch}->{$datestring} = 1;
-        }
-        $cache->set_in_cache( 'exception_holidays', $exception_holidays,
-            { expiry => 76800 } );
-    }
-
-    return $exception_holidays->{ $self->{branchcode} } // {};
-}
-
-sub is_exception_holiday {
-    my ( $self, $date ) = @_;
-
-    return 1 if ( $self->exception_holidays->{$date} );
-    return 0;
-}
-
-sub single_holidays {
-    my ($self) = @_;
-
-    my $cache           = Koha::Caches->get_instance();
-    my $single_holidays = $cache->get_from_cache('single_holidays');
-
-    # $single_holidays looks like:
+    # $holidays looks like:
     # {
-    #   CPL =>  [
-    #        [0] 20131122,
-    #         ...
-    #    ],
-    #   ...
+    #    20131122 => 1, # single_holiday
+    #    20131123 => 0, # exception_holiday
+    #    ...
     # }
 
     # Populate the cache if necessary
-    unless ($single_holidays) {
+    unless ($holidays) {
         my $dbh = C4::Context->dbh;
-        $single_holidays = {};
+        $holidays = {};
 
-        # Push holidays for each branch
-        my $single_holidays_sth = $dbh->prepare(
-'SELECT day, month, year, branchcode FROM special_holidays WHERE isexception = 0'
+        # Add holidays for each branch
+        my $holidays_sth = $dbh->prepare(
+'SELECT day, month, year, isexception FROM special_holidays WHERE branchcode = ?'
         );
-        $single_holidays_sth->execute();
+        $holidays_sth->execute($self->{branchcode});
 
-        while ( my ( $day, $month, $year, $branch ) =
-            $single_holidays_sth->fetchrow )
+        while ( my ( $day, $month, $year, $exception ) =
+            $holidays_sth->fetchrow )
         {
             my $datestring =
                 sprintf( "%04d", $year )
               . sprintf( "%02d", $month )
               . sprintf( "%02d", $day );
 
-            $single_holidays->{$branch}->{$datestring} = 1;
+            $holidays->{$datestring} = !$exception;
         }
-        $cache->set_in_cache( 'single_holidays', $single_holidays,
-            { expiry => 76800 } );
+        $cache->set_in_cache( $key, $holidays, { expiry => 76800 } );
     }
 
-    return $single_holidays->{ $self->{branchcode} } // {};
+    return $holidays // {};
 }
 
 sub is_single_holiday {
     my ( $self, $date ) = @_;
 
-    return 1 if ( $self->single_holidays->{$date} );
+    return 1 if ( $self->_holidays->{$date} );
+    return 0;
+}
+
+sub is_exception_holiday {
+    my ( $self, $date ) = @_;
+
+    return 1
+      if ( defined( $self->_holidays->{$date} ) && !$self->_holidays->{$date} );
     return 0;
 }
 
@@ -258,17 +223,13 @@ sub is_holiday {
     my $localdt = $dt->clone();
     my $day   = $localdt->day;
     my $month = $localdt->month;
-    my $ymd   = $localdt->ymd('')  ;
+    my $ymd   = $localdt->ymd('');
 
     #Change timezone to "floating" before doing any calculations or comparisons
     $localdt->set_time_zone("floating");
     $localdt->truncate( to => 'day' );
 
-
-    if ( $self->is_exception_holiday( $ymd ) == 1 ) {
-        # exceptions are not holidays
-        return 0;
-    }
+    return $self->_holidays->{$ymd} if defined($self->_holidays->{$ymd});
 
     my $dow = $localdt->day_of_week;
     # Representation fix
@@ -286,10 +247,6 @@ sub is_holiday {
         return 1;
     }
 
-    if ($self->is_single_holiday(  $ymd  ) == 1 ) {
-        return 1;
-    }
-
     # damn have to go to work after all
     return 0;
 }
@@ -485,18 +442,11 @@ C<$unit> is a string value 'days' or 'hours' toflag granularity of duration
 Currently unit is only used to invoke Staffs return Monday at 10 am rule this
 parameter will be removed when issuingrules properly cope with that
 
+=head2 is_single_holiday
 
-=head2 single_holidays
-
-my $rc = $self->single_holidays(  $ymd  );
-
-Passed a $date in Ymd (yyyymmdd) format -  returns 1 if date is a single_holiday, or 0 if not.
-
-=head2 exception_holidays
-
-my $exceptions = $self->exception_holidays;
+my $rc = $self->is_single_holiday( $ymd );
 
-Returns a hashref of exception holidays for the branch
+Passed a $date in Ymd (yyyymmdd) format - returns 1 if the date is a single_holiday, or 0 if not.
 
 =head2 is_exception_holiday
 
-- 
2.20.1