icalevents icon indicating copy to clipboard operation
icalevents copied to clipboard

fix: Use the event start time zone to create rules so that daily rules don't end up on the wrong day

Open RickMeasham opened this issue 3 years ago • 3 comments

This pull requests resolves issue https://github.com/jazzband/icalevents/issues/89


This change results in odd errors in the cest test.

However it isn't clear what that test is doing, so I'm not sure

  1. If the current test is correct and I have somehow broken it, or
  2. If the current test is incorrect and needs to be updated.

This PR results in three fewer rows in the evs in the assertion (112 rather than 115), however those rows are in the middle of the whole result set:

2021-06-14 16:00:00+02:00: Busy (ended)
2021-05-27 15:00:00+02:00: Busy (ended)
2021-05-20 11:00:00+02:00: Busy (ended)

Again, I don't know what that test is testing so I cannot provide a fix.

RickMeasham avatar Oct 30 '21 01:10 RickMeasham

#77 also causes a weird change in the number of evs in that test when I was testing it locally. I imported that calendar into Google Calendar and it agreed there should be 115 events. So it would seem that is the appropriate number? ¯\_(ツ)_/¯ .

From what I can tell that test is handling a VTIMEZONE declaration. But the calendar itself is complex enough that subtle changes appear to cause this library to return incorrect results. (In both your PR and #77 it is dealing with timezone handling.)

tresni avatar Oct 30 '21 23:10 tresni

@tresni Without taking any look at it at all (I will later) I wonder if there's 115 events in the calendar, but some part of icalevents removes duplicates -- which don't occur until you fix the time zone.

Again, this is purely a random guess as to why events in the middle of a calendar might go missing. I will take a look later at specifically which events go missing.

RickMeasham avatar Oct 31 '21 22:10 RickMeasham

@RickMeasham I will check today. Are you ok with me updating this pr?

eigenmannmartin avatar Nov 09 '21 10:11 eigenmannmartin

Thank you very much for this implementation. I was able to verify that it was already fixed. Let me know if that is not the case :-)

eigenmannmartin avatar Sep 09 '24 11:09 eigenmannmartin