Ubuntu UI Toolkit

Merge lp:~zsombi/ubuntu-ui-toolkit/30-alarm-update-fix into lp:ubuntu-ui-toolkit/staging

  1. 30-alarm-update-fix
  2. Merge into staging
Proposed by Zsombor Egri
Status: Merged
Approved by: Cris Dywan
Approved revision: 1208
Merged at revision: 1210
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/30-alarm-update-fix
Merge into: lp:ubuntu-ui-toolkit/staging
Prerequisite: lp:~zsombi/ubuntu-ui-toolkit/20-alarm-model-update
Diff against target: 179 lines (+75/-39)
3 files modified
modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp (+38/-38)
modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h (+0/-1)
tests/unit/tst_alarms/tst_alarms.cpp (+37/-0)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/30-alarm-update-fix
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Nekhelesh Ramananthan Pending
Review via email: mp+231882@code.launchpad.net

Commit message

Fixing alarm disabling issue.

Change organiser item fields only if those were marked as changed.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1207
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/892/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/724
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/724
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/724/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/724

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/892/rebuild

review: Approve (continuous-integration)
lp:~zsombi/ubuntu-ui-toolkit/30-alarm-update-fix updated
1208. By Zsombor Egri

prereq merge

Revision history for this message
Cris Dywan (kalikiana) wrote :

I guess this is a race condition in the backend? If so maybe it's worth having a little comment pointing this out?

Other than that it seems sensible.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1208
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/895/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/727
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/727
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/727/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/727

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/895/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

erm I was testing this to see if it causes any regressions in the clock app but it seems it got top approved before I could complete?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp'
--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2014-08-22 14:49:12 +0000
+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_organizer.cpp 2014-08-22 14:49:12 +0000
@@ -172,26 +172,45 @@
172{172{
173 event.setCollectionId(collection.id());173 event.setCollectionId(collection.id());
174 event.setAllDay(false);174 event.setAllDay(false);
175 event.setStartDateTime(AlarmData::transcodeDate(alarm.date, Qt::UTC));175 if (alarm.changes & AlarmData::Date) {
176 event.setDisplayLabel(alarm.message);176 event.setStartDateTime(AlarmData::transcodeDate(alarm.date, Qt::UTC));
177177 }
178 if (alarm.enabled) {178 if (alarm.changes & AlarmData::Message) {
179 // set visual and audible reminder serving as alarm note179 event.setDisplayLabel(alarm.message);
180 QOrganizerItemVisualReminder visual;180 }
181 visual.setSecondsBeforeStart(0);181
182 visual.setMessage(alarm.message);182 if (alarm.changes & AlarmData::Enabled) {
183 event.saveDetail(&visual);183 QOrganizerItemVisualReminder visual = event.detail(QOrganizerItemDetail::TypeVisualReminder);
184184 QOrganizerItemAudibleReminder audible = event.detail(QOrganizerItemDetail::TypeAudibleReminder);
185 QOrganizerItemAudibleReminder audible;185
186 audible.setSecondsBeforeStart(0);186 if (alarm.enabled) {
187 audible.setDataUrl(alarm.sound);187 if (visual.isEmpty()) {
188 event.saveDetail(&audible);188 visual.setSecondsBeforeStart(0);
189 visual.setMessage(alarm.message);
190 event.saveDetail(&visual);
191 }
192 if (audible.isEmpty()) {
193 audible.setSecondsBeforeStart(0);
194 audible.setDataUrl(alarm.sound);
195 event.saveDetail(&audible);
196 }
197 } else {
198 event.removeDetail(&visual);
199 event.removeDetail(&audible);
200 }
189 }201 }
190202
191 // save the sound as description as the audible reminder may be off203 // save the sound as description as the audible reminder may be off
192 event.setDescription(alarm.sound.toString());204 if (alarm.changes && AlarmData::Sound) {
205 event.setDescription(alarm.sound.toString());
206 }
193207
194 // set repeating208 // set repeating, reset recurrence no matter if we had it or not
209 if (((alarm.changes & AlarmData::Type) == AlarmData::Type)
210 || ((alarm.changes & AlarmData::Days) == AlarmData::Days)) {
211 QOrganizerItemRecurrence old = event.detail(QOrganizerItemDetail::TypeRecurrence);
212 event.removeDetail(&old);
213 }
195 switch (alarm.type) {214 switch (alarm.type) {
196 case UCAlarm::OneTime: {215 case UCAlarm::OneTime: {
197 break;216 break;
@@ -212,25 +231,6 @@
212 }231 }
213}232}
214233
215void AlarmsAdapter::updateOrganizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event)
216{
217 // remove affected details
218 if (!alarm.enabled || (alarm.changes & AlarmData::Enabled)) {
219 // remove previously set reminders
220 QOrganizerItemVisualReminder visual = event.detail(QOrganizerItemDetail::TypeVisualReminder);
221 event.removeDetail(&visual);
222 QOrganizerItemAudibleReminder audible = event.detail(QOrganizerItemDetail::TypeAudibleReminder);
223 event.removeDetail(&audible);
224 }
225 if (((alarm.changes & AlarmData::Type) == AlarmData::Type)
226 || ((alarm.changes & AlarmData::Days) == AlarmData::Days)) {
227 QOrganizerItemRecurrence old = event.detail(QOrganizerItemDetail::TypeRecurrence);
228 event.removeDetail(&old);
229 }
230
231 organizerEventFromAlarmData(alarm, event);
232}
233
234int AlarmsAdapter::alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm)234int AlarmsAdapter::alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm)
235{235{
236 if (event.isEmpty()) {236 if (event.isEmpty()) {
@@ -452,8 +452,8 @@
452 QOrganizerTodo event;452 QOrganizerTodo event;
453453
454 if (!alarm.cookie.isValid()) {454 if (!alarm.cookie.isValid()) {
455 // new event455 // new event, mark all fields dirty
456 AlarmsAdapter::get()->organizerEventFromAlarmData(alarm, event);456 alarm.changes = AlarmData::AllFields;
457 } else {457 } else {
458 // update existing event458 // update existing event
459 QOrganizerItemId itemId = alarm.cookie.value<QOrganizerItemId>();459 QOrganizerItemId itemId = alarm.cookie.value<QOrganizerItemId>();
@@ -462,8 +462,8 @@
462 setStatus(AlarmRequest::Saving, AlarmRequest::Fail, UCAlarm::AdaptationError);462 setStatus(AlarmRequest::Saving, AlarmRequest::Fail, UCAlarm::AdaptationError);
463 return false;463 return false;
464 }464 }
465 AlarmsAdapter::get()->updateOrganizerEventFromAlarmData(alarm, event);
466 }465 }
466 AlarmsAdapter::get()->organizerEventFromAlarmData(alarm, event);
467467
468 QOrganizerItemSaveRequest *operation = new QOrganizerItemSaveRequest(q_ptr);468 QOrganizerItemSaveRequest *operation = new QOrganizerItemSaveRequest(q_ptr);
469 operation->setManager(AlarmsAdapter::get()->manager);469 operation->setManager(AlarmsAdapter::get()->manager);
470470
=== modified file 'modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h'
--- modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h 2014-08-22 14:49:12 +0000
+++ modules/Ubuntu/Components/plugin/adapters/alarmsadapter_p.h 2014-08-22 14:49:12 +0000
@@ -85,7 +85,6 @@
85 void saveAlarms();85 void saveAlarms();
8686
87 void organizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event);87 void organizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event);
88 void updateOrganizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event);
89 int alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm);88 int alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm);
90 QSet<Qt::DayOfWeek> daysToSet(const AlarmData &alarm) const;89 QSet<Qt::DayOfWeek> daysToSet(const AlarmData &alarm) const;
91 void daysFromSet(AlarmData &alarm, QSet<Qt::DayOfWeek> set);90 void daysFromSet(AlarmData &alarm, QSet<Qt::DayOfWeek> set);
9291
=== modified file 'tests/unit/tst_alarms/tst_alarms.cpp'
--- tests/unit/tst_alarms/tst_alarms.cpp 2014-08-22 14:49:12 +0000
+++ tests/unit/tst_alarms/tst_alarms.cpp 2014-08-22 14:49:12 +0000
@@ -104,6 +104,18 @@
104 return false;104 return false;
105 }105 }
106106
107 AlarmData getAlarmDataFromAlarmCookie(UCAlarm *alarm)
108 {
109 UCAlarmPrivate *pAlarm = UCAlarmPrivate::get(alarm);
110 AlarmList alarms = AlarmManager::instance().alarms();
111 int index = alarms.indexOfAlarm(pAlarm->rawData.cookie);
112 AlarmData data;
113 if (index >= 0) {
114 data = alarms[index];
115 }
116 return data;
117 }
118
107private Q_SLOTS:119private Q_SLOTS:
108120
109 void initTestCase()121 void initTestCase()
@@ -526,6 +538,31 @@
526 QCOMPARE(alarm, saved);538 QCOMPARE(alarm, saved);
527 QCOMPARE(alarm.sound().toString(), saved.sound().toString());539 QCOMPARE(alarm.sound().toString(), saved.sound().toString());
528 }540 }
541
542 // guard bug #1360101
543 void test_create_update_and_disable_alarm() {
544 UCAlarm alarm(QDateTime::currentDateTime(), UCAlarm::AutoDetect, "test_create_update_and_disable_alarm");
545 alarm.setSound(QUrl("file:///usr/share/sounds/ubuntu/ringtones/Celestial.ogg"));
546 alarm.save();
547 waitForRequest(&alarm);
548 QVERIFY(containsAlarm(&alarm));
549
550 // update alarm to occur 1h earlier
551 QDateTime date = alarm.date();
552 date.addSecs(-60);
553 alarm.save();
554 waitForRequest(&alarm);
555 QVERIFY(containsAlarm(&alarm));
556
557 // disable alarm
558 alarm.setEnabled(false);
559 alarm.save();
560 waitForRequest(&alarm);
561 QVERIFY(containsAlarm(&alarm));
562 AlarmData data = getAlarmDataFromAlarmCookie(&alarm);
563 QVERIFY(data.cookie.isValid());
564 QCOMPARE(data.enabled, false);
565 }
529};566};
530567
531QTEST_MAIN(tst_UCAlarms)568QTEST_MAIN(tst_UCAlarms)

Subscribers

People subscribed via source and target branches