Merge lp:~renatofilho/indicator-datetime/fix-1533681 into lp:indicator-datetime/15.10

[Renato Araujo Oliveira Filho]

Only play a sound alert if the event contains a SOUND reminder.

How to Test:

1 - Create an alarm on google wit a pop-up notification at event time
2 - Create an alarm on google with e-mail notification at event time
3 - Create an alarm on google without any notification at event time
5 - Create a event with a reminder at event time on your device
4 - Sync your device with google
5 - Make sure that the event with pop-up notification does not play any sound but shows a pop up when it starts
6 - Make sure that the event with e-mail notification does not show any notification and does not play any sound
7 - Make sure the event created on device shows a notification and play a sound when it starts.

[Charles Kerr (charlesk)]

Basically the right idea, but implementation is much more complicated than necessary. See comments inline

review: Needs Fixing

[lp:~renatofilho/indicator-datetime/fix-1533681]

449. By Renato Araujo Oliveira Filho on 2016-03-31

Remove type property from Alarm.

450. By Renato Araujo Oliveira Filho on 2016-03-31

Update tests.

451. By Renato Araujo Oliveira Filho on 2016-04-01

Does not play sound for events without a sound set.

452. By Renato Araujo Oliveira Filho on 2016-04-01

Parent merged.

453. By Renato Araujo Oliveira Filho on 2016-04-01

better code.

454. By Renato Araujo Oliveira Filho on 2016-04-01

Update tests.

[Arthur Mello (artmello)]

lgtm

review: Approve

[Charles Kerr (charlesk)]

to me, too

review: Approve

[lp:~renatofilho/indicator-datetime/fix-1533681]

455. By Renato Araujo Oliveira Filho on 2016-04-01

Parent merged.

456. By Renato Araujo Oliveira Filho on 2016-04-06

Parent merged.

457. By Renato Araujo Oliveira Filho on 2016-04-06

Fixed sound file used by event alarms.

458. By Renato Araujo Oliveira Filho on 2016-04-14

Play event sound even if the event has only text reminders.

459. By Renato Araujo Oliveira Filho on 2016-04-15

revert last change.

460. By Renato Araujo Oliveira Filho on 2016-04-19

Parent merged.

461. By Renato Araujo Oliveira Filho on 2016-04-19

Only play sounds for alarms.

462. By Renato Araujo Oliveira Filho on 2016-04-19

Update sound test.

=== modified file 'include/datetime/appointment.h'
--- include/datetime/appointment.h	2016-03-16 14:42:06 +0000
+++ include/datetime/appointment.h	2016-04-19 16:49:39 +0000
@@ -39,6 +39,8 @@
     DateTime time;
 
     bool operator== (const Alarm& that) const;
+    bool has_sound() const;
+    bool has_text() const;
 };
 
 /**
 
=== modified file 'src/appointment.cpp'
--- src/appointment.cpp	2015-04-05 22:27:52 +0000
+++ src/appointment.cpp	2016-04-19 16:49:39 +0000
@@ -31,7 +31,17 @@
 {
   return (text==that.text)
       && (audio_url==that.audio_url)
-      && (this->time==that.time);
+          && (this->time==that.time);
+}
+
+bool Alarm::has_sound() const
+{
+    return !audio_url.empty();
+}
+
+bool Alarm::has_text() const
+{
+    return !text.empty();
 }
 
 bool Appointment::operator==(const Appointment& that) const
 
=== modified file 'src/engine-eds.cpp'
--- src/engine-eds.cpp	2016-04-19 16:49:39 +0000
+++ src/engine-eds.cpp	2016-04-19 16:49:39 +0000
@@ -603,7 +603,7 @@
         return ret;
     }
 
-    static std::string get_alarm_sound_url(ECalComponentAlarm * alarm)
+    static std::string get_alarm_sound_url(ECalComponentAlarm * alarm, const std::string & default_sound)
     {
         std::string ret;
 
@@ -624,6 +624,8 @@
 
                 icalattach_unref(attach);
             }
+            if (ret.empty())
+                ret = default_sound;
         }
 
         return ret;
@@ -940,11 +942,14 @@
                                                 DateTime{gtz, ai->occur_end});
             auto trigger_time = DateTime{gtz, ai->trigger};
             auto& alarm = alarms[instance_time][trigger_time];
-
             if (alarm.text.empty())
                 alarm.text = get_alarm_text(a);
+
             if (alarm.audio_url.empty())
-                alarm.audio_url = get_alarm_sound_url(a);
+                alarm.audio_url = get_alarm_sound_url(a,  (baseline.is_ubuntu_alarm() ?
+                                                         "file://" ALARM_DEFAULT_SOUND :
+                                                         "file://" CALENDAR_DEFAULT_SOUND));
+
             if (!alarm.time.is_set())
                 alarm.time = trigger_time;
 
@@ -958,7 +963,10 @@
             appointment.end = i.first.second;
             appointment.alarms.reserve(i.second.size());
             for (auto& j : i.second)
-                appointment.alarms.push_back(j.second);
+            {
+                if (j.second.has_text() || j.second.has_sound())
+                    appointment.alarms.push_back(j.second);
+            }
             subtask->task->appointments.push_back(appointment);
         }
     }
 
=== modified file 'src/snap.cpp'
--- src/snap.cpp	2016-02-03 17:06:31 +0000
+++ src/snap.cpp	2016-04-19 16:49:39 +0000
@@ -102,7 +102,9 @@
 
         // calendar events are muted in silent mode; alarm clocks never are
         std::shared_ptr<uin::Sound> sound;
-        if (appointment.is_ubuntu_alarm() || !silent_mode()) {
+        // FIXME: only play sounds for alarms for now, we should itegrate it with
+        // system settings to decide if we should play sounds for calendar notification or not
+        if (appointment.is_ubuntu_alarm()) {
             // create the sound.
             const auto role = appointment.is_ubuntu_alarm() ? "alarm" : "alert";
             const auto uri = get_alarm_uri(appointment, alarm, m_settings);
 
=== modified file 'tests/notification-fixture.h'
--- tests/notification-fixture.h	2016-02-10 20:48:24 +0000
+++ tests/notification-fixture.h	2016-04-19 16:49:39 +0000
@@ -53,7 +53,7 @@
   static constexpr char const * HAPTIC_METHOD_VIBRATE_PATTERN {"VibratePattern"};
 
   static constexpr int SCREEN_COOKIE {8675309};
-  static constexpr char const * SCREEN_METHOD_KEEP_DISPLAY_ON {"keepDisplayOn"}; 
+  static constexpr char const * SCREEN_METHOD_KEEP_DISPLAY_ON {"keepDisplayOn"};
   static constexpr char const * SCREEN_METHOD_REMOVE_DISPLAY_ON_REQUEST {"removeDisplayOnRequest"};
 
   static constexpr int POWERD_SYS_STATE_ACTIVE = 1;
@@ -111,7 +111,7 @@
     const auto christmas = unity::indicator::datetime::DateTime::Local(2015,12,25,0,0,0);
     appt.begin = christmas.start_of_day();
     appt.end = christmas.end_of_day();
-    appt.alarms.push_back(unity::indicator::datetime::Alarm{"Ho Ho Ho!", "", appt.begin});
+    appt.alarms.push_back(unity::indicator::datetime::Alarm{"Ho Ho Ho!", CALENDAR_DEFAULT_SOUND, appt.begin});
 
     // init an Ubuntu Alarm
     ualarm.color = "red";
@@ -165,8 +165,8 @@
                                                 NOTIFY_INTERFACE,
                                                 &error);
     g_assert_no_error(error);
-   
-    // METHOD_GET_INFO 
+
+    // METHOD_GET_INFO
     str = g_strdup("ret = ('mock-notify', 'test vendor', '1.0', '1.1')");
     dbus_test_dbus_mock_object_add_method(notify_mock,
                                           notify_obj,
@@ -196,7 +196,7 @@
     g_assert_no_error (error);
     g_free (str);
 
-    // METHOD_CLOSE 
+    // METHOD_CLOSE
     str = g_strdup_printf("self.EmitSignal('%s', '%s', 'uu', [ args[0], %d ])",
                           NOTIFY_INTERFACE,
                           SIGNAL_CLOSED,
@@ -223,8 +223,8 @@
                                                 BUS_POWERD_INTERFACE,
                                                 &error);
     g_assert_no_error(error);
-   
-    str = g_strdup_printf ("ret = '%s'", POWERD_COOKIE); 
+
+    str = g_strdup_printf ("ret = '%s'", POWERD_COOKIE);
     dbus_test_dbus_mock_object_add_method(powerd_mock,
                                           powerd_obj,
                                           POWERD_METHOD_REQUEST_SYS_STATE,
@@ -256,8 +256,8 @@
                                                 BUS_SCREEN_INTERFACE,
                                                 &error);
     g_assert_no_error(error);
-   
-    str = g_strdup_printf ("ret = %d", SCREEN_COOKIE); 
+
+    str = g_strdup_printf ("ret = %d", SCREEN_COOKIE);
     dbus_test_dbus_mock_object_add_method(screen_mock,
                                           screen_obj,
                                           SCREEN_METHOD_KEEP_DISPLAY_ON,
@@ -287,7 +287,7 @@
                                                 BUS_HAPTIC_PATH,
                                                 BUS_HAPTIC_INTERFACE,
                                                 &error);
-   
+
     dbus_test_dbus_mock_object_add_method(haptic_mock,
                                           haptic_obj,
                                           HAPTIC_METHOD_VIBRATE_PATTERN,
 
=== modified file 'tests/test-eds-ics-repeating-valarms.cpp'
--- tests/test-eds-ics-repeating-valarms.cpp	2016-04-19 16:49:39 +0000
+++ tests/test-eds-ics-repeating-valarms.cpp	2016-04-19 16:49:39 +0000
@@ -73,14 +73,14 @@
     ASSERT_EQ(1, appts.size());
     const auto& appt = appts.front();
     ASSERT_EQ(8, appt.alarms.size());
-    EXPECT_EQ(Alarm({"Time to pack!",      "", DateTime(gtz,2015,4,23,13,35,0)}), appt.alarms[0]);
-    EXPECT_EQ(Alarm({"Time to pack!",      "", DateTime(gtz,2015,4,23,13,37,0)}), appt.alarms[1]);
-    EXPECT_EQ(Alarm({"Time to pack!",      "", DateTime(gtz,2015,4,23,13,39,0)}), appt.alarms[2]);
-    EXPECT_EQ(Alarm({"Time to pack!",      "", DateTime(gtz,2015,4,23,13,41,0)}), appt.alarms[3]);
-    EXPECT_EQ(Alarm({"Go to the airport!", "", DateTime(gtz,2015,4,24,10,35,0)}), appt.alarms[4]);
-    EXPECT_EQ(Alarm({"Go to the airport!", "", DateTime(gtz,2015,4,24,10,37,0)}), appt.alarms[5]);
-    EXPECT_EQ(Alarm({"Go to the airport!", "", DateTime(gtz,2015,4,24,10,39,0)}), appt.alarms[6]);
-    EXPECT_EQ(Alarm({"Go to the airport!", "", DateTime(gtz,2015,4,24,10,41,0)}), appt.alarms[7]);
+    EXPECT_EQ(Alarm({"Time to pack!",      "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,23,13,35,0)}), appt.alarms[0]);
+    EXPECT_EQ(Alarm({"Time to pack!",      "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,23,13,37,0)}), appt.alarms[1]);
+    EXPECT_EQ(Alarm({"Time to pack!",      "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,23,13,39,0)}), appt.alarms[2]);
+    EXPECT_EQ(Alarm({"Time to pack!",      "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,23,13,41,0)}), appt.alarms[3]);
+    EXPECT_EQ(Alarm({"Go to the airport!", "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,24,10,35,0)}), appt.alarms[4]);
+    EXPECT_EQ(Alarm({"Go to the airport!", "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,24,10,37,0)}), appt.alarms[5]);
+    EXPECT_EQ(Alarm({"Go to the airport!", "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,24,10,39,0)}), appt.alarms[6]);
+    EXPECT_EQ(Alarm({"Go to the airport!", "file://" CALENDAR_DEFAULT_SOUND, DateTime(gtz,2015,4,24,10,41,0)}), appt.alarms[7]);
 
     // now let's try this out with AlarmQueue...
     // hook the planner up to a SimpleAlarmQueue and confirm that it triggers for each of the reminders
 
=== modified file 'tests/test-sound.cpp'
--- tests/test-sound.cpp	2016-02-10 20:49:01 +0000
+++ tests/test-sound.cpp	2016-04-19 16:49:39 +0000
@@ -155,8 +155,9 @@
       std::string expected_role;
       std::string expected_uri;
   } test_cases[] = {
-      { ualarm, "alarm", path_to_uri(ALARM_DEFAULT_SOUND) },
-      { appt,   "alert", path_to_uri(CALENDAR_DEFAULT_SOUND) }
+      { ualarm, "alarm", path_to_uri(ALARM_DEFAULT_SOUND) }
+    // No sound for appointments
+    //  { appt, "alert", path_to_uri(CALENDAR_DEFAULT_SOUND) }
   };
 
   auto snap = create_snap(ne, sb, settings);