fix #1285: skip conversation update if the sync is stale

Signed-off-by: Allie Crevier <allie@freedom.press>

securedrop_client/gui/widgets.py

@@ -3242,11 +3242,20 @@ def __init__(self, source_db_object: Source, controller: Controller) -> None:
         super().__init__()
 
         self.source = source_db_object
+        self.source_uuid = source_db_object.uuid
         self.controller = controller
 
+        self.controller.sync_started.connect(self._on_sync_started)
+        controller.conversation_deletion_successful.connect(
+            self._on_conversation_deletion_successful
+        )
+
         # To hold currently displayed messages.
         self.current_messages = {}  # type: Dict[str, QWidget]
 
+        self.deletion_scheduled_timestamp = datetime.utcnow()
+        self.sync_started_timestamp = datetime.utcnow()
+
         self.setObjectName("ConversationView")
 
         # Set layout
@@ -3278,6 +3287,20 @@ def __init__(self, source_db_object: Source, controller: Controller) -> None:
         except sqlalchemy.exc.InvalidRequestError as e:
             logger.debug("Error initializing ConversationView: %s", e)
 
+    @pyqtSlot(datetime)
+    def _on_sync_started(self, timestamp: datetime) -> None:
+        self.sync_started_timestamp = timestamp
+
+    @pyqtSlot(str, datetime)
+    def _on_conversation_deletion_successful(self, source_uuid: str, timestamp: datetime) -> None:
+        if self.source_uuid == source_uuid:
+            self.deletion_scheduled_timestamp = timestamp
+            for message in self.current_messages.values():
+                message.hide()
+            self.scroll.hide()
+            self.deleted_conversation_items_marker.hide()
+            self.deleted_conversation_marker.show()
+
     def update_deletion_markers(self) -> None:
         try:
             if self.source.collection:
@@ -3311,6 +3334,11 @@ def update_conversation(self, collection: list) -> None:
         passed into this method in case of a mismatch between where the widget
         has been and now is in terms of its index in the conversation.
         """
+        # If the sync started before the deletion finished, then the sync is stale and we do
+        # not want to update the conversation.
+        if self.sync_started_timestamp < self.deletion_scheduled_timestamp:
+            return
+
         self.controller.session.refresh(self.source)
 
         # Keep a temporary copy of the current conversation so we can delete any
@@ -3499,6 +3527,9 @@ def __init__(self, source: Source, controller: Controller) -> None:
         self.source_uuid = source.uuid
         controller.conversation_deleted.connect(self.on_conversation_deleted)
         controller.conversation_deletion_failed.connect(self.on_conversation_deletion_failed)
+        controller.conversation_deletion_successful.connect(
+            self._on_conversation_deletion_successful
+        )
         controller.source_deleted.connect(self.on_source_deleted)
         controller.source_deletion_failed.connect(self.on_source_deletion_failed)
 
@@ -3533,6 +3564,11 @@ def on_conversation_deleted(self, source_uuid: str) -> None:
         if self.source_uuid == source_uuid:
             self.start_conversation_deletion()
 
+    @pyqtSlot(str, datetime)
+    def _on_conversation_deletion_successful(self, source_uuid: str, timestamp: datetime) -> None:
+        if self.source_uuid == source_uuid:
+            self.end_conversation_deletion()
+
     @pyqtSlot(str)
     def on_conversation_deletion_failed(self, source_uuid: str) -> None:
         if self.source_uuid == source_uuid:

securedrop_client/logic.py

@@ -290,6 +290,15 @@ class Controller(QObject):
     """
     source_deletion_failed = pyqtSignal(str)
 
+    """
+    This signal indicates that a deletion attempt was successful at the server.
+
+    Emits:
+        str: the source UUID
+        datetime: the timestamp for when the deletion succeeded
+    """
+    conversation_deletion_successful = pyqtSignal(str, datetime)
+
     """
     This signal lets the queue manager know to add the job to the appropriate
     network queue.
@@ -1005,8 +1014,12 @@ def on_file_download_failure(self, exception: Exception) -> None:
             self.gui.update_error_status(_("The file download failed. Please try again."))
 
     def on_delete_conversation_success(self, uuid: str) -> None:
+        """
+        If the source collection has been successfully scheduled for deletion on the server, emit a
+        signal and sync.
+        """
         logger.info("Conversation %s successfully deleted at server", uuid)
-        self.api_sync.sync()
+        self.conversation_deletion_successful.emit(uuid, datetime.utcnow())
 
     def on_delete_conversation_failure(self, e: Exception) -> None:
         if isinstance(e, DeleteConversationJobException):

tests/gui/test_widgets.py

@@ -4337,6 +4337,19 @@ def test_SourceConversationWrapper_on_conversation_deleted_wrong_uuid(mocker):
     assert scw.deletion_indicator.isHidden()
 
 
+def test_SourceConversationWrapper__on_conversation_deletion_successful(mocker):
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    scw.on_conversation_deleted("123")
+
+    scw._on_conversation_deletion_successful("123", datetime.now())
+
+    assert not scw.conversation_title_bar.isHidden()
+    assert not scw.conversation_view.isHidden()
+    assert not scw.reply_box.isHidden()
+    assert scw.conversation_deletion_indicator.isHidden()
+    assert scw.deletion_indicator.isHidden()
+
+
 def test_SourceConversationWrapper_on_conversation_deletion_failed(mocker):
     scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
     scw.on_conversation_deleted("123")
@@ -4424,6 +4437,47 @@ def test_ConversationView_ConversationScrollArea_resize(mocker):
     file_widget_adjust_width.assert_called_with(cv.scroll.widget().width())
 
 
+def test_ConversationView__on_sync_started(mocker, session):
+    cv = ConversationView(factory.Source(), mocker.MagicMock())
+    timestamp = datetime.now()
+    cv._on_sync_started(timestamp)
+    assert cv.sync_started_timestamp == timestamp
+
+
+def test_ConversationView__on_conversation_deletion_successful(mocker, session):
+    source = factory.Source()
+    message = factory.Message(source=source)
+    session.add(message)
+    session.add(source)
+    session.commit()
+    cv = ConversationView(source, mocker.MagicMock())
+    timestamp = datetime.now()
+
+    cv._on_conversation_deletion_successful(cv.source.uuid, timestamp)
+
+    assert cv.deletion_scheduled_timestamp == timestamp
+    assert cv.scroll.isHidden()
+    assert cv.deleted_conversation_items_marker.isHidden()
+    assert not cv.deleted_conversation_marker.isHidden()
+    assert cv.current_messages[message.uuid].isHidden()
+
+
+def test_ConversationView_update_conversation_skips_if_sync_is_stale(mocker):
+    """
+    If the sync started before the source was scheduled for deletion, do not update the conversation
+    """
+    cv = ConversationView(factory.Source(), mocker.MagicMock())
+    cv.update_deletion_markers = mocker.MagicMock()
+    cv.sync_started_timestamp = datetime.now()
+    cv.deletion_scheduled_timestamp = datetime.now()
+    cv.update_conversation([])
+    cv.update_deletion_markers.assert_not_called()
+    # Also test that a new message will not get added if the sync is stale
+    cv.update_conversation([factory.Message()])
+    assert not cv.current_messages
+    cv.update_deletion_markers.assert_not_called()
+
+
 def test_ConversationView_update_conversation_position_follow(mocker, homedir):
     """
     Check the signal handler sets the correct value for the scrollbar to be