Fixes #6398 - Recurring all-day iCal holidays are stored one day early on hosts east of UTC.

Approved-by: Martin Gruner <mg@zammad.com>

Co-authored-by: Mantas <mantas@idev.lt>

app/models/calendar.rb

@@ -237,12 +237,12 @@ returns
       if event.rrule.present?
 
         # loop till days
-        interval_frame_start = Date.parse("#{1.year.ago}-01-01")
-        interval_frame_end   = Date.parse("#{3.years.from_now}-12-31")
+        interval_frame_start = 1.year.ago.beginning_of_year.to_date
+        interval_frame_end   = 3.years.from_now.end_of_year.to_date
         occurrences          = event.occurrences_between(interval_frame_start, interval_frame_end)
         if occurrences.present?
           occurrences.each do |occurrence|
-            result = Calendar.day_and_comment_by_event(event, occurrence.start_time)
+            result = Calendar.day_and_comment_by_event(event, Calendar.occurrence_start_time(event, occurrence))
             next if !result
 
             events[result[0]] = result[1]
@@ -260,6 +260,17 @@ returns
     events.sort.to_h
   end
 
+  # The recurrence gem starts all-day events at local midnight and returns
+  # occurrences in UTC, which lands them on the previous day east of UTC.
+  def self.occurrence_start_time(event, occurrence)
+    start_time = occurrence.start_time
+    return start_time.getlocal if event.dtstart.is_a?(Icalendar::Values::Date)
+
+    # A TZID known only from a VTIMEZONE block resolves to a fixed offset without a zone.
+    zone = event.dtstart.value.try(:time_zone)
+    zone ? start_time.in_time_zone(zone) : start_time.getlocal(event.dtstart.value.utc_offset)
+  end
+
   # get day and comment by event
   def self.day_and_comment_by_event(event, start_time)
     day = "#{start_time.year}-#{format('%<month>02d', month: start_time.month)}-#{format('%<day>02d', day: start_time.day)}"

db/migrate/20260925120000_calendar_public_holiday_resync.rb

@@ -0,0 +1,74 @@
+# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
+
+class CalendarPublicHolidayResync < ActiveRecord::Migration[8.1]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    Calendar.find_each do |calendar|
+      next if calendar.ical_url.blank?
+
+      resync(calendar)
+    end
+  end
+
+  private
+
+  def resync(calendar)
+    feed = Digest::MD5.hexdigest(calendar.ical_url)
+    return if calendar.public_holidays.none? { |_, meta| meta['feed'] == feed }
+
+    events = fetch(calendar)
+    return if events.nil?
+    return if !move_misplaced_entries(calendar, events, feed)
+
+    add_missing_entries(calendar, events, feed)
+    return if calendar.save
+
+    Rails.logger.error "Calendar #{calendar.id}: public holidays left unchanged, calendar is invalid (#{calendar.errors.full_messages.join(', ')})"
+  end
+
+  def fetch(calendar)
+    Calendar.fetch_parse(calendar.ical_url)
+  rescue => e
+    Rails.logger.error "Calendar #{calendar.id}: public holidays left unchanged, feed could not be fetched (#{e.message})"
+    nil
+  end
+
+  def move_misplaced_entries(calendar, events, feed)
+    misplaced = calendar.public_holidays.select { |day, meta| meta['feed'] == feed && events[day] != meta['summary'] }
+
+    moves = misplaced.filter_map do |day, meta|
+      corrected_day = corrected_day(day, meta['summary'], events)
+      [day, corrected_day, meta] if corrected_day
+    end
+
+    # Consecutive holidays move onto each other's day, so clear all of them before filling.
+    calendar.public_holidays.except!(*moves.map(&:first))
+    moves.each { |_, corrected_day, meta| calendar.public_holidays[corrected_day] ||= meta }
+
+    moves.any?
+  end
+
+  # Recurring feed events were imported on the UTC day of their occurrence: all-day
+  #   events one day early on hosts east of UTC, timed events one day late in zones west of it.
+  def corrected_day(day, summary, events)
+    return next_day(day) if events[next_day(day)] == summary
+
+    prev_day(day) if events[prev_day(day)] == summary
+  end
+
+  def next_day(day)
+    Date.parse(day).next_day.iso8601
+  end
+
+  def prev_day(day)
+    Date.parse(day).prev_day.iso8601
+  end
+
+  def add_missing_entries(calendar, events, feed)
+    events.each do |day, summary|
+      calendar.public_holidays[day] ||= { 'active' => true, 'summary' => summary, 'feed' => feed }
+    end
+  end
+end

spec/db/migrate/calendar_public_holiday_resync_spec.rb

@@ -0,0 +1,209 @@
+# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe CalendarPublicHolidayResync, type: :db_migration do
+  let(:ical_file) { 'calendar1.ics' }
+  let(:ical_url)  { "http://test/#{ical_file}" }
+  let(:feed)      { Digest::MD5.hexdigest(ical_url) }
+
+  let(:custom_holiday) do
+    { '2017-02-03' => { 'active' => true, 'summary' => 'Company day' } }
+  end
+
+  let(:outdated_feed_holiday) do
+    { '2014-12-23' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed } }
+  end
+
+  let(:day_early_holidays) do
+    {
+      '2016-12-23' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+      '2017-12-23' => { 'active' => false, 'summary' => 'Christmas1', 'feed' => feed },
+    }
+  end
+
+  let(:corrected_holidays) do
+    {
+      '2016-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+      '2017-12-24' => { 'active' => false, 'summary' => 'Christmas1', 'feed' => feed },
+    }
+  end
+
+  let(:remaining_feed_holidays) do
+    {
+      '2018-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+      '2019-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+      '2020-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+    }
+  end
+
+  before do
+    allow(HostnameSafetyCheck).to receive(:validate!).and_return('1.2.3.4')
+    travel_to Time.zone.parse('2017-08-24T01:04:44Z0')
+  end
+
+  context 'with a calendar synced from a feed' do
+    let(:calendar) { create(:calendar, ical_url: ical_url) }
+
+    before do
+      stub_request(:get, ical_url)
+        .to_return(body: Rails.root.join('test/data/calendar', ical_file).read)
+
+      calendar.update!(public_holidays: stored_holidays)
+    end
+
+    context 'when feed entries were stored one day early' do
+      let(:stored_holidays) { day_early_holidays.merge(custom_holiday).merge(outdated_feed_holiday) }
+
+      it 'moves them to their day, keeps their flags, adds the missing feed days and leaves other entries alone' do
+        expect { migrate }
+          .to change { calendar.reload.public_holidays }
+          .from(stored_holidays)
+          .to(corrected_holidays.merge(remaining_feed_holidays).merge(custom_holiday).merge(outdated_feed_holiday))
+      end
+
+      it 'fetches the feed only once' do
+        allow(Calendar).to receive(:fetch_parse).and_call_original
+
+        migrate
+
+        expect(Calendar).to have_received(:fetch_parse).once
+      end
+
+      context 'when the feed cannot be fetched' do
+        before do
+          stub_request(:get, ical_url).to_return(status: 404)
+        end
+
+        it 'keeps the existing entries' do
+          expect { migrate }.not_to change { calendar.reload.public_holidays }
+        end
+      end
+
+      context 'when the calendar no longer passes validation' do
+        before do
+          calendar.update_columns(business_hours: {})
+          allow(Rails.logger).to receive(:error)
+        end
+
+        it 'keeps the existing entries' do
+          expect { migrate }.not_to change { calendar.reload.public_holidays }
+        end
+
+        it 'logs the reason' do
+          migrate
+
+          expect(Rails.logger).to have_received(:error).with(%r{Calendar #{calendar.id}: .*There are no business hours configured})
+        end
+      end
+    end
+
+    context 'when feed entries on consecutive days were stored one day early' do
+      let(:ical_file) { 'calendar2.ics' }
+
+      let(:stored_holidays) do
+        {
+          '2016-12-23' => { 'active' => false, 'summary' => 'Christmas1', 'feed' => feed },
+          '2016-12-24' => { 'active' => true, 'summary' => 'Christmas2', 'feed' => feed },
+        }
+      end
+
+      it 'moves both entries to their day and keeps their flags' do
+        expect { migrate }
+          .to change { calendar.reload.public_holidays.slice('2016-12-23', '2016-12-24', '2016-12-25') }
+          .to(
+            '2016-12-24' => { 'active' => false, 'summary' => 'Christmas1', 'feed' => feed },
+            '2016-12-25' => { 'active' => true, 'summary' => 'Christmas2', 'feed' => feed },
+          )
+      end
+    end
+
+    context 'when the corrected day of a day-early feed entry holds a custom entry' do
+      let(:stored_holidays) do
+        day_early_holidays.merge('2017-12-24' => { 'active' => true, 'summary' => 'Christmas party' })
+      end
+
+      before do
+        # Saving re-attaches the feed digest to a day the initial sync filled, which a custom entry never carried.
+        calendar.update_columns(public_holidays: stored_holidays)
+      end
+
+      it 'drops the misplaced entry and keeps the custom one' do
+        expect { migrate }
+          .to change { calendar.reload.public_holidays.slice('2017-12-23', '2017-12-24') }
+          .to('2017-12-24' => { 'active' => true, 'summary' => 'Christmas party' })
+      end
+    end
+
+    context 'when a feed entry with a summary the feed does not yield sits one day before an empty feed day' do
+      let(:unmatched_holiday) do
+        { '2018-12-23' => { 'active' => false, 'summary' => 'Heiligabend', 'feed' => feed } }
+      end
+
+      context 'with other entries of the feed stored one day early' do
+        let(:stored_holidays) { day_early_holidays.merge(unmatched_holiday) }
+
+        it 'leaves it where it is and adds the feed day next to it' do
+          expect { migrate }
+            .to change { calendar.reload.public_holidays.slice('2018-12-23', '2018-12-24') }
+            .from(unmatched_holiday)
+            .to(unmatched_holiday.merge('2018-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed }))
+        end
+      end
+
+      context 'without other entries of the feed stored one day early' do
+        let(:stored_holidays) { unmatched_holiday }
+
+        it 'does not touch the calendar' do
+          expect { migrate }.not_to change { calendar.reload.attributes }
+        end
+      end
+    end
+
+    context 'when a timed feed entry in a zone west of UTC was stored one day late' do
+      let(:ical_file) { 'calendar4.ics' }
+
+      let(:stored_holidays) do
+        { '2017-01-20' => { 'active' => false, 'summary' => 'evening shift', 'feed' => feed } }
+      end
+
+      it 'moves it to its day and keeps its flag' do
+        expect { migrate }
+          .to change { calendar.reload.public_holidays.slice('2017-01-19', '2017-01-20') }
+          .to('2017-01-19' => { 'active' => false, 'summary' => 'evening shift', 'feed' => feed })
+      end
+    end
+
+    context 'when feed entries are stored on their day' do
+      let(:stored_holidays) { corrected_holidays.merge(remaining_feed_holidays).merge(custom_holiday) }
+
+      it 'does not touch the calendar' do
+        expect { migrate }.not_to change { calendar.reload.attributes }
+      end
+    end
+
+    context 'when no entry comes from the feed' do
+      let(:stored_holidays) { custom_holiday }
+
+      it 'does not fetch the feed' do
+        allow(Calendar).to receive(:fetch_parse).and_call_original
+
+        migrate
+
+        expect(Calendar).not_to have_received(:fetch_parse)
+      end
+
+      it 'does not touch the calendar' do
+        expect { migrate }.not_to change { calendar.reload.attributes }
+      end
+    end
+  end
+
+  context 'with a calendar without a feed' do
+    let(:calendar) { create(:calendar, public_holidays: custom_holiday) }
+
+    it 'does not touch the calendar' do
+      expect { migrate }.not_to change { calendar.reload.attributes }
+    end
+  end
+end

spec/models/calendar_spec.rb

@@ -3,8 +3,6 @@
 require 'rails_helper'
 require 'models/concerns/has_audit_logs_examples'
 
-# Without `export TZ="Europe/London"` in the environment, the tests will fail. :(
-
 RSpec.describe Calendar, type: :model do
   subject(:calendar) { create(:calendar) }
 
@@ -77,6 +75,8 @@ RSpec.describe Calendar, type: :model do
           .to_return(body: Rails.root.join('test/data/calendar/calendar1.ics').read)
         stub_request(:get, 'http://test/calendar3.ics')
           .to_return(body: Rails.root.join('test/data/calendar/calendar3.ics').read)
+        stub_request(:get, 'http://test/calendar4.ics')
+          .to_return(body: Rails.root.join('test/data/calendar/calendar4.ics').read)
         travel_to Time.zone.parse('2017-08-24T01:04:44Z0')
       end
 
@@ -87,9 +87,16 @@ RSpec.describe Calendar, type: :model do
             '2017-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
             '2018-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
             '2019-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+            '2020-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
           )
         end
 
+        context 'when the host time zone is east of UTC', time_zone: 'Europe/Berlin' do
+          it 'keeps recurring all-day events on their day' do
+            expect(calendar.public_holidays.keys).to eq(%w[2016-12-24 2017-12-24 2018-12-24 2019-12-24 2020-12-24])
+          end
+        end
+
         context 'with one-time and n-time (recurring) events' do
           subject(:calendar) do
             create(:calendar, ical_url: 'http://test/calendar3.ics')
@@ -103,10 +110,30 @@ RSpec.describe Calendar, type: :model do
               '2017-01-26' => { 'active' => true, 'summary' => 'day3', 'feed' => feed },
               '2017-02-26' => { 'active' => true, 'summary' => 'day3', 'feed' => feed },
               '2017-03-26' => { 'active' => true, 'summary' => 'day3', 'feed' => feed },
-              '2017-04-25' => { 'active' => true, 'summary' => 'day3', 'feed' => feed },
+              '2017-04-26' => { 'active' => true, 'summary' => 'day3', 'feed' => feed },
               '2017-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
               '2018-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
               '2019-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+              '2020-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+            )
+          end
+        end
+
+        context 'with timed recurring events' do
+          subject(:calendar) do
+            create(:calendar, ical_url: 'http://test/calendar4.ics')
+          end
+
+          it 'stores each occurrence on its day in the event time zone' do
+            expect(calendar.public_holidays).to eq(
+              '2017-01-05' => { 'active' => true, 'summary' => 'night shift', 'feed' => feed },
+              '2017-01-12' => { 'active' => true, 'summary' => 'custom zone shift', 'feed' => feed },
+              '2017-01-19' => { 'active' => true, 'summary' => 'evening shift', 'feed' => feed },
+              '2017-02-05' => { 'active' => true, 'summary' => 'night shift', 'feed' => feed },
+              '2017-02-12' => { 'active' => true, 'summary' => 'custom zone shift', 'feed' => feed },
+              '2017-02-19' => { 'active' => true, 'summary' => 'evening shift', 'feed' => feed },
+              '2017-03-05' => { 'active' => true, 'summary' => 'night shift', 'feed' => feed },
+              '2017-04-05' => { 'active' => true, 'summary' => 'night shift', 'feed' => feed },
             )
           end
         end
@@ -181,6 +208,7 @@ RSpec.describe Calendar, type: :model do
             '2018-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
             '2019-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
             '2020-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+            '2021-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
           )
         end
 
@@ -203,6 +231,8 @@ RSpec.describe Calendar, type: :model do
               '2018-12-25' => { 'active' => true, 'summary' => 'Christmas2', 'feed' => feed },
               '2019-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
               '2019-12-25' => { 'active' => true, 'summary' => 'Christmas2', 'feed' => feed },
+              '2020-12-24' => { 'active' => true, 'summary' => 'Christmas1', 'feed' => feed },
+              '2020-12-25' => { 'active' => true, 'summary' => 'Christmas2', 'feed' => feed },
             )
         end
       end

spec/support/time_zone.rb

@@ -2,16 +2,14 @@
 
 RSpec.configure do |config|
   config.around(:each, :time_zone) do |example|
-    # RSpec System/Capybara tests use TZ environment variable to set timezone in the browser.
-    if example.metadata[:type] == :system
-      old_tz = ENV['TZ']
-      ENV['TZ'] = example.metadata[:time_zone]
-    end
+    old_tz = ENV['TZ']
+
+    # Rails' Time.zone does not reach Ruby's own local Time, gems built on it,
+    #   or the browser in system specs, which all follow TZ instead.
+    ENV['TZ'] = example.metadata[:time_zone]
 
-    # Other RSpec tests run inside the same process and don't take TZ variable into account.
-    #   However, they should still have mocking of the test time zone for the Time object applied.
     Time.use_zone(example.metadata[:time_zone]) { example.run }
   ensure
-    ENV['TZ'] = old_tz if example.metadata[:type] == :system
+    ENV['TZ'] = old_tz
   end
 end

test/data/calendar/calendar4.ics

@@ -0,0 +1,86 @@
+BEGIN:VCALENDAR
+METHOD:PUBLISH
+VERSION:2.0
+X-WR-CALNAME:test4
+PRODID:-//Apple Inc.//Mac OS X 10.12.6//EN
+X-WR-TIMEZONE:Europe/Berlin
+CALSCALE:GREGORIAN
+BEGIN:VTIMEZONE
+TZID:Europe/Berlin
+BEGIN:DAYLIGHT
+TZOFFSETFROM:+0100
+RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
+DTSTART:19810329T020000
+TZNAME:GMT+2
+TZOFFSETTO:+0200
+END:DAYLIGHT
+BEGIN:STANDARD
+TZOFFSETFROM:+0200
+RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU
+DTSTART:19961027T030000
+TZNAME:GMT+1
+TZOFFSETTO:+0100
+END:STANDARD
+END:VTIMEZONE
+BEGIN:VTIMEZONE
+TZID:America/New_York
+BEGIN:DAYLIGHT
+TZOFFSETFROM:-0500
+RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
+DTSTART:20070311T020000
+TZNAME:EDT
+TZOFFSETTO:-0400
+END:DAYLIGHT
+BEGIN:STANDARD
+TZOFFSETFROM:-0400
+RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
+DTSTART:20071104T020000
+TZNAME:EST
+TZOFFSETTO:-0500
+END:STANDARD
+END:VTIMEZONE
+BEGIN:VTIMEZONE
+TZID:Customized Time Zone
+BEGIN:STANDARD
+TZOFFSETFROM:+0100
+TZOFFSETTO:+0100
+DTSTART:16010101T000000
+END:STANDARD
+END:VTIMEZONE
+BEGIN:VEVENT
+CREATED:20170824T123409Z
+UID:0B8A5C1E-3F7D-4A62-9E1B-5C2D8F4A7E10
+RRULE:FREQ=MONTHLY;INTERVAL=1;COUNT=4
+DTEND;TZID=Europe/Berlin:20170105T013000
+TRANSP:OPAQUE
+X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
+SUMMARY:night shift
+DTSTART;TZID=Europe/Berlin:20170105T003000
+DTSTAMP:20170824T134117Z
+SEQUENCE:0
+END:VEVENT
+BEGIN:VEVENT
+CREATED:20170824T123419Z
+UID:7D3E9B24-6A15-4C8F-B0D7-1E4F6A9C2B58
+RRULE:FREQ=MONTHLY;INTERVAL=1;COUNT=2
+DTEND;TZID=Customized Time Zone:20170112T013000
+TRANSP:OPAQUE
+X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
+SUMMARY:custom zone shift
+DTSTART;TZID=Customized Time Zone:20170112T003000
+DTSTAMP:20170824T134117Z
+SEQUENCE:0
+END:VEVENT
+BEGIN:VEVENT
+CREATED:20170824T123429Z
+UID:9C4B7E12-8D3A-4F61-A2E5-6B1D9C3F8A47
+RRULE:FREQ=MONTHLY;INTERVAL=1;COUNT=2
+DTEND;TZID=America/New_York:20170119T210000
+TRANSP:OPAQUE
+X-APPLE-TRAVEL-ADVISORY-BEHAVIOR:AUTOMATIC
+SUMMARY:evening shift
+DTSTART;TZID=America/New_York:20170119T200000
+DTSTAMP:20170824T134117Z
+SEQUENCE:0
+END:VEVENT
+END:VCALENDAR