diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index d109be405..785920f4f 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -22,7 +22,7 @@ import sys from gettext import gettext as _ -from typing import List +from typing import Dict, List # noqa: F401 from uuid import uuid4 from PyQt5.QtCore import Qt, pyqtSlot, pyqtSignal, QTimer, QSize, pyqtBoundSignal, QObject from PyQt5.QtGui import QIcon, QPalette, QBrush, QColor, QFont, QLinearGradient @@ -609,6 +609,8 @@ def __init__(self, parent: QObject): self.layout.addWidget(self.source_list) self.layout.addWidget(self.view_holder) + self.source_conversations = {} # type: Dict[Source, SourceConversationWrapper] + def setup(self, controller): """ Pass through the controller object to this widget. @@ -631,7 +633,14 @@ def on_source_changed(self): source = self.source_list.get_current_source() if source: - conversation_wrapper = SourceConversationWrapper(source, self.controller) + # Try to get the SourceConversationWrapper from the persistent dict, + # else we create it. + try: + conversation_wrapper = self.source_conversations[source] + except KeyError: + conversation_wrapper = SourceConversationWrapper(source, self.controller) + self.source_conversations[source] = conversation_wrapper + self.set_conversation(conversation_wrapper) else: self.clear_conversation() diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index ca4651f07..cfeee4dc7 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -470,6 +470,55 @@ def test_MainView_on_source_changed_updates_conversation_view(mocker, session): assert add_file_fn.call_count == 1 +def test_MainView_on_source_changed_SourceConversationWrapper_is_preserved(mocker, session): + """ + SourceConversationWrapper contains ReplyBoxWidget - this tests that we do not recreate + SourceConversationWrapper when we click away from a given source. We should create it the + first time, and then it should persist. + """ + mv = MainView(None) + mv.source_list = mocker.MagicMock() + mv.set_conversation = mocker.MagicMock() + mv.controller = mocker.MagicMock(is_authenticated=True) + source = factory.Source() + source2 = factory.Source() + session.add(source) + session.add(source2) + session.commit() + + source_conversation_init = mocker.patch( + 'securedrop_client.gui.widgets.SourceConversationWrapper.__init__', return_value=None) + + # We expect on the first call, SourceConversationWrapper.__init__ should be called. + mv.source_list.get_current_source = mocker.MagicMock(return_value=source) + mv.on_source_changed() + assert mv.set_conversation.call_count == 1 + assert source_conversation_init.call_count == 1 + + # Reset mock call counts for next call of on_source_changed. + source_conversation_init.reset_mock() + mv.set_conversation.reset_mock() + + # Now click on another source (source2). Since this is the first time we have clicked + # on source2, we expect on the first call, SourceConversationWrapper.__init__ should be + # called. + mv.source_list.get_current_source = mocker.MagicMock(return_value=source2) + mv.on_source_changed() + assert mv.set_conversation.call_count == 1 + assert source_conversation_init.call_count == 1 + + # Reset mock call counts for next call of on_source_changed. + source_conversation_init.reset_mock() + mv.set_conversation.reset_mock() + + # But if we click back (call on_source_changed again) to the source, + # its SourceConversationWrapper should _not_ be recreated. + mv.source_list.get_current_source = mocker.MagicMock(return_value=source) + mv.on_source_changed() + assert mv.set_conversation.call_count == 1 + assert source_conversation_init.call_count == 0 + + def test_MainView_set_conversation(mocker): """ Ensure the passed-in widget is added to the layout of the main view holder