Merge branch 'GMT' into 'master'

[mkcal] Don't store time zone for r and exDateTimes when used for all day events. Contributes to JB#47188 See merge request mer-core/mkcal!24
sailfishos · Sep 4, 2019 · 76ed7d6 · 76ed7d6
2 parents dbd5721 + 59e47b6
commit 76ed7d6
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 9 deletions.
diff --git a/src/sqliteformat.cpp b/src/sqliteformat.cpp
@@ -529,11 +529,21 @@ bool SqliteFormat::Private::modifyRdates(Incidence::Ptr incidence, int rowid, DB
             }
         }
 
+        // Both for rDateTimes and exDateTimes, there are possible issues
+        // with all day events. KCalCore::Recurrence::timesInInterval()
+        // is returning repeating events in clock time for all day events,
+        // Thus being yyyy-mm-ddT00:00:00 and then "converted" to local
+        // zone, for display (meaning being after yyyy-mm-ddT00:00:00+xxxx).
+        // When saving, we don't want to store this local zone info, otherwise,
+        // the saved date-time won't match when read in another time zone.
         type = SqliteFormat::RDateTime;
         DateTimeList dateTimeList = incidence->recurrence()->rDateTimes();
         DateTimeList::ConstIterator it;
         for (it = dateTimeList.constBegin(); it != dateTimeList.constEnd(); ++it) {
-            if (!modifyRdate(rowid, type, (*it), (dbop == DBUpdate ? DBInsert : dbop), stmt2)) {
+            bool allDay(incidence->allDay() && it->isLocalZone() && it->time() == QTime(0,0));
+            if (!modifyRdate(rowid, type,
+                             (allDay) ? KDateTime(it->date(), QTime(0, 0), KDateTime::ClockTime) : (*it),
+                             (dbop == DBUpdate ? DBInsert : dbop), stmt2)) {
                 qCWarning(lcMkcal) << "failed to modify rdatetimes for incidence" << incidence->uid();
                 success = false;
             }
@@ -542,7 +552,10 @@ bool SqliteFormat::Private::modifyRdates(Incidence::Ptr incidence, int rowid, DB
         type = SqliteFormat::XDateTime;
         dateTimeList = incidence->recurrence()->exDateTimes();
         for (it = dateTimeList.constBegin(); it != dateTimeList.constEnd(); ++it) {
-            if (!modifyRdate(rowid, type, (*it), (dbop == DBUpdate ? DBInsert : dbop), stmt2)) {
+            bool allDay(incidence->allDay() && it->isLocalZone() && it->time() == QTime(0,0));
+            if (!modifyRdate(rowid, type,
+                             (allDay) ? KDateTime(it->date(), QTime(0, 0), KDateTime::ClockTime) : (*it),
+                             (dbop == DBUpdate ? DBInsert : dbop), stmt2)) {
                 qCWarning(lcMkcal) << "failed to modify xdatetimes for incidence" << incidence->uid();
                 success = false;
             }

diff --git a/tests/tst_storage.cpp b/tests/tst_storage.cpp
@@ -217,18 +217,73 @@ void tst_storage::tst_origintimes()
     QCOMPARE(ss->toLocalOriginTime(localTime), ss->toLocalOriginTime(utcTime));
 }
 
+void tst_storage::tst_rawEvents_data()
+{
+    QTest::addColumn<QDate>("date");
+    QTest::addColumn<QTime>("startTime");
+    QTest::addColumn<QTime>("endTime");
+    QTest::addColumn<QString>("timeZone");
+
+    QTest::newRow("non all day event in clock time")
+        << QDate(2010, 01, 01)
+        << QTime(12, 0) << QTime(13, 0)
+        << QString();
+    QTest::newRow("non all day event in Europe/Helsinki")
+        << QDate(2010, 02, 01)
+        << QTime(12, 0) << QTime(13, 0)
+        << QString("Europe/Helsinki");
+    QTest::newRow("non all day event in Pacific/Midway")
+        << QDate(2010, 03, 01)
+        << QTime(8, 0) << QTime(9, 0)
+        << QString("Pacific/Midway");
+    QTest::newRow("all day event stored as local clock")
+        << QDate(2010, 04, 01)
+        << QTime(0, 0) << QTime()
+        << QString();
+    QTest::newRow("all day event stored as date only")
+        << QDate(2010, 05, 01)
+        << QTime() << QTime()
+        << QString();
+}
+
 void tst_storage::tst_rawEvents()
 {
-    // TODO: Should split tests if making more cases outside storage
+    QFETCH(QDate, date);
+    QFETCH(QTime, startTime);
+    QFETCH(QTime, endTime);
+    QFETCH(QString, timeZone);
+
+    KDateTime::Spec spec(timeZone.isEmpty() ? KDateTime::Spec(KDateTime::ClockTime)
+                         : KDateTime::Spec(KSystemTimeZones::zone(timeZone)));
     auto event = KCalCore::Event::Ptr(new KCalCore::Event);
-    // NOTE: no other events should be made happening this day
-    QDate startDate(2010, 12, 1);
-    event->setDtStart(KDateTime(startDate, QTime(12, 0), KDateTime::ClockTime));
-    event->setDtEnd(KDateTime(startDate, QTime(13, 0), KDateTime::ClockTime));
+    if (startTime.isValid()) {
+        event->setDtStart(KDateTime(date, startTime, spec));
+        if (endTime.isValid()) {
+            event->setDtEnd(KDateTime(date, endTime, spec));
+        } else if (startTime == QTime(0, 0)) {
+            event->setAllDay(true);
+        }
+    } else {
+        event->setDtStart(KDateTime(date, KDateTime::ClockTime));
+    }
+    event->setSummary(QStringLiteral("testing rawExpandedEvents()"));
 
     KCalCore::Recurrence *recurrence = event->recurrence();
     recurrence->setDaily(1);
     recurrence->setStartDateTime(event->dtStart());
+    recurrence->setDuration(5);
+    recurrence->setAllDay(event->allDay());
+    if (event->dtStart().isDateOnly()) {
+        // Save exception as clock time
+        recurrence->addExDateTime(KDateTime(event->dtStart().date().addDays(1), QTime(0,0), KDateTime::ClockTime));
+        // Save exception as a local zone
+        recurrence->addExDateTime(KDateTime(event->dtStart().date().addDays(2), QTime(0,0)));
+    } else {
+        // Register an exception in spec of the event
+        recurrence->addExDateTime(event->dtStart().addDays(1));
+        // Register an exception in another time zone
+        recurrence->addExDateTime(event->dtStart().addDays(2).toTimeSpec(KSystemTimeZones::zone("America/Toronto")));
+    }
 
     m_calendar->addEvent(event, NotebookId);
     m_storage->save();
@@ -237,14 +292,20 @@ void tst_storage::tst_rawEvents()
 
     auto fetchEvent = m_calendar->event(uid);
     QVERIFY(fetchEvent);
+    QCOMPARE(fetchEvent->allDay(), event->allDay());
     KCalCore::Recurrence *fetchRecurrence = fetchEvent->recurrence();
     QVERIFY(fetchRecurrence);
+    QCOMPARE(fetchRecurrence->allDay(), recurrence->allDay());
 
-    // should return occurrence for both days
+    // should return occurrence for both days and omit exceptions
     mKCal::ExtendedCalendar::ExpandedIncidenceList events
-        = m_calendar->rawExpandedEvents(startDate, startDate.addDays(1), false, false, KDateTime::Spec(KDateTime::LocalZone));
+        = m_calendar->rawExpandedEvents(date, date.addDays(3), false, false, KDateTime::Spec(KDateTime::LocalZone));
 
     QCOMPARE(events.size(), 2);
+    QCOMPARE(events[0].first.dtStart, event->dtStart().toLocalZone().dateTime());
+    QCOMPARE(events[0].first.dtEnd, event->dtEnd().toLocalZone().dateTime());
+    QCOMPARE(events[1].first.dtStart, event->dtStart().addDays(3).toLocalZone().dateTime());
+    QCOMPARE(events[1].first.dtEnd, event->dtEnd().addDays(3).toLocalZone().dateTime());
 }
 
 // Check that the creation date can be tuned and restored properly.
@@ -605,6 +666,8 @@ void tst_storage::tst_icalAllDay()
     QCOMPARE(event->dtEnd(), fetchEvent->dtEnd());
 }
 
+
+
 void tst_storage::openDb(bool clear)
 {
     m_calendar = ExtendedCalendar::Ptr(new ExtendedCalendar(KDateTime::Spec::LocalZone()));

diff --git a/tests/tst_storage.h b/tests/tst_storage.h
@@ -28,6 +28,7 @@ private slots:
     void tst_alldayRecurrence();
     void tst_origintimes();
     void tst_recurrence();
+    void tst_rawEvents_data();
     void tst_rawEvents();
     void tst_dateCreated_data();
     void tst_dateCreated();