diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 3389c09984..0c8c402612 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -667,7 +667,10 @@ def refresh_source_conversations(self) -> None: source_widget = self.source_list.get_source_widget(source.uuid) if source_widget: source_widget.deletion_indicator.stop() - conversation_wrapper.conversation_view.update_conversation(source.collection) + try: + conversation_wrapper.conversation_view.update_conversation(source.collection) + except sqlalchemy.exc.InvalidRequestError as e: + logger.debug("Error refreshing source conversations: %s", e) def delete_conversation(self, source_uuid: str) -> None: """ @@ -3428,7 +3431,10 @@ def __init__(self, source_db_object: Source, controller: Controller): main_layout.addWidget(self.scroll) - self.update_conversation(self.source.collection) + try: + self.update_conversation(self.source.collection) + except sqlalchemy.exc.InvalidRequestError as e: + logger.debug("Error initializing ConversationView: %s", e) def update_deletion_markers(self, collection): if collection: @@ -3628,7 +3634,15 @@ def on_reply_sent(self, source_uuid: str, reply_uuid: str, reply_text: str) -> N self.reply_flag = True if source_uuid == self.source.uuid: self.add_reply_from_reply_box(reply_uuid, reply_text) - self.update_deletion_markers(self.source.collection) + try: + self.update_deletion_markers(self.source.collection.copy()) + except sqlalchemy.exc.InvalidRequestError as e: + # The only way we should get here is if + # source.collection can't be populated, presumably + # because it had never been loaded, and the source and + # its conversation items were deleted between adding + # the reply and updating deletion markers. + logger.debug("Error in ConversationView.on_reply_sent: %s", e) class SourceConversationWrapper(QWidget): diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index fc032b75e4..3067c1f3f2 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -4,7 +4,7 @@ import math import random from datetime import datetime -from unittest.mock import Mock, patch +from unittest.mock import Mock, PropertyMock, patch import arrow import pytest @@ -905,6 +905,48 @@ def test_MainView_refresh_source_conversations(homedir, mocker, qtbot, session_m mv.refresh_source_conversations() +def test_MainView_refresh_source_conversations_with_deleted( + homedir, mocker, session, session_maker +): + mv = MainView(None) + + mock_gui = mocker.MagicMock() + controller = logic.Controller("http://localhost", mock_gui, session_maker, homedir) + controller.api = mocker.MagicMock() + + debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") + mv.setup(controller) + + source1 = factory.Source(uuid="rsc-123") + session.add(source1) + + source2 = factory.Source(uuid="rsc-456") + session.add(source2) + + session.commit() + + sources = [source1, source2] + + mv.source_list.update(sources) + mv.show() + + def collection_error(): + raise sqlalchemy.exc.InvalidRequestError() + + mocker.patch( + "securedrop_client.gui.widgets.SourceList.get_selected_source", return_value=source1 + ) + mv.on_source_changed() + + with patch( + "securedrop_client.db.Source.collection", new_callable=PropertyMock + ) as mock_collection: + ire = sqlalchemy.exc.InvalidRequestError() + mock_collection.side_effect = ire + mv.refresh_source_conversations() + debug_logger.assert_any_call("Error refreshing source conversations: %s", ire) + + def test_MainView_set_conversation(mocker): """ Ensure the passed-in widget is added to the layout of the main view holder @@ -4417,6 +4459,29 @@ def test_ConversationView_init(mocker, homedir): assert isinstance(cv.scroll.conversation_layout, QVBoxLayout) +def test_ConversationView_init_with_deleted_source(mocker, homedir, session): + """ + Ensure the conversation view has a layout to add widgets to. + """ + mocked_controller = mocker.MagicMock() + debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") + + def collection_error(): + raise sqlalchemy.exc.InvalidRequestError() + + source = factory.Source(uuid="ire-123") + session.add(source) + session.commit() + + with patch( + "securedrop_client.db.Source.collection", new_callable=PropertyMock + ) as mock_collection: + ire = sqlalchemy.exc.InvalidRequestError() + mock_collection.side_effect = ire + ConversationView(source, mocked_controller) + debug_logger.assert_any_call("Error initializing ConversationView: %s", ire) + + def test_ConversationView_ConversationScrollArea_resize(mocker): """ Test that the resize event handler calls adjust_width on each conversation item, passing in the @@ -4574,6 +4639,30 @@ def test_ConversationView_on_reply_sent_does_not_add_message_intended_for_differ assert not cv.add_reply.called +def test_ConversationView_on_reply_sent_with_deleted_source(mocker): + """ + Check that replying to a deleted source causes an error and is logged. + """ + source = factory.Source() + controller = mocker.MagicMock() + cv = ConversationView(source, controller) + cv.add_reply_from_reply_box = mocker.MagicMock() + + def collection_error(): + raise sqlalchemy.exc.InvalidRequestError() + + debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") + with patch( + "securedrop_client.db.Source.collection", new_callable=PropertyMock + ) as mock_collection: + ire = sqlalchemy.exc.InvalidRequestError() + mock_collection.side_effect = ire + + cv.on_reply_sent(source.uuid, "mock", "mock") + + debug_logger.assert_any_call("Error in ConversationView.on_reply_sent: %s", ire) + + def test_ConversationView_add_reply_from_reply_box(mocker, session, session_maker, homedir): """ Adding a reply from reply box results in a new ReplyWidget added to the layout. @@ -5054,7 +5143,7 @@ def test_ReplyBoxWidget_on_sync_source_deleted(mocker, source): controller = mocker.MagicMock() rb = ReplyBoxWidget(s, controller) - error_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") + debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") def pretend_source_was_deleted(self): raise sqlalchemy.orm.exc.ObjectDeletedError(attributes.instance_state(s), None) @@ -5062,7 +5151,7 @@ def pretend_source_was_deleted(self): with patch.object(ReplyBoxWidget, "update_authentication_state") as uas: uas.side_effect = pretend_source_was_deleted rb._on_synced("syncing") - error_logger.assert_called_once_with( + debug_logger.assert_called_once_with( "During sync, ReplyBoxWidget found its source had been deleted." ) @@ -5129,14 +5218,14 @@ def test_ReplyBoxWidget_on_authentication_changed_source_deleted(mocker, source) co = mocker.MagicMock() rb = ReplyBoxWidget(s, co) - error_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") + debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug") with mocker.patch( "securedrop_client.gui.widgets.ReplyBoxWidget.update_authentication_state", side_effect=sqlalchemy.orm.exc.ObjectDeletedError(attributes.instance_state(s), None), ): rb._on_authentication_changed(True) - error_logger.assert_called_once_with( + debug_logger.assert_called_once_with( "On authentication change, ReplyBoxWidget found its source had been deleted." )