Merge lp:~alecu/ubuntuone-client/config-notifications-2 into lp:ubuntuone-client

[Alejandro J. Cura]

disable notifications according to config, don't show partial count of files done (LP:#717172, LP:#715842, LP:#715887)

disable notifications according to config, don't show partial count of files done (LP:#717172, LP:#715842, LP:#715887)

[Martin Albisetti (beuno)]

review: Approve

[Eric Casteleijn (thisfred)]

Great branch! +1

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~alecu/ubuntuone-client/config-notifications-2 into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
checking for gtk-doc >= 1.0...
  testing gtkdocize... found 1.16
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running gtkdocize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --enable-gtk-doc --enable-debug ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent...

[Ubuntu One Auto Pilot (otto-pilot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'tests/status/test_aggregator.py'
--- tests/status/test_aggregator.py	2011-02-11 22:01:23 +0000
+++ tests/status/test_aggregator.py	2011-02-16 19:09:08 +0000
@@ -132,18 +132,27 @@
     def __init__(self, application_name="fake app"):
         """Initialize this instance."""
         self.notifications_shown = []
+        self.notification_switch = None
         self.application_name = application_name
 
     def send_notification(self, title, message, icon=None, append=False):
         """Show a notification to the user."""
+        if (self.notification_switch is not None
+            and not self.notification_switch.enabled):
+            return
         self.notifications_shown.append((title, message, icon, append))
         return len(self.notifications_shown) - 1
 
-    def update_notification(self, notification, new_title, new_message,
+    def update_notification(self, notification_id, new_title, new_message,
                             new_icon=None):
         """Update the notification with a new message body."""
+        if (self.notification_switch is not None
+            and not self.notification_switch.enabled):
+            return
+        if notification_id is None:
+            return
         title, old_message, icon, append = self.notifications_shown[
-            notification]
+            notification_id]
         if new_icon is not None:
             icon = new_icon
         # we store it as a new notification, to ease testing
@@ -155,8 +164,9 @@
     """Builds a notification singleton, that logs all notifications shown."""
     instance = FakeNotification()
 
-    def get_instance():
+    def get_instance(notification_switch):
         """Returns the single instance."""
+        instance.notification_switch = notification_switch
         return instance
 
     return get_instance
@@ -192,6 +202,7 @@
         self.discovered = 0
         self.completed = 0
         self.restart_progress_bubble_called = False
+        self.notification_switch = aggregator.NotificationSwitch()
 
     def get_discovery_message(self):
         """Return the file discovery message."""
@@ -220,13 +231,122 @@
         """Reset the progress bubble."""
         self.restart_progress_bubble_called = True
 
+    def build_notification(self):
+        """Create a new toggleable notification object."""
+        return self.notification_switch.build_notification()
+
+
+class ToggleableNotificationTestCase(TestCase):
+    """Test the ToggleableNotification class."""
+
+    def setUp(self):
+        """Initialize this test instance."""
+        self.patch(aggregator, "Notification", FakeNotification)
+        self.notification_switch = aggregator.NotificationSwitch()
+        self.toggleable = self.notification_switch.build_notification()
+
+    def assertShown(self, notification):
+        """Assert that the notification was shown."""
+        self.assertIn(notification,
+                      self.toggleable.notification.notifications_shown)
+
+    def assertNotShown(self, notification):
+        """Assert that the notification was shown."""
+        self.assertNotIn(notification,
+                         self.toggleable.notification.notifications_shown)
+
+    def test_send_notification_passes_thru(self):
+        """The send_notification method passes thru."""
+        args = (1, 2, 3, 4)
+        self.toggleable.send_notification(*args)
+        self.assertShown(args)
+
+    def test_update_notification_passes_thru(self):
+        """The update_notification method passes thru."""
+        show_args = ("title", "body", "icon", "append")
+        update_args = ("other title", "other body")
+        expected = update_args + show_args[2:]
+        notification_id = self.toggleable.send_notification(*show_args)
+        self.toggleable.update_notification(notification_id, *update_args)
+        self.assertShown(expected)
+
+    def test_send_notification_honored_when_enabled(self):
+        """The send_notification method is honored when enabled."""
+        self.notification_switch.enable_notifications()
+        args = (aggregator.UBUNTUONE_TITLE, "hello", None, False)
+        self.toggleable.send_notification(*args)
+        self.assertShown(args)
+
+    def test_send_notification_ignored_when_disabled(self):
+        """The send_notification method is ignored when disabled."""
+        self.notification_switch.disable_notifications()
+        args = (aggregator.UBUNTUONE_TITLE, "hello", None, False)
+        self.toggleable.send_notification(*args)
+        self.assertNotShown(args)
+
+    def test_update_notification_honored_when_enabled(self):
+        """The update_notification method is honored when enabled."""
+        self.notification_switch.enable_notifications()
+        show_args = ("title", "body", "icon", "append")
+        update_args = ("other title", "other body")
+        expected = update_args + show_args[2:]
+        notification_id = self.toggleable.send_notification(*show_args)
+        self.toggleable.update_notification(notification_id, *update_args)
+        self.assertShown(expected)
+
+    def test_update_notification_ignored_when_disabled(self):
+        """The update_notification method is ignored when disabled."""
+        self.notification_switch.disable_notifications()
+        show_args = ("title", "body", "icon", "append")
+        update_args = ("other title", "other body")
+        expected = update_args + show_args[2:]
+        notification_id = self.toggleable.send_notification(*show_args)
+        self.toggleable.update_notification(notification_id, *update_args)
+        self.assertNotShown(expected)
+
+    def test_update_after_disabled_show(self):
+        """An update is ignored if the corresponding show was disabled."""
+        self.notification_switch.disable_notifications()
+        show_args = ("title", "body", "icon", "append")
+        update_args = ("other title", "other body")
+        expected = update_args + show_args[2:]
+        notification_id = self.toggleable.send_notification(*show_args)
+        self.notification_switch.enable_notifications()
+        self.toggleable.update_notification(notification_id, *update_args)
+        self.assertNotShown(expected)
+
+
+class NotificationSwitchTestCase(TestCase):
+    """Test the NotificationSwitch class."""
+
+    def setUp(self):
+        """Initialize this test instance."""
+        self.notification_switch = aggregator.NotificationSwitch()
+
+    def test_build_notification(self):
+        """A new notification instance is returned."""
+        notification = self.notification_switch.build_notification()
+        self.assertEqual(notification.notification_switch,
+                         self.notification_switch)
+
+    def test_enable_notifications(self):
+        """The switch is turned on."""
+        self.notification_switch.enable_notifications()
+        self.assertTrue(self.notification_switch.enabled)
+
+    def test_disable_notifications(self):
+        """The switch is turned off."""
+        self.notification_switch.disable_notifications()
+        self.assertFalse(self.notification_switch.enabled)
+
 
 class FileDiscoveryBubbleTestCase(TestCase):
     """Test the FileDiscoveryBubble class."""
 
     def setUp(self):
         """Initialize this test instance."""
-        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
         self.clock = PatchedClock()
         self.aggregator = FakeStatusAggregator(clock=self.clock)
         self.bubble = aggregator.FileDiscoveryBubble(self.aggregator,
@@ -384,7 +504,8 @@
 
     def setUp(self):
         """Initialize this test instance."""
-        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
         self.clock = PatchedClock()
         self.aggregator = FakeStatusAggregator(clock=self.clock)
         self.bubble = aggregator.ProgressBubble(self.aggregator,
@@ -468,7 +589,8 @@
 
     def setUp(self):
         """Initialize this test instance."""
-        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
         self.clock = PatchedClock()
         self.aggregator = FakeStatusAggregator(clock=self.clock)
         self.bubble = aggregator.FinalStatusBubble(self.aggregator)
@@ -611,6 +733,7 @@
     def __init__(self, clock):
         """Initialize this fake instance."""
         self.queued_commands = set()
+        self.notification_switch = aggregator.NotificationSwitch()
 
     def queue_done(self):
         """The queue completed all operations."""
@@ -624,6 +747,10 @@
         """A new command was unqueued."""
         self.queued_commands.discard(command)
 
+    def build_notification(self):
+        """Create a new toggleable notification object."""
+        return self.notification_switch.build_notification()
+
     download_started = misc_command_queued
     download_finished = misc_command_unqueued
     upload_started = misc_command_queued
@@ -637,7 +764,8 @@
         """Initialize this test instance."""
         BaseTwistedTestCase.setUp(self)
         self.patch(aggregator, "StatusAggregator", FakeAggregator)
-        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
         self.patch(aggregator, "Messaging", FakeMessaging)
         self.fakefsm = None
         self.fakevm = FakeVolumeManager()
@@ -827,6 +955,12 @@
              aggregator.ConnectionMadeStatus.MESSAGE_ONE, None, False),
             self.status_frontend.notification.notifications_shown[0])
 
+    def test_set_show_all_notifications(self):
+        """Test the set_show_all_notifications method."""
+        self.status_frontend.set_show_all_notifications(False)
+        self.assertFalse(self.status_frontend.aggregator.
+                         notification_switch.enabled)
+
 
 class StatusEventTestCase(TestCase):
     """Test the status event class and children."""
@@ -1119,8 +1253,8 @@
         self.aggregator.download_total = 8
         self.aggregator.done_counter = 9
         self.aggregator.total_counter = 20
-        expected = (aggregator.PROGRESS_UPLOADED % (5, 10) + " " +
-                    aggregator.PROGRESS_DOWNLOADED % (3, 8) + " " +
+        expected = (aggregator.FILES_UPLOADING % 10 + " " +
+                    aggregator.FILES_DOWNLOADING % 8 + " " +
                     aggregator.PROGRESS_COMPLETED % int(100.0 * 9 / 20))
         result = self.aggregator.get_progress_message()
         self.assertEqual(expected, result)
@@ -1133,7 +1267,7 @@
         self.aggregator.download_total = 8
         self.aggregator.done_counter = 9
         self.aggregator.total_counter = 20
-        expected = (aggregator.PROGRESS_DOWNLOADED % (3, 8) + " " +
+        expected = (aggregator.FILES_DOWNLOADING % 8 + " " +
                     aggregator.PROGRESS_COMPLETED % int(100.0 * 9 / 20))
         result = self.aggregator.get_progress_message()
         self.assertEqual(expected, result)
@@ -1146,7 +1280,7 @@
         self.aggregator.download_total = 0
         self.aggregator.done_counter = 9
         self.aggregator.total_counter = 20
-        expected = (aggregator.PROGRESS_UPLOADED % (5, 10) + " " +
+        expected = (aggregator.FILES_UPLOADING % 10 + " " +
                     aggregator.PROGRESS_COMPLETED % int(100.0 * 9 / 20))
         result = self.aggregator.get_progress_message()
         self.assertEqual(expected, result)
@@ -1189,13 +1323,37 @@
         self.aggregator.restart_progress_bubble()
         self.assertTrue(self.aggregator.progress_bubble.started)
 
-    def test_queue_done(self):
-        """On queue done, show final bubble and reset counters."""
-        fc = FakeCommand()
-        self.status_frontend.upload_started(fc)
-        old_final_bubble = self.aggregator.final_status_bubble
-        self.aggregator.queue_done()
-        self.assertTrue(old_final_bubble.shown)
+    def test_queue_done_shows_bubble_when_downloads_happened(self):
+        """On queue done, show final bubble if downloads happened."""
+        fc = FakeCommand()
+        self.status_frontend.download_started(fc)
+        self.status_frontend.download_finished(fc)
+        old_final_bubble = self.aggregator.final_status_bubble
+        self.aggregator.queue_done()
+        self.assertTrue(old_final_bubble.shown)
+
+    def test_queue_done_shows_bubble_when_uploads_happened(self):
+        """On queue done, show final bubble if uploads happened."""
+        fc = FakeCommand()
+        self.status_frontend.upload_started(fc)
+        self.status_frontend.upload_finished(fc)
+        old_final_bubble = self.aggregator.final_status_bubble
+        self.aggregator.queue_done()
+        self.assertTrue(old_final_bubble.shown)
+
+    def test_queue_done_does_not_show_bubble_when_no_transfers_happened(self):
+        """On queue done, don't show final bubble if no transfers happened."""
+        fc = FakeCommand()
+        self.status_frontend.upload_started(fc)
+        old_final_bubble = self.aggregator.final_status_bubble
+        self.aggregator.queue_done()
+        self.assertFalse(old_final_bubble.shown)
+
+    def test_queue_done_resets_status_and_hides_progressbar(self):
+        """On queue done, reset counters and hide progressbar."""
+        fc = FakeCommand()
+        self.status_frontend.upload_started(fc)
+        self.aggregator.queue_done()
         self.assertStatusReset()
         self.assertEqual(0.0, self.aggregator.progress_bar.percentage)
         self.assertFalse(self.aggregator.progress_bar.visible)
@@ -1229,10 +1387,13 @@
 
     def test_all_together_now(self):
         """Make all parts work together."""
-        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
         clock = PatchedClock()
         upload = FakeCommand()
         sf = aggregator.StatusFrontend(clock=clock)
+        sf.set_show_all_notifications(True)
+
         clock.advance(aggregator.ProgressBubble.sleep_delay)
         # the progress bar is not visible yet
         self.assertFalse(sf.aggregator.progress_bar.visible)
@@ -1276,3 +1437,49 @@
         clock.advance(aggregator.ProgressBubble.sleep_delay * 2)
         # no more notifications are shown
         self.assertEqual(5, len(notifications_shown))
+
+    def test_all_together_now_off(self):
+        """Make all parts work together, but with notifications off."""
+        self.patch(aggregator, "ToggleableNotification",
+                   FakeNotificationSingleton())
+        clock = PatchedClock()
+        upload = FakeCommand()
+        sf = aggregator.StatusFrontend(clock=clock)
+        sf.set_show_all_notifications(False)
+
+        clock.advance(aggregator.ProgressBubble.sleep_delay)
+        # the progress bar is not visible yet
+        self.assertFalse(sf.aggregator.progress_bar.visible)
+        sf.upload_started(upload)
+        # the progress bar is now shown
+        self.assertTrue(sf.aggregator.progress_bar.visible)
+        notifications_shown = (sf.aggregator.file_discovery_bubble.
+                                             notification.notifications_shown)
+        # no notifications shown, never
+        self.assertEqual(0, len(notifications_shown))
+        clock.advance(aggregator.FileDiscoveryGatheringState.initial_delay)
+        self.assertEqual(0, len(notifications_shown))
+        download = FakeCommand()
+        sf.download_started(download)
+        self.assertEqual(0, len(notifications_shown))
+        # the progress still is zero
+        self.assertEqual(0.0, sf.aggregator.progress_bar.percentage)
+        clock.advance(aggregator.FileDiscoveryUpdateState.updates_delay)
+        self.assertEqual(0, len(notifications_shown))
+        clock.advance(aggregator.FileDiscoveryUpdateState.updates_timeout -
+                      aggregator.FileDiscoveryUpdateState.updates_delay)
+        sf.upload_finished(upload)
+        # the progress bubble has no notifications yet
+        self.assertEqual(None, sf.aggregator.progress_bubble.notification)
+        clock.advance(aggregator.ProgressBubble.sleep_delay)
+        self.assertEqual(0, len(notifications_shown))
+        sf.upload_finished(download)
+        # the progress still is now 100%
+        self.assertEqual(100.0, sf.aggregator.progress_bar.percentage)
+        self.assertEqual(0, len(notifications_shown))
+        clock.advance(aggregator.ProgressBubble.sleep_delay)
+        self.assertEqual(0, len(notifications_shown))
+        sf.queue_done()
+        self.assertEqual(0, len(notifications_shown))
+        clock.advance(aggregator.ProgressBubble.sleep_delay * 2)
+        self.assertEqual(0, len(notifications_shown))

=== modified file 'ubuntuone/status/aggregator.py'
--- ubuntuone/status/aggregator.py	2011-02-14 19:16:19 +0000
+++ ubuntuone/status/aggregator.py	2011-02-16 19:09:08 +0000
@@ -39,16 +39,52 @@
     logger.addHandler(debug_handler)
 
 UBUNTUONE_TITLE = "Ubuntu One"
-NEW_UDFS_SENDER = "New Cloud Folder(s) Available"
+NEW_UDFS_SENDER = "New cloud folder(s) available"
 FINAL_COMPLETED = "File synchronization completed."
-FINAL_UPLOADED = "%d files were uploaded to your cloud."
-FINAL_DOWNLOADED = "%d files were downloaded to your computer."
-FILES_UPLOADING = "%d files are uploading to your cloud."
-FILES_DOWNLOADING = "%d files are downloading to your computer."
-PROGRESS_UPLOADED = "Uploaded %d/%d files."
-PROGRESS_DOWNLOADED = "Downloaded %d/%d files."
+FINAL_UPLOADED = "%d file(s) were uploaded to your personal cloud."
+FINAL_DOWNLOADED = "%d file(s) were downloaded to your computer."
+FILES_UPLOADING = "%d file(s) are uploading to your personal cloud."
+FILES_DOWNLOADING = "%d file(s) are downloading to your computer."
+PROGRESS_UPLOADED = "Uploaded %d/%d file(s)."
 PROGRESS_COMPLETED = "%d%% completed."
 
+class ToggleableNotification(object):
+    """A controller for notifications that can be turned off."""
+
+    def __init__(self, notification_switch):
+        """Initialize this instance."""
+        self.notification_switch = notification_switch
+        self.notification = Notification()
+
+    def send_notification(self, *args):
+        """Passthru the notification."""
+        if self.notification_switch.enabled:
+            return self.notification.send_notification(*args)
+
+    def update_notification(self, notification_id, *args):
+        """Passthru the update."""
+        if notification_id is not None and self.notification_switch.enabled:
+            return self.notification.update_notification(notification_id,
+                                                         *args)
+
+
+class NotificationSwitch(object):
+    """A switch that turns notifications on and off."""
+
+    enabled = True
+
+    def build_notification(self):
+        """Return a new notification instance."""
+        return ToggleableNotification(self)
+
+    def enable_notifications(self):
+        """Turn the switch on."""
+        self.enabled = True
+
+    def disable_notifications(self):
+        """Turn the switch off."""
+        self.enabled = False
+
 
 class StatusEvent(object):
     """An event representing a status change."""
@@ -328,7 +364,7 @@
 
     def _start(self):
         """The first file was found, so start gathering."""
-        self.notification = Notification()
+        self.notification = self.status_aggregator.build_notification()
         self._change_state(FileDiscoveryGatheringState)
 
     def _popup(self):
@@ -381,7 +417,7 @@
 
     def _timeout(self, result):
         """Show the bubble."""
-        self.notification = Notification()
+        self.notification = self.status_aggregator.build_notification()
         text = self.status_aggregator.get_progress_message()
         self.notification.send_notification(UBUNTUONE_TITLE, text)
         self.restart()
@@ -453,7 +489,7 @@
 
     def show(self):
         """Show the final status notification."""
-        self.notification = Notification()
+        self.notification = self.status_aggregator.build_notification()
         text = self.status_aggregator.get_final_status_message()
         self.notification.send_notification(UBUNTUONE_TITLE, text)
 
@@ -475,9 +511,14 @@
     def __init__(self, clock=reactor):
         """Initialize this instance."""
         self.clock = clock
+        self.notification_switch = NotificationSwitch()
         self.reset()
         self.progress_bar = ProgressBar(clock=self.clock)
 
+    def build_notification(self):
+        """Create a new toggleable notification object."""
+        return self.notification_switch.build_notification()
+
     def reset(self):
         """Reset all counters and notifications."""
         self.total_counter = 0
@@ -521,11 +562,9 @@
         assert self.total_counter > 0
         parts = []
         if self.upload_total:
-            parts.append(PROGRESS_UPLOADED % (self.upload_done,
-                                              self.upload_total))
+            parts.append(FILES_UPLOADING % self.upload_total)
         if self.download_total:
-            parts.append(PROGRESS_DOWNLOADED % (self.download_done,
-                                                self.download_total))
+            parts.append(FILES_DOWNLOADING % self.download_total)
         progress_percentage = 100.0 * self.done_counter / self.total_counter
         parts.append(PROGRESS_COMPLETED % int(progress_percentage))
         return " ".join(parts)
@@ -546,7 +585,8 @@
 
     def queue_done(self):
         """Show final bubble and reset counters."""
-        self.final_status_bubble.show()
+        if self.upload_done + self.download_done > 0:
+            self.final_status_bubble.show()
         self.progress_bar.completed()
         self.reset()
 
@@ -603,7 +643,7 @@
     def __init__(self, clock=reactor):
         """Initialize this instance."""
         self.aggregator = StatusAggregator(clock=clock)
-        self.notification = Notification()
+        self.notification = self.aggregator.build_notification()
         self.messaging = Messaging()
         self.udf_message = None
 
@@ -681,3 +721,7 @@
 
     def set_show_all_notifications(self, value):
         """Set the flag to show all notifications."""
+        if value:
+            self.aggregator.notification_switch.enable_notifications()
+        else:
+            self.aggregator.notification_switch.disable_notifications()