Fixes #4307 - Calendar subscription exports wrong timestamps

lib/calendar_subscriptions.rb

@@ -56,10 +56,11 @@ class CalendarSubscriptions
 
     cal = Icalendar::Calendar.new
 
+    tz = TZInfo::Timezone.get(@time_zone)
+
     # https://github.com/zammad/zammad/issues/3989
     # https://datatracker.ietf.org/doc/html/rfc5545#section-3.2.19
     if events_data.first.present?
-      tz = TZInfo::Timezone.get(@time_zone)
       timezone = tz.ical_timezone(DateTime.parse(events_data.first[:dtstart].to_s))
       cal.add_timezone(timezone)
     end
@@ -67,8 +68,18 @@ class CalendarSubscriptions
     events_data.each do |event_data|
 
       cal.event do |e|
-        e.dtstart = Icalendar::Values::DateTime.new(event_data[:dtstart], 'tzid' => @time_zone)
-        e.dtend   = Icalendar::Values::DateTime.new(event_data[:dtend], 'tzid' => @time_zone)
+        dtstart = DateTime.parse(event_data[:dtstart].to_s)
+        dtend   = DateTime.parse(event_data[:dtend].to_s)
+
+        # by design all new/open ticket events are scheduled at midnight:
+        # skip adding TZ offset
+        if !event_data[:type].match('new_open')
+          dtstart = tz.utc_to_local(dtstart)
+          dtend = tz.utc_to_local(dtend)
+        end
+
+        e.dtstart = Icalendar::Values::DateTime.new(dtstart, 'tzid' => @time_zone)
+        e.dtend   = Icalendar::Values::DateTime.new(dtend, 'tzid' => @time_zone)
         if event_data[:alarm]
           e.alarm do |a|
             a.action  = 'DISPLAY'

lib/calendar_subscriptions/tickets.rb

@@ -90,6 +90,7 @@ class CalendarSubscriptions::Tickets
       event_data[:dtend]       = Icalendar::Values::Date.new(Time.zone.today, 'tzid' => @time_zone)
       event_data[:summary]     = "#{translated_state} #{translated_ticket}: '#{ticket.title}'"
       event_data[:description] = "T##{ticket.number}"
+      event_data[:type]        = 'new_open'
 
       events_data.push event_data
     end
@@ -154,6 +155,7 @@ class CalendarSubscriptions::Tickets
           trigger: '-PT1M',
         }
       end
+      event_data[:type] = 'pending'
 
       events_data.push event_data
     end
@@ -208,6 +210,7 @@ class CalendarSubscriptions::Tickets
           trigger: '-PT1M',
         }
       end
+      event_data[:type] = 'escalated'
 
       events_data.push event_data
     end

lib/omniauth/saml_database.rb

@@ -10,7 +10,7 @@ class SamlDatabase < OmniAuth::Strategies::SAML
 
     # Use meta URL as entity id/issues as it is best practice.
     # See: https://community.zammad.org/t/saml-oidc-third-party-authentication/2533/13
-    entity_id                      = "#{http_type}://#{fqdn}/auth/saml/metadata"
+    entity_id                      = 'zammad-saml-sp'
     assertion_consumer_service_url = "#{http_type}://#{fqdn}/auth/saml/callback"
 
     config  = Setting.get('auth_saml_credentials') || {}

spec/lib/calendar_subscriptions_spec.rb

@@ -3,6 +3,12 @@
 require 'rails_helper'
 
 RSpec.describe CalendarSubscriptions, :aggregate_failures do
+  # Set second fraction to zero for easier comparsion.
+  def dt_now
+    now = DateTime.now
+    DateTime.new(now.year, now.month, now.day, now.hour, now.minute, now.second, 0)
+  end
+
   let(:groups) { create_list(:group, 2) }
   let(:agents) do
     [
@@ -11,8 +17,6 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
     ]
   end
   let(:tickets) do
-    travel_to DateTime.now - 3.months
-
     tickets = [
       create(:ticket,
              group: groups.first,
@@ -21,7 +25,7 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
              group:        groups.first,
              owner:        agents.first,
              state:        Ticket::State.lookup(name: 'pending reminder'),
-             pending_time: DateTime.now + 2.days),
+             pending_time: dt_now + 2.days),
       create(:ticket,
              group: groups.first,
              owner: agents.first),
@@ -33,7 +37,7 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
              group:        groups.second,
              owner:        agents.second,
              state:        Ticket::State.lookup(name: 'pending reminder'),
-             pending_time: DateTime.now + 2.days),
+             pending_time: dt_now + 2.days),
       create(:ticket,
              group: groups.second,
              owner: agents.second),
@@ -45,7 +49,7 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
              group:        groups.first,
              owner:        User.find(1),
              state:        Ticket::State.lookup(name: 'pending reminder'),
-             pending_time: DateTime.now + 2.days),
+             pending_time: dt_now + 2.days),
 
       create(:ticket,
              group: groups.first,
@@ -59,7 +63,7 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
              group:        groups.second,
              owner:        User.find(1),
              state:        Ticket::State.lookup(name: 'pending reminder'),
-             pending_time: DateTime.now + 2.days),
+             pending_time: dt_now + 2.days),
 
       create(:ticket,
              group: groups.second,
@@ -67,19 +71,14 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
     ]
     # set escalation_at manually, clear cache to have correct content later
     [2, 5, 8, 11].each do |index|
-      tickets[index].update_columns(escalation_at: DateTime.now + 2.weeks)
+      tickets[index].update_columns(escalation_at: dt_now + 2.weeks)
     end
     Rails.cache.clear
 
-    travel_back
     tickets
   end
-  let(:ical) { described_class.new(agent).all }
-  let(:calendars) do
-    Icalendar::Calendar.parse(ical).each do |calendar|
-      calendar.events.sort_by!(&:description)
-    end
-  end
+  let(:ical)      { described_class.new(agent).all }
+  let(:calendars) { Icalendar::Calendar.parse(ical) }
 
   # https://github.com/zammad/zammad/issues/3989
   # https://datatracker.ietf.org/doc/html/rfc5545#section-3.2.19
@@ -101,44 +100,85 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
     end
   end
 
+  def event_to_ticket(event)
+    Ticket.find_by(number: event.description.to_s[2..])
+  end
+
   shared_examples 'verify events' do |params|
     it 'has ticket related events' do
-      params[:expectations].each do |expected|
-        event = calendars.first.events[expected[:event_id]]
-        ticket = tickets[expected[:ticket_id]]
+      calendars.first.events.each do |event|
+        ticket = event_to_ticket(event)
 
-        expect(event.dtstart.strftime('%Y-%m-%d')).to match(Time.zone.today.to_s)
         expect(event.description.to_s).to match("T##{ticket.number}")
-        expect(event.summary).to match(ticket.title)
-        expect(event.has_alarm?).to be expected[:alarm] || false
+        expect(event.summary.to_s).to match(ticket.title)
+
+        if !event.summary.to_s.match?(%r{^new})
+          expect(event.has_alarm?).to be params[:alarm]
+        end
+      end
+    end
+  end
+
+  def verify_timestamp(dtstart, dtend, tstart)
+    expect(dtstart).to match(tstart)
+    expect(dtend).to match(tstart)
+  end
+
+  def verify_offset(dtstart, dtend, tstart)
+    time_zone = Setting.get('timezone_default').presence || 'UTC'
+    tz = TZInfo::Timezone.get(time_zone)
+
+    expect(dtstart.utc_offset).to match(tz.utc_to_local(tstart).utc_offset)
+    expect(dtend.utc_offset).to match(tz.utc_to_local(tstart).utc_offset)
+  end
+
+  # https://github.com/zammad/zammad/issues/4307
+  shared_examples 'verify timestamps' do
+    it 'event timestamps match related ticket timestamps' do
+
+      calendars.first.events.each do |event|
+        ticket = event_to_ticket(event)
+
+        dtstart = event.dtstart
+        dtend = event.dtend
+
+        case event.summary.to_s
+        when %r{new}
+          tstart = ticket.updated_at
+
+          expect(dtstart.strftime('%Y-%m-%d')).to match(ticket.updated_at.strftime('%Y-%m-%d'))
+          expect(dtend.strftime('%Y-%m-%d')).to match(ticket.updated_at.strftime('%Y-%m-%d'))
+          verify_offset(dtstart, dtend, tstart)
+
+          next
+        when %r{pending reminder}
+          tstart = ticket.pending_time
+        when %r{ticket escalation}
+          tstart = ticket.escalation_at
+        end
+
+        verify_timestamp(dtstart, dtend, tstart)
+        verify_offset(dtstart, dtend, tstart)
       end
     end
   end
 
   describe 'with subscriber agent in first group' do
+    let(:agent) { agents.first }
+
     context 'with default subscriptions' do
       before do
         tickets
         calendars
       end
 
-      let(:agent) { agents.first }
-
       include_examples 'verify ical'
-
       include_examples 'verify calendar', {
         count:  1,
         events: 4,
       }
-
-      include_examples 'verify events', {
-        expectations: [
-          { event_id: 0, ticket_id: 0 },
-          { event_id: 1, ticket_id: 1 },
-          { event_id: 2, ticket_id: 2 },
-          { event_id: 3, ticket_id: 2 }
-        ]
-      }
+      include_examples 'verify events', { alarm: false }
+      include_examples 'verify timestamps'
     end
 
     context 'with specific subscriptions' do
@@ -166,31 +206,19 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
         calendars
       end
 
-      let(:agent) { agents.first }
-
       include_examples 'verify ical'
-
       include_examples 'verify calendar', {
         count:  1,
         events: 8,
       }
-
-      include_examples 'verify events', {
-        expectations: [
-          { event_id: 0, ticket_id: 0 },
-          { event_id: 1, ticket_id: 1, alarm: true },
-          { event_id: 2, ticket_id: 2 },
-          { event_id: 3, ticket_id: 2, alarm: true },
-          { event_id: 4, ticket_id: 6 },
-          { event_id: 5, ticket_id: 7, alarm: true },
-          { event_id: 6, ticket_id: 8 },
-          { event_id: 7, ticket_id: 8, alarm: true }
-        ]
-      }
+      include_examples 'verify events', { alarm: true }
+      include_examples 'verify timestamps'
     end
   end
 
   describe 'with subscriber agent in second group' do
+    let(:agent) { agents.second }
+
     context 'with default subscriptions' do
       before do
         Setting.set('timezone_default', 'Europe/Berlin')
@@ -198,23 +226,12 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
         calendars
       end
 
-      let(:agent) { agents.second }
-
-      include_examples 'verify ical'
-
-      include_examples 'verify calendar', {
+      include_examples 'verify ical', {
         count:  1,
         events: 4,
       }
-
-      include_examples 'verify events', {
-        expectations: [
-          { event_id: 0, ticket_id: 3 },
-          { event_id: 1, ticket_id: 4 },
-          { event_id: 2, ticket_id: 5 },
-          { event_id: 3, ticket_id: 5 }
-        ]
-      }
+      include_examples 'verify events', { alarm: false }
+      include_examples 'verify timestamps'
     end
 
     context 'with specific subscriptions' do
@@ -244,27 +261,13 @@ RSpec.describe CalendarSubscriptions, :aggregate_failures do
         calendars
       end
 
-      let(:agent) { agents.second }
-
       include_examples 'verify ical'
-
       include_examples 'verify calendar', {
         count:  1,
         events: 8,
       }
-
-      include_examples 'verify events', {
-        expectations: [
-          { event_id: 0, ticket_id: 3 },
-          { event_id: 1, ticket_id: 4 },
-          { event_id: 2, ticket_id: 5 },
-          { event_id: 3, ticket_id: 5 },
-          { event_id: 4, ticket_id: 9 },
-          { event_id: 5, ticket_id: 10 },
-          { event_id: 6, ticket_id: 11 },
-          { event_id: 7, ticket_id: 11 },
-        ]
-      }
+      include_examples 'verify events', { alarm: false }
+      include_examples 'verify timestamps'
     end
   end
 end