Holidays update

[github-actions]

No description provided.

[naveensingh]

@Aga-C if you can, please check the holidays for your country: https://github.com/FossifyOrg/Calendar/pull/173/files#diff-4611a3af6b8d110909a5949c975ad17778111b697d7098ba0186733df06aa6de

[Aga-C]

• All holidays appear one day prior ("Niedziela" is "Sunday").

• Unnecessary slash in one of the holidays. However, it appears to be Calendar's bug because it's correct according to ICS specification: https://www.kanzaki.com/docs/ical/text.html.

• Non fixed-date holidays appear only for 1 next year.

[naveensingh]

All holidays appear one day prior ("Niedziela" is "Sunday").

The data from the library looks correct, this is probably related to how the DST is handled as in #165 and #166.

{
    date: '2024-03-31 00:00:00',
    start: 2024-03-30T23:00:00.000Z,
    end: 2024-03-31T22:00:00.000Z,
    name: 'Niedziela Wielkanocna',
    type: 'public',
    rule: 'easter'
}

Non fixed-date holidays appear only for 1 next year.

It's done on purpose. The more holidays we add, the larger the app size. As of now, the ICS asset files added about 70KB when a release APK is created on master. I'll think about increasing it to something like 3 or 5 years.

I'll check these issues after I'm done with some other apps, thanks!

[Aga-C]

The data from the library looks correct, this is probably related to how the DST

ICS file has incorrect data. Maybe it's timezone issue, but in the generator itself:

BEGIN:VEVENT
UID:e868ffd62e1b080c889b575c5481bfbf969f1fbbac21f590fccd137ecb9763fa
SUMMARY:Niedziela Wielkanocna
DTSTAMP:20240314T224634Z
DTSTART;VALUE=DATE:20240330
DTEND;VALUE=DATE:20240331
STATUS:CONFIRMED
END:VEVENT

[naveensingh]

Oops, I forgot that timezone info should be preserved when generating ICS from the date-holidays data. Currently, the 'time' part of the DateTime is ignored completely:

Calendar/.github/workflows/holiday-generator/main.js

Lines 56 to 58 in 1a0ca55

	function getDateArray(date) {
	return [date.getFullYear(), date.getMonth() + 1, date.getDate()];
	}

I'll address this later as well (adding to my TODO list).

[naveensingh]

@Aga-C Thanks :)

[tswistak]

Oops, I forgot that timezone info should be preserved when generating ICS from the date-holidays data. Currently, the 'time' part of the DateTime is ignored completely:

Calendar/.github/workflows/holiday-generator/main.js

Lines 56 to 58 in 1a0ca55

	function getDateArray(date) {
	return [date.getFullYear(), date.getMonth() + 1, date.getDate()];
	}

I'll address this later as well (adding to my TODO list).

I think it's not necessarily needed to keep timezone info in ICS, rather it should be considered in this code. As I see, the Date object returns date information based on the timezone you're currently in, that's why there are wrong dates. I can take a look into it today, to get a date in a given country's timezone.

[naveensingh]

I think it's not necessarily needed to keep timezone info in ICS, rather it should be considered in this code

I meant we should save the timestamps in UTC directly like yyyyMMdd'T'HHmmssZ and let the in-app parser do its thing but converting UTC to local time directly in the generator should also work fine.

[tswistak]

I meant we should save the timestamps in UTC directly like yyyyMMdd'T'HHmmssZ and let the in-app parser do its thing but converting UTC to local time directly in the generator should also work fine.

Will the app still consider that properly as the all-day events? Wouldn't it lead to issues with adding events from other countries? E.g. if I add USA holidays while living in Europe, I'd like to see that e.g. M.L.K Day is on 20th February (all-day), not 20th February 5:00 till 21st February 4:59.

[naveensingh]

Actually no, the importer would fail to detect it as an all-day event. All holidays should be treated as all-day events (even if the data says otherwise because we can not handle every case). Let's proceed with your proposed solution. Just adding the country's timzone offset to the start and end datetime should fix this issue.

E.g. if I add USA holidays while living in Europe, I'd like to see that e.g. M.L.K Day is on 20th February (all-day), not 20th February 5:00 till 21st February 4:59.

It's been a while since I worked with this but IIRC all-day events will show up on the same date in any timezone (the date-time event fields are shifted such that an all-day event on 4th July shows up on 4th July regardless of the user's timezone).

[tswistak]

@naveensingh I've just found a quick solution in date provider. I can set it to return dates in UTC with this function: https://github.com/commenthol/date-holidays-parser/blob/master/docs/Holidays.md#holidayssettimezonetimezone. I'll test it out and if it works correctly, I'll send a PR with a fix.

[curbengh]

australia.ics of this PR only includes country-wide public holidays and missing state-specific holidays (e.g. labour day, king's birthday, easter tuesday).

[tswistak]

australia.ics of this PR only includes country-wide public holidays and missing state-specific holidays (e.g. labour day, king's birthday, easter tuesday).

Holidays generator author here. I did it this way because I didn't know how we should treat regional/state holidays. The same goes for many other countries, e.g. the USA - each state has their own holidays and there are countrywide ones. When we have just the USA or Australia in the app, not particular states, I thought it's better to add only countrywide holidays. It can be always changed, though.

Generally, there are three options how we can approach holiday generation in this case:

1. We only have countrywide holidays, we omit state/regional ones. So it's like it is now.
2. We create a separate ICS file for each state/region, but it will create an enormous number of ICS files. For example, for the USA we would have 50 files instead of just one.
3. Add all state/regional holidays to the country ICS. But, since I don't live in a country like the USA or Australia, I can't tell if I would like to see e.g. California holidays while living in Montana.

In my opinion, the current solution (first option above) is the most universal, but I'm open to modifying the current solution into the third option.

[curbengh]

3. Add all state/regional holidays to the country ICS. But, since I don't live in a country like the USA or Australia, I can't tell if I would like to see e.g. California holidays while living in Montana.

apple/google/microsoft show regional holidays when importing australia holidays, so australian users are accustomed to it or even expecting it.

[tswistak]

I can modify it, but I need to know if it should be done only for Australia or for all countries. @naveensingh what do you think?

[naveensingh]

Let's wait for some user feedback before making any further changes.

Later we'll consider adding a custom field like X-HOLIDAY-TYPE in the ICS files and use it at runtime to allow users to choose between all holidays vs public holidays.