Address code review feedback

securedrop_client/gui/widgets.py

@@ -549,133 +549,6 @@ def _on_clicked(self):
         self.window.show_login()
 
 
-class SourceConversationWrapper(QWidget):
-    """
-    Wrapper for a source's conversation including the chat window, profile tab, and other
-    per-source resources.
-    """
-
-    deleting_account = False
-    deleting_conversation = False
-
-    def __init__(self, source: Source, controller: Controller) -> None:
-        super().__init__()
-
-        self.setObjectName("SourceConversationWrapper")
-
-        self.source = source
-        self.source_uuid = source.uuid
-        controller.conversation_deleted.connect(self.on_conversation_deleted)
-        controller.conversation_deletion_failed.connect(self.on_conversation_deletion_failed)
-        controller.source_deleted.connect(self.on_source_deleted)
-        controller.source_deletion_failed.connect(self.on_source_deletion_failed)
-
-        # Set layout
-        layout = QVBoxLayout()
-        self.setLayout(layout)
-
-        # Set margins and spacing
-        layout.setContentsMargins(0, 0, 0, 0)
-        layout.setSpacing(0)
-
-        # Create widgets
-        self.conversation_title_bar = SourceProfileShortWidget(source, controller)
-        self.conversation_view = ConversationView(source, controller)
-        self.reply_box = ReplyBoxWidget(source, controller)
-        self.deletion_indicator = SourceDeletionIndicator()
-        self.conversation_deletion_indicator = ConversationDeletionIndicator()
-
-        # Add widgets
-        layout.addWidget(self.conversation_title_bar)
-        layout.addWidget(self.conversation_view)
-        layout.addWidget(self.deletion_indicator)
-        layout.addWidget(self.conversation_deletion_indicator)
-        layout.addWidget(self.reply_box)
-
-        # Connect reply_box to conversation_view
-        self.reply_box.reply_sent.connect(self.conversation_view.on_reply_sent)
-        self.conversation_view.conversation_updated.connect(self.on_conversation_updated)
-
-    @pyqtSlot(str)
-    def on_conversation_deleted(self, source_uuid: str):
-        if self.source_uuid == source_uuid:
-            self.start_conversation_deletion()
-
-    @pyqtSlot(str)
-    def on_conversation_deletion_failed(self, source_uuid: str):
-        if self.source_uuid == source_uuid:
-            self.end_conversation_deletion()
-
-    @pyqtSlot()
-    def on_conversation_updated(self):
-        self.conversation_title_bar.update_timestamp()
-
-    @pyqtSlot(str)
-    def on_source_deleted(self, source_uuid: str):
-        if self.source_uuid == source_uuid:
-            self.start_account_deletion()
-
-    @pyqtSlot(str)
-    def on_source_deletion_failed(self, source_uuid: str):
-        if self.source_uuid == source_uuid:
-            self.end_account_deletion()
-
-    def start_conversation_deletion(self):
-        self.reply_box.setProperty("class", "deleting_conversation")
-        self.deleting_conversation = True
-        self.start_deletion()
-        self.conversation_deletion_indicator.start()
-        self.deletion_indicator.stop()
-
-    def start_account_deletion(self):
-        self.reply_box.setProperty("class", "deleting")
-        self.deleting_account = True
-        self.reply_box.text_edit.setText("")
-        self.start_deletion()
-
-        palette = QPalette()
-        palette.setBrush(QPalette.Background, QBrush(QColor("#9495b9")))
-        palette.setBrush(QPalette.Foreground, QBrush(QColor("#ffffff")))
-
-        self.conversation_title_bar.setPalette(palette)
-        self.conversation_title_bar.setAutoFillBackground(True)
-
-        self.conversation_deletion_indicator.stop()
-        self.deletion_indicator.start()
-
-    def start_deletion(self):
-        css = load_css("sdclient.css")
-        self.reply_box.setStyleSheet(css)
-        self.setStyleSheet(css)
-
-        self.reply_box.text_edit.setDisabled(True)
-        self.reply_box.send_button.setDisabled(True)
-        self.conversation_title_bar.setDisabled(True)
-        self.conversation_view.hide()
-
-    def end_conversation_deletion(self):
-        self.deleting_conversation = False
-        self.end_deletion()
-
-    def end_account_deletion(self):
-        self.deleting_account = False
-        self.end_deletion()
-
-    def end_deletion(self):
-        self.reply_box.setProperty("class", "")
-        css = load_css("sdclient.css")
-        self.reply_box.setStyleSheet(css)
-        self.setStyleSheet(css)
-
-        self.reply_box.setEnabled(True)
-        self.reply_box.send_button.setEnabled(True)
-        self.conversation_title_bar.setEnabled(True)
-        self.conversation_view.show()
-
-        self.conversation_deletion_indicator.stop()
-        self.deletion_indicator.stop()
-
-
 class MainView(QWidget):
     """
     Represents the main content of the application (containing the source list
@@ -1418,6 +1291,9 @@ def set_snippet(self, source_uuid: str, collection_uuid: str = None, content: st
             content = ""
         elif not self.source.server_collection:
             if self.source.interaction_count > 0:
+                # The server only ever increases the interaction
+                # count, so if it's non-zero but the source collection
+                # is empty, we know the conversation has been deleted.
                 content = _("\u2014 All files and messages deleted for this source \u2014")
             else:
                 content = ""
@@ -3755,6 +3631,133 @@ def on_reply_sent(self, source_uuid: str, reply_uuid: str, reply_text: str) -> N
             self.update_deletion_markers(self.source.collection)
 
 
+class SourceConversationWrapper(QWidget):
+    """
+    Wrapper for a source's conversation including the chat window, profile tab, and other
+    per-source resources.
+    """
+
+    deleting_account = False
+    deleting_conversation = False
+
+    def __init__(self, source: Source, controller: Controller) -> None:
+        super().__init__()
+
+        self.setObjectName("SourceConversationWrapper")
+
+        self.source = source
+        self.source_uuid = source.uuid
+        controller.conversation_deleted.connect(self.on_conversation_deleted)
+        controller.conversation_deletion_failed.connect(self.on_conversation_deletion_failed)
+        controller.source_deleted.connect(self.on_source_deleted)
+        controller.source_deletion_failed.connect(self.on_source_deletion_failed)
+
+        # Set layout
+        layout = QVBoxLayout()
+        self.setLayout(layout)
+
+        # Set margins and spacing
+        layout.setContentsMargins(0, 0, 0, 0)
+        layout.setSpacing(0)
+
+        # Create widgets
+        self.conversation_title_bar = SourceProfileShortWidget(source, controller)
+        self.conversation_view = ConversationView(source, controller)
+        self.reply_box = ReplyBoxWidget(source, controller)
+        self.deletion_indicator = SourceDeletionIndicator()
+        self.conversation_deletion_indicator = ConversationDeletionIndicator()
+
+        # Add widgets
+        layout.addWidget(self.conversation_title_bar)
+        layout.addWidget(self.conversation_view)
+        layout.addWidget(self.deletion_indicator)
+        layout.addWidget(self.conversation_deletion_indicator)
+        layout.addWidget(self.reply_box)
+
+        # Connect reply_box to conversation_view
+        self.reply_box.reply_sent.connect(self.conversation_view.on_reply_sent)
+        self.conversation_view.conversation_updated.connect(self.on_conversation_updated)
+
+    @pyqtSlot(str)
+    def on_conversation_deleted(self, source_uuid: str):
+        if self.source_uuid == source_uuid:
+            self.start_conversation_deletion()
+
+    @pyqtSlot(str)
+    def on_conversation_deletion_failed(self, source_uuid: str):
+        if self.source_uuid == source_uuid:
+            self.end_conversation_deletion()
+
+    @pyqtSlot()
+    def on_conversation_updated(self):
+        self.conversation_title_bar.update_timestamp()
+
+    @pyqtSlot(str)
+    def on_source_deleted(self, source_uuid: str):
+        if self.source_uuid == source_uuid:
+            self.start_account_deletion()
+
+    @pyqtSlot(str)
+    def on_source_deletion_failed(self, source_uuid: str):
+        if self.source_uuid == source_uuid:
+            self.end_account_deletion()
+
+    def start_conversation_deletion(self):
+        self.reply_box.setProperty("class", "deleting_conversation")
+        self.deleting_conversation = True
+        self.start_deletion()
+        self.conversation_deletion_indicator.start()
+        self.deletion_indicator.stop()
+
+    def start_account_deletion(self):
+        self.reply_box.setProperty("class", "deleting")
+        self.deleting_account = True
+        self.reply_box.text_edit.setText("")
+        self.start_deletion()
+
+        palette = QPalette()
+        palette.setBrush(QPalette.Background, QBrush(QColor("#9495b9")))
+        palette.setBrush(QPalette.Foreground, QBrush(QColor("#ffffff")))
+
+        self.conversation_title_bar.setPalette(palette)
+        self.conversation_title_bar.setAutoFillBackground(True)
+
+        self.conversation_deletion_indicator.stop()
+        self.deletion_indicator.start()
+
+    def start_deletion(self):
+        css = load_css("sdclient.css")
+        self.reply_box.setStyleSheet(css)
+        self.setStyleSheet(css)
+
+        self.reply_box.text_edit.setDisabled(True)
+        self.reply_box.send_button.setDisabled(True)
+        self.conversation_title_bar.setDisabled(True)
+        self.conversation_view.hide()
+
+    def end_conversation_deletion(self):
+        self.deleting_conversation = False
+        self.end_deletion()
+
+    def end_account_deletion(self):
+        self.deleting_account = False
+        self.end_deletion()
+
+    def end_deletion(self):
+        self.reply_box.setProperty("class", "")
+        css = load_css("sdclient.css")
+        self.reply_box.setStyleSheet(css)
+        self.setStyleSheet(css)
+
+        self.reply_box.setEnabled(True)
+        self.reply_box.send_button.setEnabled(True)
+        self.conversation_title_bar.setEnabled(True)
+        self.conversation_view.show()
+
+        self.conversation_deletion_indicator.stop()
+        self.deletion_indicator.stop()
+
+
 class ReplyBoxWidget(QWidget):
     """
     A textbox where a journalist can enter a reply.
@@ -4196,13 +4199,13 @@ def __init__(self, source, controller):
         header_layout.addWidget(menu, alignment=Qt.AlignRight)
 
         # Create horizontal line
-        self.horizontal_line = QWidget()
-        self.horizontal_line.setObjectName("SourceProfileShortWidget_horizontal_line")
-        self.horizontal_line.setSizePolicy(QSizePolicy.Expanding, QSizePolicy.Fixed)
+        horizontal_line = QWidget()
+        horizontal_line.setObjectName("SourceProfileShortWidget_horizontal_line")
+        horizontal_line.setSizePolicy(QSizePolicy.Expanding, QSizePolicy.Fixed)
 
         # Add widgets
         layout.addWidget(header)
-        layout.addWidget(self.horizontal_line)
+        layout.addWidget(horizontal_line)
 
     def update_timestamp(self):
         """

securedrop_client/logic.py

@@ -997,6 +997,7 @@ def on_delete_conversation_success(self, uuid: str) -> None:
     def on_delete_conversation_failure(self, e: Exception) -> None:
         if isinstance(e, DeleteConversationJobException):
             error = _("Failed to delete conversation at server")
+            logger.debug("Failed to delete conversation %s at server", e.source_uuid)
             self.gui.update_error_status(error)
             self.conversation_deletion_failed.emit(e.source_uuid)
 

tests/test_logic.py

@@ -1845,8 +1845,11 @@ def test_Controller_on_delete_conversation_success(mocker, homedir):
 
 def test_Controller_on_delete_conversation_failure(homedir, config, mocker, session_maker, session):
     mock_gui = mocker.MagicMock()
+    debug_logger = mocker.patch("securedrop_client.logic.logger.debug")
+
     co = Controller("http://localhost", mock_gui, session_maker, homedir)
     co.on_delete_conversation_failure(DeleteConversationJobException("weow", "uuid"))
+    debug_logger.assert_called_once_with("Failed to delete conversation %s at server", "uuid")
     co.gui.update_error_status.assert_called_with("Failed to delete conversation at server")