Merge lp:~nataliabidart/ubuntuone-control-panel/letmeremove into lp:ubuntuone-control-panel

[Natalia Bidart]

- Local device can now be removed (LP: #810662).

[lp:~nataliabidart/ubuntuone-control-panel/letmeremove]

188. By Natalia Bidart on 2011-07-19

Added code to remove local device once removed.

189. By Natalia Bidart on 2011-07-19

Merged trunk in.

[Roberto Alsina (ralsina)]

+1

review: Approve

[Alejandro J. Cura (alecu)]

Clean code, tests pass, works irl. Approved!

review: Approve

=== modified file 'data/qt/device.ui'
--- data/qt/device.ui	2011-07-11 11:19:09 +0000
+++ data/qt/device.ui	2011-07-19 19:17:30 +0000
@@ -24,7 +24,7 @@
    <item>
     <widget class="QLabel" name="device_name_label">
      <property name="text">
-      <string>Another device</string>
+      <string>Local device</string>
      </property>
     </widget>
    </item>
@@ -41,6 +41,13 @@
      </property>
     </spacer>
    </item>
+   <item>
+    <widget class="QPushButton" name="remove_device_button">
+     <property name="text">
+      <string>Delete device</string>
+     </property>
+    </widget>
+   </item>
   </layout>
  </widget>
  <resources>

=== modified file 'data/qt/devices.ui'
--- data/qt/devices.ui	2011-07-11 11:19:09 +0000
+++ data/qt/devices.ui	2011-07-19 19:17:30 +0000
@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>688</width>
-    <height>371</height>
+    <width>325</width>
+    <height>252</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -21,111 +21,75 @@
     <number>0</number>
    </property>
    <item>
-    <widget class="QFrame" name="frame">
-     <property name="frameShape">
-      <enum>QFrame::StyledPanel</enum>
-     </property>
-     <property name="frameShadow">
-      <enum>QFrame::Raised</enum>
-     </property>
-     <layout class="QVBoxLayout" name="verticalLayout_2">
-      <item>
-       <layout class="QHBoxLayout" name="horizontalLayout_2">
-        <item>
-         <layout class="QVBoxLayout" name="local_device_box"/>
-        </item>
-        <item>
-         <spacer name="horizontalSpacer_2">
-          <property name="orientation">
-           <enum>Qt::Horizontal</enum>
-          </property>
-          <property name="sizeHint" stdset="0">
-           <size>
-            <width>40</width>
-            <height>20</height>
-           </size>
-          </property>
-         </spacer>
-        </item>
-        <item>
-         <widget class="QPushButton" name="delete_device_button">
-          <property name="text">
-           <string>Delete device</string>
-          </property>
-         </widget>
-        </item>
-       </layout>
-      </item>
-      <item>
-       <layout class="QHBoxLayout" name="horizontalLayout">
-        <item>
-         <widget class="QLabel" name="other_devices_label">
-          <property name="text">
-           <string>Other devices</string>
-          </property>
-         </widget>
-        </item>
-        <item>
-         <spacer name="horizontalSpacer">
-          <property name="orientation">
-           <enum>Qt::Horizontal</enum>
-          </property>
-          <property name="sizeHint" stdset="0">
-           <size>
-            <width>40</width>
-            <height>20</height>
-           </size>
-          </property>
-         </spacer>
-        </item>
-       </layout>
-      </item>
-      <item>
-       <widget class="QListWidget" name="list_devices">
-        <property name="alternatingRowColors">
-         <bool>true</bool>
-        </property>
-        <property name="iconSize">
-         <size>
-          <width>32</width>
-          <height>32</height>
-         </size>
-        </property>
-        <property name="spacing">
-         <number>0</number>
-        </property>
-        <property name="selectionRectVisible">
-         <bool>false</bool>
-        </property>
-       </widget>
-      </item>
-      <item>
-       <layout class="QHBoxLayout" name="horizontalLayout_3">
-        <item>
-         <widget class="QPushButton" name="manage_devices_button">
-          <property name="sizePolicy">
-           <sizepolicy hsizetype="Fixed" vsizetype="Fixed">
-            <horstretch>0</horstretch>
-            <verstretch>0</verstretch>
-           </sizepolicy>
-          </property>
-          <property name="layoutDirection">
-           <enum>Qt::RightToLeft</enum>
-          </property>
-          <property name="text">
-           <string>Go to the web page to manage your other devices</string>
-          </property>
-          <property name="icon">
-           <iconset resource="images.qrc">
-            <normaloff>:/external_icon_white.png</normaloff>:/external_icon_white.png</iconset>
-          </property>
-         </widget>
-        </item>
-       </layout>
-      </item>
-     </layout>
+    <layout class="QVBoxLayout" name="local_device_box"/>
+   </item>
+   <item>
+    <layout class="QHBoxLayout" name="horizontalLayout">
+     <item>
+      <widget class="QLabel" name="other_devices_label">
+       <property name="text">
+        <string>Other devices</string>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <spacer name="horizontalSpacer">
+       <property name="orientation">
+        <enum>Qt::Horizontal</enum>
+       </property>
+       <property name="sizeHint" stdset="0">
+        <size>
+         <width>40</width>
+         <height>20</height>
+        </size>
+       </property>
+      </spacer>
+     </item>
+    </layout>
+   </item>
+   <item>
+    <widget class="QListWidget" name="list_devices">
+     <property name="alternatingRowColors">
+      <bool>true</bool>
+     </property>
+     <property name="iconSize">
+      <size>
+       <width>32</width>
+       <height>32</height>
+      </size>
+     </property>
+     <property name="spacing">
+      <number>0</number>
+     </property>
+     <property name="selectionRectVisible">
+      <bool>false</bool>
+     </property>
     </widget>
    </item>
+   <item>
+    <layout class="QHBoxLayout" name="horizontalLayout_3">
+     <item>
+      <widget class="QPushButton" name="manage_devices_button">
+       <property name="sizePolicy">
+        <sizepolicy hsizetype="Fixed" vsizetype="Fixed">
+         <horstretch>0</horstretch>
+         <verstretch>0</verstretch>
+        </sizepolicy>
+       </property>
+       <property name="layoutDirection">
+        <enum>Qt::RightToLeft</enum>
+       </property>
+       <property name="text">
+        <string>Go to the web page to manage your other devices</string>
+       </property>
+       <property name="icon">
+        <iconset resource="images.qrc">
+         <normaloff>:/external_icon_white.png</normaloff>:/external_icon_white.png</iconset>
+       </property>
+      </widget>
+     </item>
+    </layout>
+   </item>
   </layout>
  </widget>
  <resources>

=== modified file 'ubuntuone/controlpanel/gui/qt/device.py'
--- ubuntuone/controlpanel/gui/qt/device.py	2011-07-11 18:00:59 +0000
+++ ubuntuone/controlpanel/gui/qt/device.py	2011-07-19 19:17:30 +0000
@@ -18,22 +18,30 @@
 
 """The user interface for the control panel for Ubuntu One."""
 
-from PyQt4 import QtGui
-
+from PyQt4 import QtGui, QtCore
+
+from twisted.internet import defer
+
+from ubuntuone.controlpanel.backend import (
+    DEVICE_TYPE_COMPUTER,
+    DEVICE_TYPE_PHONE,
+)
+from ubuntuone.controlpanel.gui import DEVICE_CONFIRM_REMOVE
 from ubuntuone.controlpanel.gui.qt.ui import device_ui
 
 COMPUTER_ICON = "computer"
 PHONE_ICON = "phone"
 DEFAULT_ICON = COMPUTER_ICON
 
-COMPUTER_TYPE = "Computer"
-PHONE_TYPE = "Phone"
-
 DEVICE_TYPE_TO_ICON_MAP = {
-    COMPUTER_TYPE: COMPUTER_ICON,
-    PHONE_TYPE: PHONE_ICON,
+    DEVICE_TYPE_COMPUTER: COMPUTER_ICON,
+    DEVICE_TYPE_PHONE: PHONE_ICON,
 }
 
+CANCEL = QtGui.QMessageBox.Cancel
+NO = QtGui.QMessageBox.No
+YES = QtGui.QMessageBox.Yes
+
 
 def icon_name_from_type(device_type):
     """Get the icon name for the device."""
@@ -44,11 +52,16 @@
 class DeviceWidget(QtGui.QWidget):
     """The widget for each device in the control panel."""
 
-    def __init__(self, *args):
+    removed = QtCore.pyqtSignal()
+    removeCanceled = QtCore.pyqtSignal()
+
+    def __init__(self, backend, device_id, **kwargs):
         """Initialize the UI of the widget."""
-        QtGui.QWidget.__init__(self, *args)
+        QtGui.QWidget.__init__(self, **kwargs)
         self.ui = device_ui.Ui_Form()
         self.ui.setupUi(self)
+        self.id = device_id
+        self.backend = backend
 
     def update_device_info(self, device_info):
         """Update the device info."""
@@ -57,6 +70,20 @@
         pixmap = QtGui.QPixmap(pixmap_name)
         self.ui.device_icon_label.setPixmap(pixmap)
 
+    @defer.inlineCallbacks
+    @QtCore.pyqtSlot()
+    def on_remove_device_button_clicked(self):
+        """The user wants to remove this device."""
+        msg = DEVICE_CONFIRM_REMOVE
+        buttons = YES | NO
+        response = QtGui.QMessageBox.warning(self, '', msg, buttons, NO)
+
+        if response == YES:
+            yield self.backend.remove_device(device_id=self.id)
+            self.removed.emit()
+        else:
+            self.removeCanceled.emit()
+
 
 def get_device_for_list_widget(device_info):
     """Return a QListWidgetItem representing a device with the proper info."""

=== modified file 'ubuntuone/controlpanel/gui/qt/devices.py'
--- ubuntuone/controlpanel/gui/qt/devices.py	2011-07-10 21:35:02 +0000
+++ ubuntuone/controlpanel/gui/qt/devices.py	2011-07-19 19:17:30 +0000
@@ -81,8 +81,12 @@
 
     def update_local_device(self, device_info):
         """Update the info for the local device."""
-        device_widget = device.DeviceWidget()
+        device_widget = device.DeviceWidget(backend=self.backend,
+                                            device_id=device_info['device_id'])
         device_widget.update_device_info(device_info)
+        f = lambda: self.clear_device_info(self.ui.local_device_box)
+        device_widget.removed.connect(f)
+
         self.ui.local_device_box.addWidget(device_widget)
 
     def create_remote_device(self, device_info):

=== modified file 'ubuntuone/controlpanel/gui/qt/loadingoverlay.py'
--- ubuntuone/controlpanel/gui/qt/loadingoverlay.py	2011-07-12 18:47:13 +0000
+++ ubuntuone/controlpanel/gui/qt/loadingoverlay.py	2011-07-19 19:17:30 +0000
@@ -62,7 +62,8 @@
 
     def eventFilter(self, obj, event):
         """Filter events from Frame content to draw the dot animation."""
-        if obj == self.ui.frm_box and event.type() == QtCore.QEvent.Paint:
+        if getattr(self, 'ui', None) is not None and \
+           obj == self.ui.frm_box and event.type() == QtCore.QEvent.Paint:
             painter = QtGui.QPainter()
             painter.begin(obj)
             painter.setRenderHint(QtGui.QPainter.Antialiasing, True)

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2011-07-13 18:45:43 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2011-07-19 19:17:30 +0000
@@ -40,6 +40,7 @@
     "name": "desktop i5",
     "is_local": False,
     "configurable": False,
+    "device_id": '1258-6854',
 }
 
 SAMPLE_PHONE_INFO = {
@@ -47,6 +48,7 @@
     "name": "nokia 1100",
     "is_local": False,
     "configurable": False,
+    "device_id": '987456-2321',
 }
 
 SAMPLE_DEVICES_INFO = [
@@ -55,6 +57,7 @@
         "name": "toshiba laptop",
         "is_local": True,
         "configurable": False,
+        "device_id": '0000',
     },
     SAMPLE_COMPUTER_INFO,
     SAMPLE_PHONE_INFO,

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_device.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_device.py	2011-07-13 18:45:43 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_device.py	2011-07-19 19:17:30 +0000
@@ -20,9 +20,20 @@
 
 from PyQt4 import QtGui
 
+from twisted.internet import defer
+
 from ubuntuone.controlpanel.gui.qt import device as gui
-from ubuntuone.controlpanel.gui.qt.tests import (BaseTestCase,
-    SAMPLE_COMPUTER_INFO, SAMPLE_PHONE_INFO)
+from ubuntuone.controlpanel.gui.qt.tests import (
+    BaseTestCase,
+    FakedConfirmDialog,
+    FakedControlPanelBackend,
+    SAMPLE_COMPUTER_INFO,
+    SAMPLE_PHONE_INFO,
+)
+
+# Access to a protected member
+# Instance of 'ControlBackend' has no '_called' member
+# pylint: disable=W0212, E1103
 
 
 class DeviceWidgetTestCase(BaseTestCase):
@@ -31,6 +42,17 @@
     innerclass_ui = gui.device_ui
     innerclass_name = "Ui_Form"
     class_ui = gui.DeviceWidget
+    backend = FakedControlPanelBackend()
+    device_id = 'zaraza'
+    kwargs = {'backend': backend, 'device_id': device_id}
+
+    def test_has_id(self):
+        """The device as an id, None by default."""
+        self.assertEqual(self.ui.id, self.device_id)
+
+    def test_has_backend(self):
+        """The device as a backend, None by default."""
+        self.assertIs(self.ui.backend, self.backend)
 
     def test_update_device_info(self):
         """The widget is updated with the info."""
@@ -48,8 +70,8 @@
 
     def test_icon_name_from_type(self):
         """Get the right icon name for a device type."""
-        self.assertIconMatchesType(gui.COMPUTER_TYPE, gui.COMPUTER_ICON)
-        self.assertIconMatchesType(gui.PHONE_TYPE, gui.PHONE_ICON)
+        self.assertIconMatchesType(gui.DEVICE_TYPE_COMPUTER, gui.COMPUTER_ICON)
+        self.assertIconMatchesType(gui.DEVICE_TYPE_PHONE, gui.PHONE_ICON)
         self.assertIconMatchesType("other random type", gui.COMPUTER_ICON)
 
     def _test_update_device_info_sets_right_icon(self, info):
@@ -77,3 +99,66 @@
         info = SAMPLE_PHONE_INFO
         item = gui.get_device_for_list_widget(info)
         self.assertEqual(item.text(), info["name"])
+
+
+class RemoveDeviceTestCase(DeviceWidgetTestCase):
+    """The test suite for the device deletion."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(RemoveDeviceTestCase, self).setUp()
+        FakedConfirmDialog.response = gui.NO
+        FakedConfirmDialog.args = None
+        FakedConfirmDialog.kwargs = None
+        self.patch(gui.QtGui, 'QMessageBox', FakedConfirmDialog)
+
+    def test_remove_device_opens_confirmation_dialog(self):
+        """A confirmation dialog is opened when user clicks 'delete device'."""
+        self.ui.ui.remove_device_button.click()
+
+        msg = gui.DEVICE_CONFIRM_REMOVE
+        buttons = gui.YES | gui.NO
+        self.assertEqual(FakedConfirmDialog.args,
+                         (self.ui, '', msg, buttons, gui.NO))
+        self.assertEqual(FakedConfirmDialog.kwargs, {})
+
+    def test_remove_device_does_not_remove_if_answer_is_no(self):
+        """The device is not removed is answer is No."""
+        FakedConfirmDialog.response = gui.NO
+        self.ui.removed.connect(self._set_called)
+        self.ui.ui.remove_device_button.click()
+
+        self.assertNotIn('remove_device', self.ui.backend._called)
+        self.assertEqual(self._called, False)
+
+    def test_remove_device_does_remove_if_answer_is_yes(self):
+        """The device is removed is answer is Yes."""
+        FakedConfirmDialog.response = gui.YES
+        self.ui.ui.remove_device_button.click()
+
+        self.assert_backend_called('remove_device', device_id=self.device_id)
+
+    @defer.inlineCallbacks
+    def test_remove_device_emits_signal_when_removed(self):
+        """The signal 'removed' is emitted when removed."""
+        d = defer.Deferred()
+
+        def check(device_id):
+            """Fire deferred when the device was removed."""
+            d.callback(device_id)
+
+        FakedConfirmDialog.response = gui.YES
+        self.ui.removed.connect(self._set_called)
+        self.patch(self.ui.backend, 'remove_device', check)
+        self.ui.ui.remove_device_button.click()
+
+        yield d
+        self.assertEqual(self._called, ((), {}))
+
+    def test_remove_device_emits_signal_when_not_removed(self):
+        """The signal 'removeCanceled' is emitted when user cancels removal."""
+        FakedConfirmDialog.response = gui.NO
+        self.ui.removeCanceled.connect(self._set_called)
+        self.ui.ui.remove_device_button.click()
+
+        self.assertEqual(self._called, ((), {}))

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_devices.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_devices.py	2011-07-11 11:19:09 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_devices.py	2011-07-19 19:17:30 +0000
@@ -22,6 +22,7 @@
 
 from ubuntuone.controlpanel.gui.qt import devices as gui
 from ubuntuone.controlpanel.gui.qt.tests import (
+    FakedConfirmDialog,
     SAMPLE_DEVICES_INFO,
 )
 from ubuntuone.controlpanel.gui.qt.tests.test_ubuntuonebin import (
@@ -40,6 +41,7 @@
     def setUp(self):
         yield super(DevicesPanelTestCase, self).setUp()
         self.ui.backend.next_result = SAMPLE_DEVICES_INFO
+        self.patch(gui.QtGui, 'QMessageBox', FakedConfirmDialog)
 
     def test_is_processing_while_asking_info(self):
         """The ui is processing while the contents are loaded."""
@@ -76,9 +78,10 @@
         local, remote = SAMPLE_DEVICES_INFO[0], SAMPLE_DEVICES_INFO[1:]
 
         self.assertEqual(self.ui.ui.local_device_box.count(), 1)
-        local_device = self.ui.ui.local_device_box.itemAt(0)
-        self.assertEqual(local_device.widget().ui.device_name_label.text(),
+        local_device = self.ui.ui.local_device_box.itemAt(0).widget()
+        self.assertEqual(local_device.ui.device_name_label.text(),
                          local['name'])
+        self.assertEqual(local_device.id, local['device_id'])
 
         self.assertEqual(self.ui.ui.list_devices.count(),
                          len(remote))
@@ -97,3 +100,12 @@
         self.ui.ui.manage_devices_button.click()
 
         self.assertEqual(self._called, ((gui.EDIT_DEVICES_LINK,), {}))
+
+    def test_remove_device_widget_after_removal(self):
+        """When a device widget was deleted, remove it from the UI."""
+        self.ui.process_info(SAMPLE_DEVICES_INFO)
+
+        local_device = self.ui.ui.local_device_box.itemAt(0).widget()
+        local_device.removed.emit()
+
+        self.assertTrue(self.ui.ui.local_device_box.itemAt(0) is None)

=== modified file 'ubuntuone/controlpanel/gui/tests/__init__.py'
--- ubuntuone/controlpanel/gui/tests/__init__.py	2011-07-01 12:39:37 +0000
+++ ubuntuone/controlpanel/gui/tests/__init__.py	2011-07-19 19:17:30 +0000
@@ -21,7 +21,11 @@
 import os
 
 from ubuntuone.controlpanel import gui
-from ubuntuone.controlpanel.backend import ControlBackend
+from ubuntuone.controlpanel.backend import (
+    ControlBackend,
+    DEVICE_TYPE_COMPUTER,
+    DEVICE_TYPE_PHONE,
+)
 from ubuntuone.controlpanel.tests import USER_HOME, ROOT_PATH
 
 # Attribute 'yyy' defined outside __init__, access to a protected member
@@ -95,22 +99,27 @@
 FAKE_VOLUMES_MINIMAL_INFO = [(u'', u'147852369', [ROOT, MUSIC_FOLDER])]
 
 FAKE_DEVICE_INFO = {
-    'device_id': '1258-6854', 'device_name': 'Baz', 'device_type': 'Computer',
+    'device_id': '1258-6854', 'device_name': 'Baz',
+    'device_type': DEVICE_TYPE_COMPUTER,
     'is_local': 'True', 'configurable': 'True', 'limit_bandwidth': 'True',
     'max_upload_speed': '1000', 'max_download_speed': '72548',
     'show_all_notifications': 'True',
 }
 
 FAKE_DEVICES_INFO = [
-    {'device_id': '0', 'name': 'Ubuntu One @ Foo', 'type': 'Computer',
-     'is_local': '', 'configurable': ''},
-    {'device_id': '1', 'name': 'Ubuntu One @ Bar', 'type': 'Phone',
-     'is_local': '', 'configurable': ''},
-    {'device_id': '2', 'name': 'Ubuntu One @ Z', 'type': 'Computer',
+    {'device_id': '0', 'name': 'Ubuntu One @ Foo',
+     'type': DEVICE_TYPE_COMPUTER,
+     'is_local': '', 'configurable': ''},
+    {'device_id': '1', 'name': 'Ubuntu One @ Bar',
+     'type': DEVICE_TYPE_PHONE,
+     'is_local': '', 'configurable': ''},
+    {'device_id': '2', 'name': 'Ubuntu One @ Z',
+     'type': DEVICE_TYPE_COMPUTER,
      'is_local': '', 'configurable': 'True', 'limit_bandwidth': '',
      'max_upload_speed': '0', 'max_download_speed': '0',
      'show_all_notifications': ''},
-    {'device_id': '1258-6854', 'name': 'Ubuntu One @ Baz', 'type': 'Computer',
+    {'device_id': '1258-6854', 'name': 'Ubuntu One @ Baz',
+     'type': DEVICE_TYPE_COMPUTER,
      'is_local': 'True', 'configurable': 'True', 'limit_bandwidth': 'True',
      'max_upload_speed': '1000', 'max_download_speed': '72548',
      'show_all_notifications': 'True'},  # local