From 1a9545ab2cb7127f690a8a3f10cfd687b7f14a05 Mon Sep 17 00:00:00 2001 From: Emmi Takkinen Date: Wed, 4 Mar 2020 13:54:34 +0200 Subject: [PATCH] Bug 19475: Don't copy holiday if it already exists in target calendar Calendars copy tool created duplicate values to database. Holidays in target calendar weren't checked before inserting new holidays. This patch fixes this. To test: 1. Add holidays for branch A 2. Copy branch A calendar to branch B 3. Repeat copy to branch B => Check database, branch B has now duplicate holidays 4. Delete holidays from branches A and B 5. Apply patch 6. Repeat steps 1-3 => Check database, no duplicates Sponsored-by: Koha-Suomi Oy Signed-off-by: Michal Denar Signed-off-by: Fridolin Somers Signed-off-by: Jonathan Druart Amended patch: Remove trailing whitespaces and add missing semicolon --- C4/Calendar.pm | 38 ++++++++++++++++++++++++++++++-------- t/db_dependent/Holidays.t | 28 ++++++++++++++-------------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/C4/Calendar.pm b/C4/Calendar.pm index 321aad76d5..76c498ffaa 100644 --- a/C4/Calendar.pm +++ b/C4/Calendar.pm @@ -690,14 +690,36 @@ sub copy_to_branch { my $today = sprintf ISO_DATE_FORMAT, $y,$m,$d; my $wdh = $self->get_week_days_holidays; - $target_calendar->insert_week_day_holiday( weekday => $_, %{ $wdh->{$_} } ) - foreach keys %$wdh; - $target_calendar->insert_day_month_holiday(%$_) - foreach values %{ $self->get_day_month_holidays }; - $target_calendar->insert_exception_holiday(%$_) - foreach grep { $_->{date} gt $today } values %{ $self->get_exception_holidays }; - $target_calendar->insert_single_holiday(%$_) - foreach grep { $_->{date} gt $today } values %{ $self->get_single_holidays }; + my $target_wdh = $target_calendar->get_week_days_holidays; + foreach my $key (keys %$wdh) { + unless (grep { $_ eq $key } keys %$target_wdh) { + $target_calendar->insert_week_day_holiday( weekday => $key, %{ $wdh->{$key} } ) + } + } + + my $dmh = $self->get_day_month_holidays; + my $target_dmh = $target_calendar->get_day_month_holidays; + foreach my $values (values %$dmh) { + unless (grep { $_->{day} eq $values->{day} && $_->{month} eq $values->{month} } values %$target_dmh) { + $target_calendar->insert_day_month_holiday(%{ $values }); + } + } + + my $exception_holidays = $self->get_exception_holidays; + my $target_exceptions = $target_calendar->get_exception_holidays; + foreach my $values ( grep {$_->{date} gt $today} values %{ $exception_holidays }) { + unless ( grep { $_->{date} eq $values->{date} } values %$target_exceptions) { + $target_calendar->insert_exception_holiday(%{ $values }); + } + } + + my $single_holidays = $self->get_single_holidays; + my $target_singles = $target_calendar->get_single_holidays; + foreach my $values ( grep {$_->{date} gt $today} values %{ $single_holidays }) { + unless ( grep { $_->{date} eq $values->{date} } values %$target_singles){ + $target_calendar->insert_single_holiday(%{ $values }); + } + } return 1; } diff --git a/t/db_dependent/Holidays.t b/t/db_dependent/Holidays.t index ee737e6778..036487079e 100755 --- a/t/db_dependent/Holidays.t +++ b/t/db_dependent/Holidays.t @@ -204,7 +204,7 @@ sub _add_exception { $schema->storage->txn_rollback; subtest 'copy_to_branch' => sub { - + plan tests => 8; $schema->storage->txn_begin; @@ -281,12 +281,12 @@ subtest 'copy_to_branch' => sub { #Select all rows with same values from database my $dbh = C4::Context->dbh; - my $get_repeatable_holidays = "SELECT a.* FROM repeatable_holidays a - JOIN (SELECT branchcode, weekday, day, month, COUNT(*) - FROM repeatable_holidays - GROUP BY branchcode, weekday, day, month HAVING count(*) > 1) b - ON a.branchcode = b.branchcode - AND ( a.weekday = b.weekday OR (a.day = b.day AND a.month = b.month)) + my $get_repeatable_holidays = "SELECT a.* FROM repeatable_holidays a + JOIN (SELECT branchcode, weekday, day, month, COUNT(*) + FROM repeatable_holidays + GROUP BY branchcode, weekday, day, month HAVING count(*) > 1) b + ON a.branchcode = b.branchcode + AND ( a.weekday = b.weekday OR (a.day = b.day AND a.month = b.month)) ORDER BY a.branchcode;"; my $sth = $dbh->prepare($get_repeatable_holidays); $sth->execute; @@ -295,14 +295,14 @@ subtest 'copy_to_branch' => sub { while(my $row = $sth->fetchrow_hashref){ push @repeatable_holidays, $row } - + is( scalar(@repeatable_holidays), 0, "None of the repeatable holidays were doubled"); - my $get_special_holidays = "SELECT a.* FROM special_holidays a - JOIN (SELECT branchcode, day, month, year, isexception, COUNT(*) - FROM special_holidays - GROUP BY branchcode, day, month, year, isexception HAVING count(*) > 1) b - ON a.branchcode = b.branchcode + my $get_special_holidays = "SELECT a.* FROM special_holidays a + JOIN (SELECT branchcode, day, month, year, isexception, COUNT(*) + FROM special_holidays + GROUP BY branchcode, day, month, year, isexception HAVING count(*) > 1) b + ON a.branchcode = b.branchcode AND a.day = b.day AND a.month = b.month AND a.year = b.year AND a.isexception = b.isexception ORDER BY a.branchcode;"; $sth = $dbh->prepare($get_special_holidays); @@ -317,4 +317,4 @@ subtest 'copy_to_branch' => sub { $schema->storage->txn_rollback; -} +}; -- 2.20.1