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

[Zsombor Egri]

Fixing alarm disabling issue.

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

[PS Jenkins bot (ps-jenkins)]

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]

1208. By Zsombor Egri on 2014-08-22

prereq merge

[Cris Dywan (kalikiana)]

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

[PS Jenkins bot (ps-jenkins)]

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)

[Nekhelesh Ramananthan (nik90)]

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?

=== 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 @@
 {
     event.setCollectionId(collection.id());
     event.setAllDay(false);
-    event.setStartDateTime(AlarmData::transcodeDate(alarm.date, Qt::UTC));
-    event.setDisplayLabel(alarm.message);
-
-    if (alarm.enabled) {
-        // set visual and audible reminder serving as alarm note
-        QOrganizerItemVisualReminder visual;
-        visual.setSecondsBeforeStart(0);
-        visual.setMessage(alarm.message);
-        event.saveDetail(&visual);
-
-        QOrganizerItemAudibleReminder audible;
-        audible.setSecondsBeforeStart(0);
-        audible.setDataUrl(alarm.sound);
-        event.saveDetail(&audible);
+    if (alarm.changes & AlarmData::Date) {
+        event.setStartDateTime(AlarmData::transcodeDate(alarm.date, Qt::UTC));
+    }
+    if (alarm.changes & AlarmData::Message) {
+        event.setDisplayLabel(alarm.message);
+    }
+
+    if (alarm.changes & AlarmData::Enabled) {
+        QOrganizerItemVisualReminder visual = event.detail(QOrganizerItemDetail::TypeVisualReminder);
+        QOrganizerItemAudibleReminder audible = event.detail(QOrganizerItemDetail::TypeAudibleReminder);
+
+        if (alarm.enabled) {
+            if (visual.isEmpty()) {
+                visual.setSecondsBeforeStart(0);
+                visual.setMessage(alarm.message);
+                event.saveDetail(&visual);
+            }
+            if (audible.isEmpty()) {
+                audible.setSecondsBeforeStart(0);
+                audible.setDataUrl(alarm.sound);
+                event.saveDetail(&audible);
+            }
+        } else {
+            event.removeDetail(&visual);
+            event.removeDetail(&audible);
+        }
     }
 
     // save the sound as description as the audible reminder may be off
-    event.setDescription(alarm.sound.toString());
+    if (alarm.changes && AlarmData::Sound) {
+        event.setDescription(alarm.sound.toString());
+    }
 
-    // set repeating
+    // set repeating, reset recurrence no matter if we had it or not
+    if (((alarm.changes & AlarmData::Type) == AlarmData::Type)
+            || ((alarm.changes & AlarmData::Days) == AlarmData::Days)) {
+        QOrganizerItemRecurrence old = event.detail(QOrganizerItemDetail::TypeRecurrence);
+        event.removeDetail(&old);
+    }
     switch (alarm.type) {
     case UCAlarm::OneTime: {
         break;
@@ -212,25 +231,6 @@
     }
 }
 
-void AlarmsAdapter::updateOrganizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event)
-{
-    // remove affected details
-    if (!alarm.enabled || (alarm.changes & AlarmData::Enabled)) {
-        // remove previously set reminders
-        QOrganizerItemVisualReminder visual = event.detail(QOrganizerItemDetail::TypeVisualReminder);
-        event.removeDetail(&visual);
-        QOrganizerItemAudibleReminder audible = event.detail(QOrganizerItemDetail::TypeAudibleReminder);
-        event.removeDetail(&audible);
-    }
-    if (((alarm.changes & AlarmData::Type) == AlarmData::Type)
-            || ((alarm.changes & AlarmData::Days) == AlarmData::Days)) {
-        QOrganizerItemRecurrence old = event.detail(QOrganizerItemDetail::TypeRecurrence);
-        event.removeDetail(&old);
-    }
-
-    organizerEventFromAlarmData(alarm, event);
-}
-
 int AlarmsAdapter::alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm)
 {
     if (event.isEmpty()) {
@@ -452,8 +452,8 @@
     QOrganizerTodo event;
 
     if (!alarm.cookie.isValid()) {
-        // new event
-        AlarmsAdapter::get()->organizerEventFromAlarmData(alarm, event);
+        // new event, mark all fields dirty
+        alarm.changes = AlarmData::AllFields;
     } else {
         // update existing event
         QOrganizerItemId itemId = alarm.cookie.value<QOrganizerItemId>();
@@ -462,8 +462,8 @@
             setStatus(AlarmRequest::Saving, AlarmRequest::Fail, UCAlarm::AdaptationError);
             return false;
         }
-        AlarmsAdapter::get()->updateOrganizerEventFromAlarmData(alarm, event);
     }
+    AlarmsAdapter::get()->organizerEventFromAlarmData(alarm, event);
 
     QOrganizerItemSaveRequest *operation = new QOrganizerItemSaveRequest(q_ptr);
     operation->setManager(AlarmsAdapter::get()->manager);

=== 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 @@
     void saveAlarms();
 
     void organizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event);
-    void updateOrganizerEventFromAlarmData(const AlarmData &alarm, QOrganizerTodo &event);
     int alarmDataFromOrganizerEvent(const QOrganizerTodo &event, AlarmData &alarm);
     QSet<Qt::DayOfWeek> daysToSet(const AlarmData &alarm) const;
     void daysFromSet(AlarmData &alarm, QSet<Qt::DayOfWeek> set);

=== 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 @@
         return false;
     }
 
+    AlarmData getAlarmDataFromAlarmCookie(UCAlarm *alarm)
+    {
+        UCAlarmPrivate *pAlarm = UCAlarmPrivate::get(alarm);
+        AlarmList alarms = AlarmManager::instance().alarms();
+        int index = alarms.indexOfAlarm(pAlarm->rawData.cookie);
+        AlarmData data;
+        if (index >= 0) {
+            data = alarms[index];
+        }
+        return data;
+    }
+
 private Q_SLOTS:
 
     void initTestCase()
@@ -526,6 +538,31 @@
         QCOMPARE(alarm, saved);
         QCOMPARE(alarm.sound().toString(), saved.sound().toString());
     }
+
+    // guard bug #1360101
+    void test_create_update_and_disable_alarm() {
+        UCAlarm alarm(QDateTime::currentDateTime(), UCAlarm::AutoDetect, "test_create_update_and_disable_alarm");
+        alarm.setSound(QUrl("file:///usr/share/sounds/ubuntu/ringtones/Celestial.ogg"));
+        alarm.save();
+        waitForRequest(&alarm);
+        QVERIFY(containsAlarm(&alarm));
+
+        // update alarm to occur 1h earlier
+        QDateTime date = alarm.date();
+        date.addSecs(-60);
+        alarm.save();
+        waitForRequest(&alarm);
+        QVERIFY(containsAlarm(&alarm));
+
+        // disable alarm
+        alarm.setEnabled(false);
+        alarm.save();
+        waitForRequest(&alarm);
+        QVERIFY(containsAlarm(&alarm));
+        AlarmData data = getAlarmDataFromAlarmCookie(&alarm);
+        QVERIFY(data.cookie.isValid());
+        QCOMPARE(data.enabled, false);
+    }
 };
 
 QTEST_MAIN(tst_UCAlarms)