Skip to content

Commit faf15ad

Browse files
plebcityKoen KonstKristjanESPERANTO
authored
Refactored calendarfetcherutils to fix many of the timezone and DST related issues and make debugging way easier (#3806)
Refactored calendarfetcherutils to remove as many of the date conversions as possible and use moment tz when calculating recurring events, this will make debugging a lot easier and fixes problems from the past with offsets and DST not being handled properly. Also added some tests to test the behavior of the refactored methodes to make sure the correct event dates are returned. Refactored calendar.js aswell to make sure the unix UTC start and end date of events are properly converted to a local timezone and displayed correctly for the user. This PR relates to: #3797 --------- Co-authored-by: Koen Konst <[email protected]> Co-authored-by: Kristjan ESPERANTO <[email protected]>
1 parent 052ec1c commit faf15ad

File tree

6 files changed

+339
-638
lines changed

6 files changed

+339
-638
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ planned for 2025-07-01
3434
- [refactor] Replace `ansis` with built-in function `util.styleText` (#3793)
3535
- [core] Integrate stuff from `vendor` and `fonts` folders into main `package.json`, simplifies install and maintaining dependencies (#3795, #3805)
3636
- [l10n] Complete translations (with the help of translation tools) (#3794)
37+
- [refactor] Refactored `calendarfetcherutils` in Calendar module to handle timezones better (#3806)
38+
- Removed as many of the date conversions as possible
39+
- Use `moment-timezone` when calculating recurring events, this will fix problems from the past with offsets and DST not being handled properly
40+
- Added some tests to test the behavior of the refactored methods to make sure the correct event dates are returned
3741

3842
### Fixed
3943

‎cspell.config.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
"dkallen",
5353
"drivelist",
5454
"DTEND",
55+
"DTSTAMP",
56+
"DTSTART",
5557
"Duffman",
5658
"earlman",
5759
"easyas",
@@ -107,6 +109,7 @@
107109
"jsonlint",
108110
"jupadin",
109111
"kaennchenstruggle",
112+
"Kalenderwoche",
110113
"kenzal",
111114
"Keyport",
112115
"khassel",

‎modules/default/calendar/calendar.js

Lines changed: 70 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Module.register("calendar", {
7777

7878
// Define required scripts.
7979
getScripts () {
80-
return ["calendarutils.js", "moment.js"];
80+
return ["calendarutils.js", "moment.js", "moment-timezone.js"];
8181
},
8282

8383
// Define required translations.
@@ -215,18 +215,9 @@ Module.register("calendar", {
215215
this.updateDom(this.config.animationSpeed);
216216
},
217217

218-
eventEndingWithinNextFullTimeUnit (event, ONE_DAY) {
219-
const now = new Date();
220-
return event.endDate - now <= ONE_DAY;
221-
},
222-
223218
// Override dom generator.
224219
getDom () {
225220
const ONE_SECOND = 1000; // 1,000 milliseconds
226-
const ONE_MINUTE = ONE_SECOND * 60;
227-
const ONE_HOUR = ONE_MINUTE * 60;
228-
const ONE_DAY = ONE_HOUR * 24;
229-
230221
const events = this.createEventList(true);
231222
const wrapper = document.createElement("table");
232223
wrapper.className = this.config.tableClass;
@@ -258,7 +249,9 @@ Module.register("calendar", {
258249
let lastSeenDate = "";
259250

260251
events.forEach((event, index) => {
261-
const dateAsString = moment(event.startDate, "x").format(this.config.dateFormat);
252+
const eventStartDateMoment = this.timestampToMoment(event.startDate);
253+
const eventEndDateMoment = this.timestampToMoment(event.endDate);
254+
const dateAsString = eventStartDateMoment.format(this.config.dateFormat);
262255
if (this.config.timeFormat === "dateheaders") {
263256
if (lastSeenDate !== dateAsString) {
264257
const dateRow = document.createElement("tr");
@@ -340,7 +333,7 @@ Module.register("calendar", {
340333
repeatingCountTitle = this.countTitleForUrl(event.url);
341334

342335
if (repeatingCountTitle !== "") {
343-
const thisYear = new Date(parseInt(event.startDate)).getFullYear(),
336+
const thisYear = eventStartDateMoment.year(),
344337
yearDiff = thisYear - event.firstYear;
345338

346339
repeatingCountTitle = `, ${yearDiff} ${repeatingCountTitle}`;
@@ -395,14 +388,14 @@ Module.register("calendar", {
395388
timeWrapper.className = `time light ${this.config.flipDateHeaderTitle ? "align-right " : "align-left "}${this.timeClassForUrl(event.url)}`;
396389
timeWrapper.style.paddingLeft = "2px";
397390
timeWrapper.style.textAlign = this.config.flipDateHeaderTitle ? "right" : "left";
398-
timeWrapper.innerHTML = moment(event.startDate, "x").format("LT");
391+
timeWrapper.innerHTML = eventStartDateMoment.format("LT");
399392

400393
// Add endDate to dataheaders if showEnd is enabled
401394
if (this.config.showEnd) {
402395
if (this.config.showEndsOnlyWithDuration && event.startDate === event.endDate) {
403396
// no duration here, don't display end
404397
} else {
405-
timeWrapper.innerHTML += ` - ${CalendarUtils.capFirst(moment(event.endDate, "x").format("LT"))}`;
398+
timeWrapper.innerHTML += ` - ${CalendarUtils.capFirst(eventEndDateMoment.format("LT"))}`;
406399
}
407400
}
408401

@@ -415,70 +408,69 @@ Module.register("calendar", {
415408
const timeWrapper = document.createElement("td");
416409

417410
eventWrapper.appendChild(titleWrapper);
418-
const now = new Date();
411+
const now = moment();
419412

420413
if (this.config.timeFormat === "absolute") {
421414
// Use dateFormat
422-
timeWrapper.innerHTML = CalendarUtils.capFirst(moment(event.startDate, "x").format(this.config.dateFormat));
415+
timeWrapper.innerHTML = CalendarUtils.capFirst(eventStartDateMoment.format(this.config.dateFormat));
423416
// Add end time if showEnd
424417
if (this.config.showEnd) {
425418
// and has a duation
426419
if (event.startDate !== event.endDate) {
427420
timeWrapper.innerHTML += "-";
428-
timeWrapper.innerHTML += CalendarUtils.capFirst(moment(event.endDate, "x").format(this.config.dateEndFormat));
421+
timeWrapper.innerHTML += CalendarUtils.capFirst(eventEndDateMoment.format(this.config.dateEndFormat));
429422
}
430423
}
431424

432425
// For full day events we use the fullDayEventDateFormat
433426
if (event.fullDayEvent) {
434427
//subtract one second so that fullDayEvents end at 23:59:59, and not at 0:00:00 one the next day
435-
event.endDate -= ONE_SECOND;
436-
timeWrapper.innerHTML = CalendarUtils.capFirst(moment(event.startDate, "x").format(this.config.fullDayEventDateFormat));
428+
eventEndDateMoment.subtract(1, "second");
429+
timeWrapper.innerHTML = CalendarUtils.capFirst(eventStartDateMoment.format(this.config.fullDayEventDateFormat));
437430
// only show end if requested and allowed and the dates are different
438-
if (this.config.showEnd && !this.config.showEndsOnlyWithDuration && moment(event.startDate, "x").format("YYYYMMDD") !== moment(event.endDate, "x").format("YYYYMMDD")) {
431+
if (this.config.showEnd && !this.config.showEndsOnlyWithDuration && !eventStartDateMoment.isSame(eventEndDateMoment, "d")) {
439432
timeWrapper.innerHTML += "-";
440-
timeWrapper.innerHTML += CalendarUtils.capFirst(moment(event.endDate, "x").format(this.config.fullDayEventDateFormat));
441-
} else
442-
if ((moment(event.startDate, "x").format("YYYYMMDD") !== moment(event.endDate, "x").format("YYYYMMDD")) && (moment(event.startDate, "x") < moment(now, "x"))) {
443-
timeWrapper.innerHTML = CalendarUtils.capFirst(moment(now, "x").format(this.config.fullDayEventDateFormat));
444-
}
445-
} else if (this.config.getRelative > 0 && event.startDate < now) {
433+
timeWrapper.innerHTML += CalendarUtils.capFirst(eventEndDateMoment.format(this.config.fullDayEventDateFormat));
434+
} else if (!eventStartDateMoment.isSame(eventEndDateMoment, "d") && eventStartDateMoment.isBefore(now)) {
435+
timeWrapper.innerHTML = CalendarUtils.capFirst(now.format(this.config.fullDayEventDateFormat));
436+
}
437+
} else if (this.config.getRelative > 0 && eventStartDateMoment.isBefore(now)) {
446438
// Ongoing and getRelative is set
447439
timeWrapper.innerHTML = CalendarUtils.capFirst(
448440
this.translate("RUNNING", {
449441
fallback: `${this.translate("RUNNING")} {timeUntilEnd}`,
450-
timeUntilEnd: moment(event.endDate, "x").fromNow(true)
442+
timeUntilEnd: eventEndDateMoment.fromNow(true)
451443
})
452444
);
453-
} else if (this.config.urgency > 0 && event.startDate - now < this.config.urgency * ONE_DAY) {
445+
} else if (this.config.urgency > 0 && eventStartDateMoment.diff(now, "d") < this.config.urgency) {
454446
// Within urgency days
455-
timeWrapper.innerHTML = CalendarUtils.capFirst(moment(event.startDate, "x").fromNow());
447+
timeWrapper.innerHTML = CalendarUtils.capFirst(eventStartDateMoment.fromNow());
456448
}
457449
if (event.fullDayEvent && this.config.nextDaysRelative) {
458450
// Full days events within the next two days
459451
if (event.today) {
460452
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("TODAY"));
461453
} else if (event.yesterday) {
462454
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("YESTERDAY"));
463-
} else if (event.startDate - now < ONE_DAY && event.startDate - now > 0) {
455+
} else if (event.tomorrow) {
464456
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("TOMORROW"));
465-
} else if (event.startDate - now < 2 * ONE_DAY && event.startDate - now > 0) {
457+
} else if (event.dayAfterTomorrow) {
466458
if (this.translate("DAYAFTERTOMORROW") !== "DAYAFTERTOMORROW") {
467459
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("DAYAFTERTOMORROW"));
468460
}
469461
}
470462
}
471463
} else {
472464
// Show relative times
473-
if (event.startDate >= now || (event.fullDayEvent && this.eventEndingWithinNextFullTimeUnit(event, ONE_DAY))) {
465+
if (eventStartDateMoment.isSameOrAfter(now) || (event.fullDayEvent && eventEndDateMoment.diff(now, "days") === 0)) {
474466
// Use relative time
475467
if (!this.config.hideTime && !event.fullDayEvent) {
476468
Log.debug("event not hidden and not fullday");
477-
timeWrapper.innerHTML = `${CalendarUtils.capFirst(moment(event.startDate, "x").calendar(null, { sameElse: this.config.dateFormat }))}`;
469+
timeWrapper.innerHTML = `${CalendarUtils.capFirst(eventStartDateMoment.calendar(null, { sameElse: this.config.dateFormat }))}`;
478470
} else {
479471
Log.debug("event full day or hidden");
480472
timeWrapper.innerHTML = `${CalendarUtils.capFirst(
481-
moment(event.startDate, "x").calendar(null, {
473+
eventStartDateMoment.calendar(null, {
482474
sameDay: this.config.showTimeToday ? "LT" : `[${this.translate("TODAY")}]`,
483475
nextDay: `[${this.translate("TOMORROW")}]`,
484476
nextWeek: "dddd",
@@ -488,33 +480,33 @@ Module.register("calendar", {
488480
}
489481
if (event.fullDayEvent) {
490482
// Full days events within the next two days
491-
if (event.today || (event.fullDayEvent && this.eventEndingWithinNextFullTimeUnit(event, ONE_DAY))) {
483+
if (event.today || (event.fullDayEvent && eventEndDateMoment.diff(now, "days") === 0)) {
492484
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("TODAY"));
493485
} else if (event.dayBeforeYesterday) {
494486
if (this.translate("DAYBEFOREYESTERDAY") !== "DAYBEFOREYESTERDAY") {
495487
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("DAYBEFOREYESTERDAY"));
496488
}
497489
} else if (event.yesterday) {
498490
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("YESTERDAY"));
499-
} else if (event.startDate - now < ONE_DAY && event.startDate - now > 0) {
491+
} else if (event.tomorrow) {
500492
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("TOMORROW"));
501-
} else if (event.startDate - now < 2 * ONE_DAY && event.startDate - now > 0) {
493+
} else if (event.dayAfterTomorrow) {
502494
if (this.translate("DAYAFTERTOMORROW") !== "DAYAFTERTOMORROW") {
503495
timeWrapper.innerHTML = CalendarUtils.capFirst(this.translate("DAYAFTERTOMORROW"));
504496
}
505497
}
506498
Log.info("event fullday");
507-
} else if (event.startDate - now < this.config.getRelative * ONE_HOUR) {
499+
} else if (eventStartDateMoment.diff(now, "h") < this.config.getRelative) {
508500
Log.info("not full day but within getrelative size");
509501
// If event is within getRelative hours, display 'in xxx' time format or moment.fromNow()
510-
timeWrapper.innerHTML = `${CalendarUtils.capFirst(moment(event.startDate, "x").fromNow())}`;
502+
timeWrapper.innerHTML = `${CalendarUtils.capFirst(eventStartDateMoment.fromNow())}`;
511503
}
512504
} else {
513505
// Ongoing event
514506
timeWrapper.innerHTML = CalendarUtils.capFirst(
515507
this.translate("RUNNING", {
516508
fallback: `${this.translate("RUNNING")} {timeUntilEnd}`,
517-
timeUntilEnd: moment(event.endDate, "x").fromNow(true)
509+
timeUntilEnd: eventEndDateMoment.fromNow(true)
518510
})
519511
);
520512
}
@@ -593,46 +585,46 @@ Module.register("calendar", {
593585
return false;
594586
},
595587

588+
/**
589+
* converts the given timestamp to a moment with a timezone
590+
* @param {number} timestamp timestamp from an event
591+
* @returns {moment.Moment} moment with a timezone
592+
*/
593+
timestampToMoment (timestamp) {
594+
return moment(timestamp, "x").tz(moment.tz.guess());
595+
},
596+
596597
/**
597598
* Creates the sorted list of all events.
598599
* @param {boolean} limitNumberOfEntries Whether to filter returned events for display.
599600
* @returns {object[]} Array with events.
600601
*/
601602
createEventList (limitNumberOfEntries) {
602-
const ONE_SECOND = 1000; // 1,000 milliseconds
603-
const ONE_MINUTE = ONE_SECOND * 60;
604-
const ONE_HOUR = ONE_MINUTE * 60;
605-
const ONE_DAY = ONE_HOUR * 24;
606-
607-
let now, today, future;
608-
if (this.config.forceUseCurrentTime || this.defaults.forceUseCurrentTime) {
609-
now = new Date();
610-
today = moment().startOf("day");
611-
future = moment().startOf("day").add(this.config.maximumNumberOfDays, "days").toDate();
612-
} else {
613-
now = new Date(Date.now()); // Can use overridden time
614-
today = moment(now).startOf("day");
615-
future = moment(now).startOf("day").add(this.config.maximumNumberOfDays, "days").toDate();
616-
}
603+
let now = moment();
604+
let today = now.clone().startOf("day");
605+
let future = now.clone().startOf("day").add(this.config.maximumNumberOfDays, "days");
606+
617607
let events = [];
618608

619609
for (const calendarUrl in this.calendarData) {
620610
const calendar = this.calendarData[calendarUrl];
621611
let remainingEntries = this.maximumEntriesForUrl(calendarUrl);
622-
let maxPastDaysCompare = now - this.maximumPastDaysForUrl(calendarUrl) * ONE_DAY;
612+
let maxPastDaysCompare = now.clone().subtract(this.maximumPastDaysForUrl(calendarUrl), "days");
623613
let by_url_calevents = [];
624614
for (const e in calendar) {
625615
const event = JSON.parse(JSON.stringify(calendar[e])); // clone object
616+
const eventStartDateMoment = this.timestampToMoment(event.startDate);
617+
const eventEndDateMoment = this.timestampToMoment(event.endDate);
626618

627619
if (this.config.hidePrivate && event.class === "PRIVATE") {
628620
// do not add the current event, skip it
629621
continue;
630622
}
631623
if (limitNumberOfEntries) {
632-
if (event.endDate < maxPastDaysCompare) {
624+
if (eventEndDateMoment.isBefore(maxPastDaysCompare)) {
633625
continue;
634626
}
635-
if (this.config.hideOngoing && event.startDate < now) {
627+
if (this.config.hideOngoing && eventStartDateMoment.isBefore(now)) {
636628
continue;
637629
}
638630
if (this.config.hideDuplicates && this.listContainsEvent(events, event)) {
@@ -641,47 +633,46 @@ Module.register("calendar", {
641633
}
642634

643635
event.url = calendarUrl;
644-
event.today = event.startDate >= today && event.startDate < today + ONE_DAY;
645-
event.dayBeforeYesterday = event.startDate >= today - ONE_DAY * 2 && event.startDate < today - ONE_DAY;
646-
event.yesterday = event.startDate >= today - ONE_DAY && event.startDate < today;
647-
event.tomorrow = !event.today && event.startDate >= today + ONE_DAY && event.startDate < today + 2 * ONE_DAY;
648-
event.dayAfterTomorrow = !event.tomorrow && event.startDate >= today + ONE_DAY * 2 && event.startDate < today + 3 * ONE_DAY;
636+
event.today = eventStartDateMoment.isSame(now, "d");
637+
event.dayBeforeYesterday = eventStartDateMoment.isSame(now.clone().subtract(2, "days"), "d");
638+
event.yesterday = eventStartDateMoment.isSame(now.clone().subtract(1, "days"), "d");
639+
event.tomorrow = eventStartDateMoment.isSame(now.clone().add(1, "days"), "d");
640+
event.dayAfterTomorrow = eventStartDateMoment.isSame(now.clone().add(2, "days"), "d");
649641

650642
/*
651643
* if sliceMultiDayEvents is set to true, multiday events (events exceeding at least one midnight) are sliced into days,
652644
* otherwise, esp. in dateheaders mode it is not clear how long these events are.
653645
*/
654-
const maxCount = Math.round((event.endDate - 1 - moment(event.startDate, "x").endOf("day").format("x")) / ONE_DAY) + 1;
646+
const maxCount = eventEndDateMoment.diff(eventStartDateMoment, "days");
655647
if (this.config.sliceMultiDayEvents && maxCount > 1) {
656648
const splitEvents = [];
657649
let midnight
658-
= moment(event.startDate, "x")
650+
= eventStartDateMoment
659651
.clone()
660652
.startOf("day")
661653
.add(1, "day")
662-
.endOf("day")
663-
.format("x");
654+
.endOf("day");
664655
let count = 1;
665-
while (event.endDate > midnight) {
656+
while (eventEndDateMoment.isAfter(midnight)) {
666657
const thisEvent = JSON.parse(JSON.stringify(event)); // clone object
667-
thisEvent.today = thisEvent.startDate >= today && thisEvent.startDate < today + ONE_DAY;
668-
thisEvent.tomorrow = !thisEvent.today && thisEvent.startDate >= today + ONE_DAY && thisEvent.startDate < today + 2 * ONE_DAY;
669-
thisEvent.endDate = moment(midnight, "x").clone().subtract(1, "day").format("x");
658+
thisEvent.today = this.timestampToMoment(thisEvent.startDate).isSame(now, "d");
659+
thisEvent.tomorrow = this.timestampToMoment(thisEvent.startDate).isSame(now.clone().add(1, "days"), "d");
660+
thisEvent.endDate = midnight.clone().subtract(1, "day").format("x");
670661
thisEvent.title += ` (${count}/${maxCount})`;
671662
splitEvents.push(thisEvent);
672663

673-
event.startDate = midnight;
664+
event.startDate = midnight.format("x");
674665
count += 1;
675-
midnight = moment(midnight, "x").add(1, "day").endOf("day").format("x"); // next day
666+
midnight = midnight.clone().add(1, "day").endOf("day"); // next day
676667
}
677668
// Last day
678669
event.title += ` (${count}/${maxCount})`;
679-
event.today += event.startDate >= today && event.startDate < today + ONE_DAY;
680-
event.tomorrow = !event.today && event.startDate >= today + ONE_DAY && event.startDate < today + 2 * ONE_DAY;
670+
event.today += this.timestampToMoment(event.startDate).isSame(now, "d");
671+
event.tomorrow = this.timestampToMoment(event.startDate).isSame(now.clone().add(1, "days"), "d");
681672
splitEvents.push(event);
682673

683674
for (let splitEvent of splitEvents) {
684-
if (splitEvent.endDate > now && splitEvent.endDate <= future) {
675+
if (this.timestampToMoment(splitEvent.endDate).isAfter(now) && this.timestampToMoment(splitEvent.endDate).isSameOrBefore(future)) {
685676
by_url_calevents.push(splitEvent);
686677
}
687678
}
@@ -716,16 +707,16 @@ Module.register("calendar", {
716707
*/
717708
if (this.config.limitDays > 0) {
718709
let newEvents = [];
719-
let lastDate = today.clone().subtract(1, "days").format("YYYYMMDD");
710+
let lastDate = today.clone().subtract(1, "days");
720711
let days = 0;
721712
for (const ev of events) {
722-
let eventDate = moment(ev.startDate, "x").format("YYYYMMDD");
713+
let eventDate = this.timestampToMoment(ev.startDate);
723714

724715
/*
725716
* if date of event is later than lastdate
726717
* check if we already are showing max unique days
727718
*/
728-
if (eventDate > lastDate) {
719+
if (eventDate.isAfter(lastDate)) {
729720
// if the only entry in the first day is a full day event that day is not counted as unique
730721
if (!this.config.limitDaysNeverSkip && newEvents.length === 1 && days === 1 && newEvents[0].fullDayEvent) {
731722
days--;

0 commit comments

Comments
 (0)