Merge lp:~zsombi/ubuntu-ui-toolkit/alarm-date-fix into lp:ubuntu-ui-toolkit/staging
- alarm-date-fix
- Merge into staging
Status: | Merged |
---|---|
Approved by: | Zsombor Egri |
Approved revision: | 983 |
Merged at revision: | 1010 |
Proposed branch: | lp:~zsombi/ubuntu-ui-toolkit/alarm-date-fix |
Merge into: | lp:ubuntu-ui-toolkit/staging |
Prerequisite: | lp:~zsombi/ubuntu-ui-toolkit/alarm-fetch-fix |
Diff against target: |
137 lines (+50/-6) 3 files modified
modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp (+9/-5) modules/Ubuntu/Components/plugin/alarmmanager_p.h (+8/-0) tests/unit/tst_alarms/tst_alarms.cpp (+33/-1) |
To merge this branch: | bzr merge lp:~zsombi/ubuntu-ui-toolkit/alarm-date-fix |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot | continuous-integration | Approve | |
Cris Dywan | Approve | ||
Review via email:
|
This proposal supersedes a proposal from 2014-03-10.
Commit message
Fix for alarm dates to occur in the same time in all timezones.
Description of the change
Fix for alarm dates to occur in the same time in all timezones.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:976
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nekhelesh Ramananthan (nik90) wrote : Posted in a previous version of this proposal | # |
So on testing this MP using the ubuntu-clock-app, I have the following observations.
1. Alarms created in the local tz are saved and properly triggered at the correct time. The correct time is also displayed in the indicator-datetime. Hence one can say that the bug #1283236 is fixed.
2. If I switch from UTC+1 (netherlands) to UTC (london) using the system settings app and then create an alarm for say 11:00 AM, it gets saved as 10:00 AM in the clock app. The indicator also shows the alarm as 10:00 AM. The alarm does *not* trigger at 10:00 AM however.
3. If I perform observation #2 but restart the phone after changing the locale, then things work as expected meaning an alarm saved at 11:00 AM gets triggered at 11:00 AM, shown in the indicator as 11:00 AM as well.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:977
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nekhelesh Ramananthan (nik90) wrote : Posted in a previous version of this proposal | # |
I tested this MP since it also includes the MP at https:/
1. Bug #1283236 is fixed. Alarms saved in local tz are triggered in local tz.
2. Bug #1283859 is not fixed. Editing and saving alarms (both single and recurring) doesn't update the alarm time shown in the indicator. The alarm also rings at the original time and not at the updated time.
3. Bug #1285958 Recurring alarms can now be saved on sundays :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal | # |
> I tested this MP since it also includes the MP at
> https:/
> fix/+merge/208749. On testing it, here are my observations.
>
> 1. Bug #1283236 is fixed. Alarms saved in local tz are triggered in local tz.
>
> 2. Bug #1283859 is not fixed. Editing and saving alarms (both single and
> recurring) doesn't update the alarm time shown in the indicator. The alarm
> also rings at the original time and not at the updated time.
Removed attach to the bug as it doesn't seem to be related to Alarm API.
>
> 3. Bug #1285958 Recurring alarms can now be saved on sundays :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cris Dywan (kalikiana) wrote : Posted in a previous version of this proposal | # |
Should be a good idea to have a small test case for transcodeDate, even if it's internal, to be sure it works as expected.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal | # |
> Should be a good idea to have a small test case for transcodeDate, even if
> it's internal, to be sure it works as expected.
Test case added.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:979
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Cris Dywan (kalikiana) wrote : Posted in a previous version of this proposal | # |
Thanks!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:982
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
- 983. By Zsombor Egri
-
staging merge
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:983
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
1 | === modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp' |
2 | --- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2014-03-31 15:14:40 +0000 |
3 | +++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2014-04-16 06:42:11 +0000 |
4 | @@ -119,7 +119,7 @@ |
5 | |
6 | int type, days; |
7 | in >> alarm.message >> alarm.date >> alarm.sound >> type >> days >> alarm.enabled; |
8 | - alarm.originalDate = alarm.date; |
9 | + alarm.originalDate = alarm.date = AlarmData::transcodeDate(alarm.date, Qt::LocalTime); |
10 | alarm.type = static_cast<UCAlarm::AlarmType>(type); |
11 | alarm.days = static_cast<UCAlarm::DaysOfWeek>(days); |
12 | |
13 | @@ -148,7 +148,7 @@ |
14 | |
15 | Q_FOREACH(const AlarmData &alarm, alarmList) { |
16 | out << alarm.message |
17 | - << alarm.originalDate |
18 | + << AlarmData::transcodeDate(alarm.originalDate, Qt::UTC) |
19 | << alarm.sound |
20 | << alarm.type |
21 | << alarm.days |
22 | @@ -162,7 +162,7 @@ |
23 | { |
24 | event.setCollectionId(collection.id()); |
25 | event.setAllDay(false); |
26 | - event.setStartDateTime(alarm.date); |
27 | + event.setStartDateTime(AlarmData::transcodeDate(alarm.date, Qt::UTC)); |
28 | event.setDisplayLabel(alarm.message); |
29 | |
30 | if (alarm.enabled) { |
31 | @@ -229,7 +229,7 @@ |
32 | |
33 | alarm.cookie = QVariant::fromValue<QOrganizerItemId>(event.id()); |
34 | alarm.message = event.displayLabel(); |
35 | - alarm.date = AlarmData::normalizeDate(event.startDateTime()); |
36 | + alarm.date = AlarmData::transcodeDate(event.startDateTime().toUTC(), Qt::LocalTime); |
37 | alarm.sound = QUrl(event.description()); |
38 | alarm.originalDate = alarm.date; |
39 | |
40 | @@ -355,6 +355,10 @@ |
41 | endDate = startDate.addDays(8); |
42 | } |
43 | |
44 | + // transcode both dates |
45 | + startDate = AlarmData::transcodeDate(startDate, Qt::UTC); |
46 | + endDate = AlarmData::transcodeDate(endDate, Qt::UTC); |
47 | + |
48 | QList<QOrganizerItem> occurrences = manager->itemOccurrences(event, startDate, endDate, 10); |
49 | // get the first occurrence and use the date from it |
50 | if ((occurrences.length() > 0) && (occurrences[0].type() == QOrganizerItemType::TypeTodoOccurrence)) { |
51 | @@ -364,7 +368,7 @@ |
52 | // check if the date is after the current datetime |
53 | // the first occurrence is the one closest to the currentDate, therefore we can safely |
54 | // set that startDate to the alarm |
55 | - alarm.date = occurrence.startDateTime(); |
56 | + alarm.date = AlarmData::transcodeDate(occurrence.startDateTime().toUTC(), Qt::LocalTime); |
57 | if (alarm.date > currentDate) { |
58 | // we have the proper date set, leave |
59 | break; |
60 | |
61 | === modified file 'modules/Ubuntu/Components/plugin/alarmmanager_p.h' |
62 | --- modules/Ubuntu/Components/plugin/alarmmanager_p.h 2014-02-28 09:02:40 +0000 |
63 | +++ modules/Ubuntu/Components/plugin/alarmmanager_p.h 2014-04-16 06:42:11 +0000 |
64 | @@ -101,6 +101,14 @@ |
65 | return QDateTime(dt.date(), time, dt.timeSpec()); |
66 | } |
67 | |
68 | + // the function normalizes and transcodes the date into UTC/LocalTime equivalent |
69 | + static QDateTime transcodeDate(const QDateTime &dt, Qt::TimeSpec targetSpec) { |
70 | + if (dt.timeSpec() == targetSpec) { |
71 | + return normalizeDate(dt); |
72 | + } |
73 | + return QDateTime(dt.date(), normalizeDate(dt).time(), targetSpec); |
74 | + } |
75 | + |
76 | unsigned int changes; |
77 | QVariant cookie; |
78 | |
79 | |
80 | === modified file 'tests/unit/tst_alarms/tst_alarms.cpp' |
81 | --- tests/unit/tst_alarms/tst_alarms.cpp 2014-04-10 09:48:30 +0000 |
82 | +++ tests/unit/tst_alarms/tst_alarms.cpp 2014-04-16 06:42:11 +0000 |
83 | @@ -31,6 +31,7 @@ |
84 | #include <QtCore/QDebug> |
85 | #include <QtTest/QTest> |
86 | #include <QtTest/QSignalSpy> |
87 | +#include <QtCore/QTimeZone> |
88 | |
89 | class tst_UCAlarms : public QObject |
90 | { |
91 | @@ -126,7 +127,7 @@ |
92 | } |
93 | |
94 | void test_singleShotAlarmPass() { |
95 | - UCAlarm alarm(QDateTime::currentDateTime().addSecs(10), "test_singleShotAlarmPass"); |
96 | + UCAlarm alarm(QDateTime::currentDateTime().addSecs(4), "test_singleShotAlarmPass"); |
97 | alarm.save(); |
98 | waitForRequest(&alarm); |
99 | QCOMPARE(alarm.error(), (int)UCAlarm::NoError); |
100 | @@ -424,6 +425,37 @@ |
101 | QVERIFY(containsAlarm(&nextAlarm)); |
102 | } |
103 | |
104 | + void test_transcodeDate_data() |
105 | + { |
106 | + QTest::addColumn<QDateTime>("date"); |
107 | + QTest::addColumn<int>("srcSpec"); |
108 | + QTest::addColumn<int>("dstSpec"); |
109 | + QTest::addColumn<bool>("xfail"); |
110 | + |
111 | + QDateTime now = QDateTime::currentDateTime(); |
112 | + QTest::newRow("Local to Local") << now << (int)Qt::LocalTime << (int)Qt::LocalTime << true; |
113 | + QTest::newRow("Local to UTC") << now << (int)Qt::LocalTime << (int)Qt::UTC << false; |
114 | + QTest::newRow("UTC to UTC") << now << (int)Qt::UTC << (int)Qt::UTC << true; |
115 | + QTest::newRow("UTC to Local") << now << (int)Qt::UTC << (int)Qt::LocalTime << false; |
116 | + } |
117 | + |
118 | + void test_transcodeDate() |
119 | + { |
120 | + QFETCH(QDateTime, date); |
121 | + QFETCH(int, srcSpec); |
122 | + QFETCH(int, dstSpec); |
123 | + QFETCH(bool, xfail); |
124 | + |
125 | + QDateTime srcDate = AlarmData::normalizeDate(QDateTime(date.date(), date.time(), (Qt::TimeSpec)srcSpec)); |
126 | + QDateTime dstDate(AlarmData::transcodeDate(srcDate, (Qt::TimeSpec)dstSpec)); |
127 | + QCOMPARE(srcDate.date(), dstDate.date()); |
128 | + QCOMPARE(srcDate.time(), dstDate.time()); |
129 | + if (xfail) { |
130 | + QEXPECT_FAIL("", "Similar timespec set, expect fail", Continue); |
131 | + } |
132 | + QVERIFY(srcDate.timeZone() != dstDate.timeZone()); |
133 | + } |
134 | + |
135 | }; |
136 | |
137 | QTEST_MAIN(tst_UCAlarms) |
PASSED: Continuous integration, rev:975 jenkins. qa.ubuntu. com/job/ ubuntu- ui-toolkit- ci/1872/ jenkins. qa.ubuntu. com/job/ generic- mediumtests- trusty/ 3809 jenkins. qa.ubuntu. com/job/ generic- mediumtests- trusty- touch/3394 jenkins. qa.ubuntu. com/job/ ubuntu- ui-toolkit- trusty- amd64-ci/ 820 jenkins. qa.ubuntu. com/job/ ubuntu- ui-toolkit- trusty- armhf-ci/ 820 jenkins. qa.ubuntu. com/job/ ubuntu- ui-toolkit- trusty- armhf-ci/ 820/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ autopilot- testrunner- otto-trusty/ 3346 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- trusty- amd64/3827 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- trusty- amd64/3827/ artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- trusty- armhf/3396 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- trusty- armhf/3396/ artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ generic- mediumtests- runner- mako/5762 s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 4654
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/ubuntu- ui-toolkit- ci/1872/ rebuild
http://