Merge lp:~thisfred/ubuntuone-client/lp-702055 into lp:ubuntuone-client

[Eric Casteleijn]

This adds notifications and messaging on new incoming shares and UDFs.

This adds notifications and messaging on new incoming shares and UDFs.

[Alejandro J. Cura (alecu)]

Great branch!

Two small issues:
 * I think that new_share_available in StatusFrontend should also pass _callback to messaging.show_message, just like new_udf_available
 * I think that StatusFrontend._callback should be named more descriptively.

Otherwise great work :-)

review: Needs Fixing

[Alejandro J. Cura (alecu)]

Also, StatusAggregator.restart_progress_bubble should be called after any send_notification. In fact, I believe we should make an aggregating/updating mechanism (like the one in FileDiscoveryBubble) in an upcoming branch, so if more events are received they are shown on the current notification.

For instance, turning on the computer and finding out 3 new udfs are available. With this branch we'll see three bubbles one after the other, each popping up immediately after the previous is hidden. With an updating mechanism we would only see one bubble, and the message would be updated as soon as the second and third udfs are found.

[Eric Casteleijn (thisfred)]

The reason share_available does not pass the callback, is because it does not need a special callback method that updates the count like new_udf. If no callback is passed in, the default is used, which opens the volumes tab in the control panel and hides the message.

I'll rename the _callback method.

And yes, we probably will want to aggregate the notifications (though maybe not for new shares.) I'll look into that on a new branch.

[Eric Casteleijn (thisfred)]

method renamed in r851

[Alejandro J. Cura (alecu)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~thisfred/ubuntuone-client/lp-702055 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 libraries...

=== modified file 'tests/platform/linux/test_messaging.py'
--- tests/platform/linux/test_messaging.py	2011-02-03 17:50:53 +0000
+++ tests/platform/linux/test_messaging.py	2011-02-09 13:12:13 +0000
@@ -49,6 +49,7 @@
         self.mocker.restore()
         self.mocker.verify()
 
+    # pylint: disable=R0913
     def _show_message_setup(self, message_time=None, message_count=None,
                             icon=None, update_count=None, hide=False):
         """Set up the mocker expectations for show_method."""
@@ -81,12 +82,15 @@
         mock_indicator.set_property("draw-attention", "true")
         mock_server.show()
         if update_count:
-            mock_indicator.set_property("count", str(update_count))
+            mock_indicator.get_property("count")
+            self.mocker.result("1200")
+            mock_indicator.set_property("count", '2500')
         if hide:
             mock_indicator.set_property('draw-attention', 'false')
             mock_indicator.hide()
         mock_indicator.show()
         self.mocker.replay()
+    # pylint: enable=R0913
 
     def test_show_message(self):
         """On message, libnotify receives the proper calls."""
@@ -116,8 +120,9 @@
         """On message count update, libnotify receives the proper calls."""
         self._show_message_setup(message_count='1200', update_count='1300')
         messaging = Messaging()
-        messaging.show_message(FAKE_SENDER, callback, message_count=1200)
-        messaging.update_count(FAKE_SENDER, 1300)
+        indicator = messaging.show_message(
+            FAKE_SENDER, callback, message_count=1200)
+        messaging.update_count(indicator, 1300)
 
     def test_hide_message(self):
         """On message hide, libnotify receives the proper calls."""

=== modified file 'tests/status/test_aggregator.py'
--- tests/status/test_aggregator.py	2011-02-04 02:25:03 +0000
+++ tests/status/test_aggregator.py	2011-02-09 13:12:13 +0000
@@ -25,6 +25,7 @@
 from ubuntuone.devtools.handlers import MementoHandler
 from ubuntuone.status import aggregator
 from ubuntuone.status.notification import AbstractNotification
+from ubuntuone.status.messaging import AbstractMessaging
 from ubuntuone.syncdaemon import status_listener
 from ubuntuone.syncdaemon.volume_manager import Share, UDF
 
@@ -144,7 +145,7 @@
             notification]
         if new_icon is not None:
             icon = new_icon
-		# we store it as a new notification, to ease testing
+        # we store it as a new notification, to ease testing
         notification_params = (new_title, new_message, icon, append)
         self.notifications_shown.append(notification_params)
 
@@ -160,30 +161,32 @@
     return get_instance
 
 
-class FakeMessaging(object):
+class FakeMessaging(AbstractMessaging):
     """A fake messaging class."""
 
-    def __init__(self, application_name="fake app"):
+    def __init__(self):  # pylint: disable=W0231
         self.messages_shown = {}
-        self.application_name = application_name
+        self.messages_updated = {}
 
-    def show_message(self, sender, callback, message_time=None,
+    # pylint: disable=R0913
+    def show_message(self, sender, callback=None, message_time=None,
                      message_count=None, icon=None):
         """Show a message to the user."""
+        if message_count and sender in self.messages_shown:
+            self.update_count(sender, message_count)
         self.messages_shown[sender] = (
             callback, message_time, message_count, icon)
+    # pylint: enable=R0913
 
-    def update_count(self, sender, new_count):
+    def update_count(self, sender, add_count):
         """Update the count for an existing indicator."""
-        callback, message_time, _, icon = self.messages_shown[sender]
-        self.messages_shown[sender] = (
-            callback, message_time, new_count, icon)
+        self.messages_updated[sender] = (sender, add_count)
 
 
 class FakeStatusAggregator(object):
     """A fake status aggregator."""
 
-    def __init__(self, clock):
+    def __init__(self, clock):          # pylint: disable=W0613
         """Initialize this instance."""
         self.discovered = 0
         self.completed = 0
@@ -523,13 +526,8 @@
 
     def __init__(self, clock):
         """Initialize this fake instance."""
-        self.interesting_events = []
         self.queued_commands = set()
 
-    def interesting_event(self, event):
-        """A new interesting event was found."""
-        self.interesting_events.append(event)
-
     def queue_done(self):
         """The queue completed all operations."""
         self.queued_commands.clear()
@@ -554,23 +552,30 @@
     def setUp(self):
         """Initialize this test instance."""
         self.patch(aggregator, "StatusAggregator", FakeAggregator)
+        self.patch(aggregator, "Notification", FakeNotificationSingleton())
+        self.patch(aggregator, "Messaging", FakeMessaging)
         self.fakefsm = None
         self.fakevm = FakeVolumeManager()
         self.status_frontend = aggregator.StatusFrontend()
         self.listener = status_listener.StatusListener(self.fakefsm,
                                                        self.fakevm,
                                                        self.status_frontend)
+
     def test_file_published(self):
         """A file published event is processed."""
         share_id = "fake share id"
         node_id = "fake node id"
         is_public = True
         public_url = "http://fake_public/url"
-
         self.listener.handle_AQ_CHANGE_PUBLIC_ACCESS_OK(share_id, node_id,
                                                         is_public, public_url)
-        event = self.status_frontend.aggregator.interesting_events[0]
-        self.assertIsInstance(event, aggregator.FilePublishingStatus)
+        self.assertEqual(
+            1, len(self.status_frontend.notification.notifications_shown))
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE,
+             'A file was just made public at http://fake_public/url', None,
+             False),
+            self.status_frontend.notification.notifications_shown[0])
 
     def test_file_unpublished(self):
         """A file unpublished event is processed."""
@@ -581,8 +586,12 @@
 
         self.listener.handle_AQ_CHANGE_PUBLIC_ACCESS_OK(share_id, node_id,
                                                         is_public, public_url)
-        event = self.status_frontend.aggregator.interesting_events[0]
-        self.assertIsInstance(event, aggregator.FileUnpublishingStatus)
+        self.assertEqual(
+            1, len(self.status_frontend.notification.notifications_shown))
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE, 'A file is no longer published', None,
+             False),
+            self.status_frontend.notification.notifications_shown[0])
 
     def test_download_started(self):
         """A download was added to the queue."""
@@ -644,22 +653,77 @@
     def test_new_share_available(self):
         """A new share is available for subscription."""
         SHARE_ID = "fake share id"
-        share = Share(volume_id=SHARE_ID)
+        FAKE_SENDER = 'Mom'
+        share = Share(volume_id=SHARE_ID, other_visible_name=FAKE_SENDER)
         self.fakevm.volumes[SHARE_ID] = share
         self.listener.handle_VM_SHARE_CREATED(SHARE_ID)
-        event = self.status_frontend.aggregator.interesting_events[0]
-        self.assertIsInstance(event, aggregator.ShareAvailableStatus)
+        self.assertEqual(
+            1, len(self.status_frontend.notification.notifications_shown))
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE,
+             'New cloud folder available: <%s> shared by <%s>' % (
+                 'None', FAKE_SENDER), None, False),
+            self.status_frontend.notification.notifications_shown[0])
+        msg = self.status_frontend.messaging.messages_shown[FAKE_SENDER]
+        # msg did not receive a time argument
+        self.assertEqual(None, msg[1])
+        # msg did not receive a count argument
+        self.assertEqual(None, msg[2])
 
     def test_new_udf_available(self):
         """A new udf is available for subscription."""
         udf = UDF()
         self.listener.handle_VM_UDF_CREATED(udf)
-        event = self.status_frontend.aggregator.interesting_events[0]
-        self.assertIsInstance(event, aggregator.UDFAvailableStatus)
+        self.assertEqual(
+            1, len(self.status_frontend.notification.notifications_shown))
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE, 'New cloud folder available: None',
+             None, False),
+            self.status_frontend.notification.notifications_shown[0])
+        self.assertEqual(
+            1, len(self.status_frontend.messaging.messages_shown))
+        self.assertEqual(
+            0, len(self.status_frontend.messaging.messages_updated))
+        msg = self.status_frontend.messaging.messages_shown[
+            aggregator.NEW_UDFS_SENDER]
+        # msg did not receive a time argument
+        self.assertEqual(None, msg[1])
+        # msg did receive a count argument
+        self.assertEqual(1, msg[2])
+
+    def test_two_new_udfs_available(self):
+        """A new udf is available for subscription."""
+        udf1 = UDF()
+        self.listener.handle_VM_UDF_CREATED(udf1)
+        udf2 = UDF()
+        self.listener.handle_VM_UDF_CREATED(udf2)
+        self.assertEqual(
+            2, len(self.status_frontend.notification.notifications_shown))
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE, 'New cloud folder available: None',
+             None, False),
+            self.status_frontend.notification.notifications_shown[0])
+        self.assertEqual(
+            (aggregator.UBUNTUONE_TITLE, 'New cloud folder available: None',
+             None, False),
+            self.status_frontend.notification.notifications_shown[1])
+        self.assertEqual(
+            1, len(self.status_frontend.messaging.messages_shown))
+        self.assertEqual(
+            1, len(self.status_frontend.messaging.messages_updated))
+        msg = self.status_frontend.messaging.messages_shown[
+            aggregator.NEW_UDFS_SENDER]
+        # msg did not receive a time argument
+        self.assertEqual(None, msg[1])
+        # msg did receive a count argument
+        self.assertEqual(1, msg[2])
+        self.assertEqual(
+            (aggregator.NEW_UDFS_SENDER, 1),
+            self.status_frontend.messaging.messages_updated[
+                aggregator.NEW_UDFS_SENDER])
 
 
 class StatusEventTestCase(TestCase):
-
     """Test the status event class and children."""
 
     CLASS = aggregator.StatusEvent
@@ -832,7 +896,6 @@
         self.assertEqual(self.aggregator.done_counter, 0)
         self.assertEqual(len(self.aggregator.files_uploading), 0)
         self.assertEqual(len(self.aggregator.files_downloading), 0)
-        self.assertEqual(len(self.aggregator.interesting_events), 0)
 
     def assertMiscCommandQueued(self, fc):
         """Assert that some command was queued."""

=== modified file 'ubuntuone/platform/linux/messaging.py'
--- ubuntuone/platform/linux/messaging.py	2011-02-02 16:28:49 +0000
+++ ubuntuone/platform/linux/messaging.py	2011-02-09 13:12:13 +0000
@@ -21,6 +21,7 @@
 # of them are available, we should fall back to silently discarding
 # messages.
 
+import subprocess
 from time import time
 
 try:
@@ -35,6 +36,22 @@
 APPLICATION_NAME = 'Ubuntu One Client'
 
 
+def hide_message(indicator):
+    """Remove the message once it has been dealt with."""
+    if USE_INDICATE:
+        indicator.set_property("draw-attention", "false")
+        indicator.hide()
+
+
+# pylint: disable=W0613
+def open_volumes(the_indicator, message_time=None):
+    """Open the control panel to the shares tab."""
+    subprocess.Popen(
+        ['ubuntuone-control-panel-gtk', '--switch-to', 'volumes'])
+    hide_message(the_indicator)
+# pylint: enable=W0613
+
+
 class Messaging(AbstractMessaging):
     """Notification of the end user."""
 
@@ -48,14 +65,18 @@
             self.server.set_desktop_file(
                 "/usr/share/applications/ubuntuone-control-panel-gtk.desktop")
 
-    def show_message(self, sender, callback, message_time=None,
+    # pylint: disable=R0913
+    def show_message(self, sender, callback=None, message_time=None,
                      message_count=None, icon=None):
         """Show a message in the messaging menu."""
         if USE_INDICATE:
-            indicator = self.indicators[sender] = indicate.Indicator()
+            indicator = indicate.Indicator()
             indicator.set_property("subtype", "u1")
             indicator.set_property("name", sender)
             indicator.set_property("sender", sender)
+
+            if callback is None:
+                callback = open_volumes
             indicator.connect("user-display", callback)
 
             if icon is not None:
@@ -70,14 +91,10 @@
             self.server.show()
             indicator.show()
             return indicator
+    # pylint: enable=R0913
 
-    def update_count(self, sender, new_count):
+    def update_count(self, indicator, add_count):
         """Update the count for an existing indicator."""
-        self.indicators[sender].set_property("count", str(new_count))
-
-
-def hide_message(indicator):
-    """Remove the message once it has been dealt with."""
-    if USE_INDICATE:
-        indicator.set_property("draw-attention", "false")
-        indicator.hide()
+        if USE_INDICATE:
+            new_count = int(indicator.get_property('count')) + add_count
+            indicator.set_property('count', str(new_count))

=== modified file 'ubuntuone/platform/windows/messaging.py'
--- ubuntuone/platform/windows/messaging.py	2011-02-01 13:51:48 +0000
+++ ubuntuone/platform/windows/messaging.py	2011-02-09 13:12:13 +0000
@@ -29,16 +29,23 @@
 class Messaging(AbstractMessaging):
     """Notification of the end user."""
 
-    def show_message(self, sender, callback, message_time=None,
+    # pylint: disable=R0913
+    def show_message(self, sender, callback=None, message_time=None,
                      message_count=None, icon=None):
         """Show a message in the messaging menu."""
-        pass
+        # TODO: make this work
+    # pylint: enable=R0913
 
-    def update_count(self, sender, new_count):
+    def update_count(self, sender, add_count):
         """Update the count for an existing indicator."""
-        pass
-
-
-def hide_message(indicator):
+        # TODO: make this work
+
+
+def hide_message(indicator):            # pylint: disable=W0613
     """Remove the message once it has been dealt with."""
-    pass
+    # TODO: make this work
+
+
+def open_volumes(the_indicator, message_time=None):  # pylint: disable=W0613
+    """Open the control panel to the shares tab."""
+    # TODO: make this work

=== modified file 'ubuntuone/status/aggregator.py'
--- ubuntuone/status/aggregator.py	2011-02-04 00:56:30 +0000
+++ ubuntuone/status/aggregator.py	2011-02-09 13:12:13 +0000
@@ -26,7 +26,7 @@
 
 from ubuntuone.logger import basic_formatter, logging
 from ubuntuone.platform.notification import Notification
-from ubuntuone.platform.messaging import Messaging
+from ubuntuone.platform.messaging import Messaging, open_volumes
 
 LOG_LEVEL = logging.DEBUG
 logger = logging.getLogger('ubuntuone.status')
@@ -39,6 +39,7 @@
     logger.addHandler(debug_handler)
 
 UBUNTUONE_TITLE = "Ubuntu One"
+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."
@@ -181,6 +182,7 @@
     def cleanup(self):
         """Cleanup this instance."""
 
+
 class FileDiscoveryIdleState(FileDiscoveryBaseState):
     """Waiting for first file to appear."""
 
@@ -406,7 +408,6 @@
         """Initialize this instance."""
         self.clock = clock
         self.reset()
-        self.messaging = Messaging()
 
     def reset(self):
         """Reset all counters and notifications."""
@@ -418,7 +419,6 @@
         self.upload_done = 0
         self.files_uploading = []
         self.files_downloading = []
-        self.interesting_events = []
 
         if self.file_discovery_bubble:
             self.file_discovery_bubble.cleanup()
@@ -433,10 +433,6 @@
             self.final_status_bubble.cleanup()
         self.final_status_bubble = FinalStatusBubble(self)
 
-    def interesting_event(self, event):
-        """A new interesting event was found."""
-        self.interesting_events.append(event)
-
     def get_discovery_message(self):
         """Get the text for the discovery bubble."""
         lines = []
@@ -546,20 +542,26 @@
         self.upload_done += 1
         self.misc_command_unqueued(command)
 
+
 class StatusFrontend(object):
     """Frontend for the status aggregator, used by the StatusListener."""
 
     def __init__(self, clock=reactor):
         """Initialize this instance."""
         self.aggregator = StatusAggregator(clock=clock)
+        self.notification = Notification()
+        self.messaging = Messaging()
+        self.udf_message = None
 
     def file_published(self, public_url):
         """A file was published."""
-        self.aggregator.interesting_event(FilePublishingStatus(public_url))
+        self.notification.send_notification(
+            UBUNTUONE_TITLE, FilePublishingStatus(public_url).one())
 
-    def file_unpublished(self, public_url):
+    def file_unpublished(self, public_url):  # pylint: disable=W0613
         """A file was unpublished."""
-        self.aggregator.interesting_event(FileUnpublishingStatus(public_url))
+        self.notification.send_notification(
+            UBUNTUONE_TITLE, FileUnpublishingStatus().one())
 
     def download_started(self, command):
         """A file was queued for download."""
@@ -591,8 +593,22 @@
 
     def new_share_available(self, share):
         """A new share is available for subscription."""
-        self.aggregator.interesting_event(ShareAvailableStatus(share))
+        self.messaging.show_message(share.other_visible_name)
+        self.notification.send_notification(
+            UBUNTUONE_TITLE, ShareAvailableStatus(share).one())
 
     def new_udf_available(self, udf):
         """A new udf is available for subscription."""
-        self.aggregator.interesting_event(UDFAvailableStatus(udf))
+        if self.udf_message is not None:
+            self.messaging.update_count(self.udf_message, 1)
+            return
+        self.udf_message = self.messaging.show_message(
+            NEW_UDFS_SENDER, callback=self._reset_udf_message_callback,
+            message_count=1)
+        self.notification.send_notification(
+            UBUNTUONE_TITLE, UDFAvailableStatus(udf).one())
+
+    def _reset_udf_message_callback(self, indicator, message_time=None):
+        """A callback wrapper that resets the udf_message to None."""
+        self.udf_message = None
+        open_volumes(indicator, message_time=message_time)

=== modified file 'ubuntuone/status/messaging.py'
--- ubuntuone/status/messaging.py	2011-02-01 19:07:16 +0000
+++ ubuntuone/status/messaging.py	2011-02-09 13:12:13 +0000
@@ -27,11 +27,13 @@
 
     __metaclass__ = ABCMeta
 
+    # pylint: disable=R0913
     @abstractmethod
-    def show_message(self, sender, callback, message_time=None,
+    def show_message(self, sender, callback=None, message_time=None,
                      message_count=None, icon=None):
         """Show a message in the messaging menu."""
+    # pylint: enable=R0913
 
     @abstractmethod
-    def update_count(self, sender, new_count):
+    def update_count(self, sender, add_count):
         """Update the count for an existing indicator."""