From cbda5fab732d2fba459b4aca2d38acd8e56cc1e2 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Fri, 17 Jul 2020 12:34:18 +0000
Subject: [PATCH] Bug 25723: (QA follow-up) Handle holiday and exception on
 same day

When a holiday is entered, then exceptions generated on a range, there exists both a holiday and exception in
the special holidays table. We should cache the exception over the holiday instead of both

Also, !1 in perl returns '' rather than 0, so we should explicitly set the value

Add blank line to clear pod error from qa tools

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Calendar.pm |  5 ++---
 t/Calendar.t     | 11 ++++++++++-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm
index c0dcd8e93d..c3210d469d 100644
--- a/Koha/Calendar.pm
+++ b/Koha/Calendar.pm
@@ -71,7 +71,7 @@ sub _holidays {
 
         # Add holidays for each branch
         my $holidays_sth = $dbh->prepare(
-'SELECT day, month, year, isexception FROM special_holidays WHERE branchcode = ?'
+'SELECT day, month, year, MAX(isexception) FROM special_holidays WHERE branchcode = ? GROUP BY day, month, year'
         );
         $holidays_sth->execute($self->{branchcode});
 
@@ -83,11 +83,10 @@ sub _holidays {
               . sprintf( "%02d", $month )
               . sprintf( "%02d", $day );
 
-            $holidays->{$datestring} = !$exception;
+            $holidays->{$datestring} = $exception ? 0 : 1;
         }
         $cache->set_in_cache( $key, $holidays, { expiry => 76800 } );
     }
-
     return $holidays // {};
 }
 
diff --git a/t/Calendar.t b/t/Calendar.t
index 8657bc4999..b4b94bf9c3 100755
--- a/t/Calendar.t
+++ b/t/Calendar.t
@@ -31,7 +31,7 @@ use Module::Load::Conditional qw/check_install/;
 
 BEGIN {
     if ( check_install( module => 'Test::DBIx::Class' ) ) {
-        plan tests => 39;
+        plan tests => 40;
     } else {
         plan skip_all => "Need Test::DBIx::Class"
     }
@@ -73,6 +73,8 @@ fixtures_ok [
         [ 'MPL', 1,  6,  2011, '', '', 0 ],
         [ 'MPL', 4,  7,  2012, '', '', 0 ],
         [ 'CPL', 6,  8,  2012, '', '', 0 ],
+        [ 'MPL', 7,  7,  2012, '', '', 1 ], # holiday exception
+        [ 'MPL', 7,  7,  2012, '', '', 0 ], # holiday
       ],
 ], "add fixtures";
 
@@ -139,6 +141,12 @@ my $holiday_for_another_branch = DateTime->new(
     day => 6, # This is a monday
 );
 
+my $holiday_excepted = DateTime->new(
+    year => 2012,
+    month => 7,
+    day => 7, # Both a holiday and exception
+);
+
 {   # Syspref-agnostic tests
     is ( $saturday->day_of_week, 6, '\'$saturday\' is actually a saturday (6th day of week)');
     is ( $sunday->day_of_week, 7, '\'$sunday\' is actually a sunday (7th day of week)');
@@ -151,6 +159,7 @@ my $holiday_for_another_branch = DateTime->new(
     is ( $cal->is_holiday($notspecial), 0, 'Fixed single date that is not a holiday test' );
     is ( $cal->is_holiday($sunday_exception), 0, 'Exception holiday is not a closed day test' );
     is ( $cal->is_holiday($holiday_for_another_branch), 0, 'Holiday defined for another branch should not be defined as an holiday' );
+    is ( $cal->is_holiday($holiday_excepted), 0, 'Holiday defined and excepted should not be a holiday' );
 }
 
 {   # Bugzilla #8966 - is_holiday truncates referenced date
-- 
2.20.1