Merge lp:~gary-wzl77/qtorganizer5-eds/fix_1445577 into lp:qtorganizer5-eds

Proposed by Gary.Wang on 2015-07-12
Status: Superseded
Proposed branch: lp:~gary-wzl77/qtorganizer5-eds/fix_1445577
Merge into: lp:qtorganizer5-eds
Diff against target: 140 lines (+74/-23)
2 files modified
organizer/qorganizer-eds-engine.cpp (+26/-20)
tests/unittest/event-test.cpp (+48/-3)
To merge this branch: bzr merge lp:~gary-wzl77/qtorganizer5-eds/fix_1445577
Reviewer Review Type Date Requested Status
Renato Araujo Oliveira Filho (community) Needs Information on 2015-07-23
Alan Pope 🍺🐧🐱 πŸ¦„ 2015-07-12 Pending
Review via email: mp+264489@code.launchpad.net

This proposal has been superseded by a proposal from 2015-08-27.

Commit Message

remove invalid QTimeZone when converting icaltimetype to QDateTime

Description of the Change

remove invalid QTimeZone when converting icaltimetype to QDateTime

To post a comment you must log in.

Reading the bug report I would say that the event is losing the timezone when edited. Is the app setting the correct timezone? If the app does not set the timezone the event will be handled as a Floating event.

review: Needs Information
Gary.Wang (gary-wzl77) wrote :

Actually, there is no such a timezone settings in app. App just follows system timezone.

My Time & Date in system settings
Time zone:
Asia/Shangehai UTC+8

----Create an event (display correctly in weeklyview)
qml: event.name: Ttttt
qml: event.startDateTime: Fri Jul 24 07:00:00 2015 GMT+0800
qml: event.endDateTime: Fri Jul 24 08:00:00 2015 GMT+0800

----Edit an event with only name changed (wrong display and event backs x hours)
qml: event.name: Ttttt-edit
qml: event.startDateTime: Fri Jul 24 07:00:00 2015 GMT+0800
qml: event.endDateTime: Fri Jul 24 08:00:00 2015 GMT+0800

87. By Gary.Wang on 2015-08-26

Convert local timezone to UTC time zone when parsing start & end datetime
if they're not floating time.

Gary.Wang (gary-wzl77) wrote :

I check this issue today and find sth as following:
when user modify a event with start or end date time changed, issue wont' occur.
when user modify a event without start datetime and end date time changed, issue occurs.
Reason for this is if start and end date time are not changed when modification, the timeSpec is for these two datetime is Qt::TimeZone instead of Qt::UTC when parsing them, then
tz.id().constData() == "able/localtime" which cause timezone == null and empty for tzId after icaltimezone_get_builtin_timezone was called.
so finally event backs x(timezone) hours after e_cal_client_modify_objects called.

As I mentioned above, we follow system timezone and have no way to set timezone in qml.
The easy fix is if start & end datetime are not floating time, then we need to convert Local timezone to UTC time zone.
Please review latest MP.
Thanks

Could you resubmit this MR adding "https://code.launchpad.net/~renatofilho/qtorganizer5-eds/fix-1437305/+merge/261666" as Prerequisite Branch, it will make only your changes visible on the diff. And make it easy to review.

Gary.Wang (gary-wzl77) wrote :

OK, I will delete this one later on and create another MP for review since I can't resubmit merge proposal in same branch.
Thanks.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'organizer/qorganizer-eds-engine.cpp'
2--- organizer/qorganizer-eds-engine.cpp 2015-07-14 00:48:09 +0000
3+++ organizer/qorganizer-eds-engine.cpp 2015-08-27 02:06:04 +0000
4@@ -1212,7 +1212,8 @@
5 {
6 uint tmTime;
7
8- if (tzId) {
9+ // check if ialtimetype contais a time and timezone
10+ if (!icaltime_is_date(value) && tzId) {
11 icaltimezone *timezone = icaltimezone_get_builtin_timezone_from_tzid(tzId);
12 // fallback: sometimes the tzId contains the location name
13 if (!timezone) {
14@@ -1237,26 +1238,31 @@
15 QDateTime finalDate(dateTime);
16 QTimeZone tz;
17
18- switch (finalDate.timeSpec()) {
19- case Qt::UTC:
20- case Qt::OffsetFromUTC:
21- // convert date to UTC timezone
22- tz = QTimeZone("UTC");
23- finalDate = finalDate.toTimeZone(tz);
24- break;
25- case Qt::TimeZone:
26- tz = finalDate.timeZone();
27- if (!tz.isValid()) {
28- // floating time
29- finalDate = QDateTime(finalDate.date(), finalDate.time(), Qt::UTC);
30+ if (!allDay) {
31+ switch (finalDate.timeSpec()) {
32+ case Qt::UTC:
33+ case Qt::OffsetFromUTC:
34+ // convert date to UTC timezone
35+ tz = QTimeZone("UTC");
36+ finalDate = finalDate.toTimeZone(tz);
37+ break;
38+ case Qt::TimeZone:
39+ tz = finalDate.timeZone();
40+ if (!tz.isValid()) {
41+ // floating time
42+ finalDate = QDateTime(finalDate.date(), finalDate.time(), Qt::UTC);
43+ } else {
44+ tz = QTimeZone("UTC");
45+ finalDate = finalDate.toTimeZone(tz);
46+ }
47+ break;
48+ case Qt::LocalTime:
49+ tz = QTimeZone(QTimeZone::systemTimeZoneId());
50+ finalDate = finalDate.toTimeZone(tz);
51+ break;
52+ default:
53+ break;
54 }
55- break;
56- case Qt::LocalTime:
57- tz = QTimeZone(QTimeZone::systemTimeZoneId());
58- finalDate = finalDate.toTimeZone(tz);
59- break;
60- default:
61- break;
62 }
63
64 if (tz.isValid()) {
65
66=== modified file 'tests/unittest/event-test.cpp'
67--- tests/unittest/event-test.cpp 2015-04-13 21:30:00 +0000
68+++ tests/unittest/event-test.cpp 2015-08-27 02:06:04 +0000
69@@ -498,15 +498,16 @@
70 QCOMPARE(errorMap[1], QOrganizerManager::InvalidCollectionError);
71 }
72
73- void testCreateAllDayEvent()
74+ void testCreateAllDayTodo()
75 {
76 static QString displayLabelValue = QStringLiteral("All day title");
77 static QString descriptionValue = QStringLiteral("All day description");
78
79+ QDateTime eventDateTime = QDateTime(QDate(2013, 9, 3), QTime(0,30,0));
80 QOrganizerTodo todo;
81 todo.setCollectionId(m_collection.id());
82 todo.setAllDay(true);
83- todo.setStartDateTime(QDateTime(QDate(2013, 9, 3), QTime(0,30,0)));
84+ todo.setStartDateTime(eventDateTime);
85 todo.setDisplayLabel(displayLabelValue);
86 todo.setDescription(descriptionValue);
87
88@@ -532,7 +533,51 @@
89
90 QOrganizerTodo todoResult = static_cast<QOrganizerTodo>(items[0]);
91 QCOMPARE(todoResult.isAllDay(), true);
92- }
93+ QCOMPARE(todoResult.startDateTime().date(), eventDateTime.date());
94+ QCOMPARE(todoResult.startDateTime().time(), QTime(0, 0, 0));
95+ }
96+
97+ void testCreateAllDayEvent()
98+ {
99+ static QString displayLabelValue = QStringLiteral("All day title");
100+ static QString descriptionValue = QStringLiteral("All day description");
101+
102+ QDateTime eventDateTime = QDateTime(QDate(2013, 9, 3), QTime(0,30,0));
103+ QOrganizerEvent event;
104+ event.setStartDateTime(eventDateTime);
105+ event.setEndDateTime(eventDateTime.addDays(1));
106+ event.setDisplayLabel(displayLabelValue);
107+ event.setDescription(descriptionValue);
108+ event.setAllDay(true);
109+
110+ QtOrganizer::QOrganizerManager::Error error;
111+ QMap<int, QtOrganizer::QOrganizerManager::Error> errorMap;
112+ QList<QOrganizerItem> items;
113+ items << event;
114+ bool saveResult = m_engine->saveItems(&items,
115+ QList<QtOrganizer::QOrganizerItemDetail::DetailType>(),
116+ &errorMap,
117+ &error);
118+ QVERIFY(saveResult);
119+ QCOMPARE(error, QOrganizerManager::NoError);
120+ QVERIFY(errorMap.isEmpty());
121+ QOrganizerItemId id = items[0].id();
122+ QVERIFY(!id.isNull());
123+
124+ QOrganizerItemFetchHint hint;
125+ QList<QOrganizerItemId> ids;
126+ ids << items[0].id();
127+ items = m_engine->items(ids, hint, &errorMap, &error);
128+ QCOMPARE(items.count(), 1);
129+
130+ QOrganizerEvent eventResult = static_cast<QOrganizerEvent>(items[0]);
131+ QCOMPARE(eventResult.isAllDay(), true);
132+ QCOMPARE(eventResult.startDateTime().date(), eventDateTime.date());
133+ QCOMPARE(eventResult.startDateTime().time(), QTime(0, 0, 0));
134+ QCOMPARE(eventResult.endDateTime().date(), eventDateTime.date().addDays(1));
135+ QCOMPARE(eventResult.endDateTime().time(), QTime(0, 0, 0));
136+ }
137+
138
139 void testCreateTodoEventWithStartDate()
140 {

Subscribers

People subscribed via source and target branches

to all changes: