no longer try to show conversation for deleted src

securedrop_client/gui/main.py

@@ -20,11 +20,13 @@
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
 import logging
-from PyQt5.QtWidgets import QMainWindow, QWidget, QHBoxLayout, QVBoxLayout, QDesktopWidget
 from typing import List
 
+from PyQt5.QtWidgets import QMainWindow, QWidget, QHBoxLayout, QVBoxLayout, QDesktopWidget
+
 from securedrop_client import __version__
 from securedrop_client.db import Source
+from securedrop_client.storage import source_exists
 from securedrop_client.gui.widgets import TopPane, LeftPane, MainView, LoginDialog, \
     SourceConversationWrapper
 from securedrop_client.resources import load_icon
@@ -86,9 +88,6 @@ def __init__(self, sdc_home: str):
         # We do this to not create/destroy widgets constantly (because it causes UI "flicker")
         self.conversations = {}
 
-        # Tracks which source is shown
-        self.current_source = None
-
         self.autosize_window()
         self.show()
 
@@ -172,9 +171,15 @@ def on_source_changed(self):
         """
         source_item = self.main_view.source_list.currentItem()
         source_widget = self.main_view.source_list.itemWidget(source_item)
-        if source_widget:
-            self.current_source = source_widget.source
-            self.show_conversation_for(self.current_source, self.controller.is_authenticated)
+
+        # Show conversation for the currently-selected source if it hasn't been deleted. If the
+        # current source no longer exists, clear the conversation for that source.
+        # from PyQt5.QtCore import pyqtRemoveInputHook; pyqtRemoveInputHook()
+        # import pdb; pdb.set_trace()
+        if source_widget and source_exists(self.controller.session, source_widget.source.uuid):
+            self.show_conversation_for(source_widget.source, self.controller.is_authenticated)
+        else:
+            self.main_view.clear_conversation()
 
     def show_conversation_for(self, source: Source, is_authenticated: bool):
         """

securedrop_client/gui/widgets.py

@@ -27,7 +27,7 @@
 from typing import List
 from uuid import uuid4
 
-from securedrop_client.db import Source, Message, File
+from securedrop_client.db import Source, Message, File, Reply
 from securedrop_client.gui import SvgLabel, SvgPushButton
 from securedrop_client.logic import Client
 from securedrop_client.resources import load_svg, load_icon, load_image
@@ -631,6 +631,12 @@ def set_conversation(self, widget):
         self.view_layout.addWidget(widget)
         widget.show()
 
+    def clear_conversation(self):
+        while self.view_layout.count():
+            child = self.view_layout.takeAt(0)
+            if child.widget():
+                child.widget().deleteLater()
+
 
 class SourceList(QListWidget):
     """
@@ -701,12 +707,7 @@ def launch(self):
         """
         message = self._construct_message(self.source)
         reply = QMessageBox.question(
-            self.parent,
-            "",
-            _(message),
-            QMessageBox.Cancel | QMessageBox.Yes,
-            QMessageBox.Cancel
-        )
+            self.parent, "", _(message), QMessageBox.Cancel | QMessageBox.Yes, QMessageBox.Cancel)
 
         if reply == QMessageBox.Yes:
             logger.debug("Deleting source %s" % (self.source.uuid,))
@@ -715,16 +716,19 @@ def launch(self):
     def _construct_message(self, source: Source) -> str:
         files = 0
         messages = 0
+        replies = 0
         for submission in source.collection:
             if isinstance(submission, Message):
                 messages += 1
+            if isinstance(submission, Reply):
+                replies += 1
             elif isinstance(submission, File):
                 files += 1
 
         message = (
             "<big>Deleting the Source account for",
             "<b>{}</b> will also".format(source.journalist_designation,),
-            "delete {} files and {} messages.</big>".format(files, messages),
+            "delete {} files, {} replies, and {} messages.</big>".format(files, replies, messages),
             "<br>",
             "<small>This Source will no longer be able to correspond",
             "through the log-in tied to this account.</small>",
@@ -1261,7 +1265,6 @@ def update_conversation(self, collection: list) -> None:
             if conversation_item.filename.endswith('msg.gpg'):
                 self.add_message(conversation_item)
             elif conversation_item.filename.endswith('reply.gpg'):
-                self.controller.session.refresh(conversation_item)
                 if conversation_item.content is not None:
                     content = conversation_item.content
                 else:
@@ -1293,7 +1296,6 @@ def add_message(self, message: Message) -> None:
         """
         Add a message from the source.
         """
-        self.controller.session.refresh(message)
         if message.content is not None:
             content = message.content
         else:
@@ -1423,7 +1425,6 @@ def __init__(self, source, parent, controller):
     def trigger(self):
         if self.controller.api is None:
             self.controller.on_action_requiring_login()
-            return
         else:
             self.messagebox.launch()
 

securedrop_client/logic.py

@@ -23,12 +23,14 @@
 import shutil
 import traceback
 import uuid
+
+from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer, QProcess
+
 from securedrop_client import storage
 from securedrop_client import db
 from securedrop_client.utils import check_dir_permissions
 from securedrop_client.crypto import GpgHelper, CryptoError
 from securedrop_client.message_sync import MessageSync, ReplySync
-from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer, QProcess
 
 logger = logging.getLogger(__name__)
 

securedrop_client/storage.py

@@ -20,10 +20,14 @@
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
 import logging
-from dateutil.parser import parse
 import glob
-from sqlalchemy import or_
 import os
+from dateutil.parser import parse
+
+from sqlalchemy import or_
+from sqlalchemy.orm.exc import NoResultFound
+from sqlalchemy.orm.session import Session
+
 from securedrop_client.db import Source, Message, File, Reply, User
 
 
@@ -396,3 +400,11 @@ def rename_file(data_dir: str, filename: str, new_filename: str):
                   os.path.join(data_dir, new_filename))
     except OSError as e:
         logger.debug('File could not be renamed: {}'.format(e))
+
+
+def source_exists(session: Session, source_uuid: str) -> bool:
+    try:
+        session.query(Source).filter_by(uuid=source_uuid).one()
+        return True
+    except NoResultFound:
+        return False

tests/gui/test_main.py

@@ -1,11 +1,16 @@
 """
 Check the core Window UI class works as expected.
 """
+from uuid import uuid4
+
 from PyQt5.QtWidgets import QApplication, QHBoxLayout
+
 from securedrop_client.db import Message
 from securedrop_client.gui.main import Window
 from securedrop_client.resources import load_icon
-from uuid import uuid4
+
+from tests import factory
+
 
 app = QApplication([])
 
@@ -220,14 +225,27 @@ def test_on_source_changed(mocker):
     """
     w = Window('mock')
     w.main_view = mocker.MagicMock()
-    w.main_view.source_list.currentItem()
-    mock_sw = w.main_view.source_list.itemWidget()
     w.show_conversation_for = mocker.MagicMock()
-    mock_controller = mocker.MagicMock(is_authenticated=True)
-    w.controller = mock_controller
+    w.controller = mocker.MagicMock(is_authenticated=True)
+
     w.on_source_changed()
-    w.show_conversation_for.assert_called_once_with(mock_sw.source,
-                                                    mock_controller.is_authenticated)
+
+    source = w.main_view.source_list.itemWidget().source
+    w.show_conversation_for.assert_called_once_with(source, True)
+
+
+def test_on_source_changed_when_source_no_longer_exists(mocker):
+    """
+    Test that conversation for a source is cleared when the source no longer exists.
+    """
+    w = Window('mock')
+    w.main_view = mocker.MagicMock()
+    w.controller = mocker.MagicMock(is_authenticated=True)
+    mocker.patch('securedrop_client.gui.main.source_exists', return_value=False)
+
+    w.on_source_changed()
+
+    w.main_view.clear_conversation.assert_called_once_with()
 
 
 def test_conversation_for(mocker):

tests/gui/test_widgets.py

@@ -394,6 +394,21 @@ def test_MainView_show_conversation(mocker):
     mv.view_layout.addWidget.assert_called_once_with(mock_widget)
 
 
+def test_MainView_clear_conversation(mocker, homedir):
+    """
+    Calling clear_conversation deletes items from layout
+    """
+    mv = MainView(None)
+    mv.view_layout = QVBoxLayout()
+    mv.view_layout.addWidget(QWidget())
+
+    assert mv.view_layout.count() == 1
+
+    mv.clear_conversation()
+
+    assert mv.view_layout.count() == 0
+
+
 def test_SourceList_update(mocker):
     """
     Check the items in the source list are cleared and a new SourceWidget for
@@ -1307,6 +1322,8 @@ def test_DeleteSourceMssageBox_launch_when_user_chooses_yes(mocker, source, sess
     session.add(message)
     message = factory.Message(source=source)
     session.add(message)
+    reply = factory.Reply(source=source)
+    session.add(reply)
     session.commit()
 
     mock_message_box_question = mocker.MagicMock(QMessageBox.question)
@@ -1326,11 +1343,11 @@ def test_DeleteSourceMssageBox_launch_when_user_chooses_yes(mocker, source, sess
     message = (
         "<big>Deleting the Source account for "
         "<b>{designation}</b> will also "
-        "delete {files} files and {messages} messages.</big> "
-        "<br> "
+        "delete {files} files, {replies} replies, and {messages} messages.</big>"
+        " <br> "
         "<small>This Source will no longer be able to correspond "
         "through the log-in tied to this account.</small>"
-    ).format(designation=source.journalist_designation, files=1, messages=2)
+    ).format(designation=source.journalist_designation, files=1, replies=1, messages=2)
     mock_message_box_question.assert_called_once_with(
         None,
         "",
@@ -1348,6 +1365,8 @@ def test_DeleteSourceMessageBox_construct_message(mocker, source, session):
     session.add(message)
     message = factory.Message(source=source)
     session.add(message)
+    reply = factory.Reply(source=source)
+    session.add(reply)
     session.commit()
 
     mock_controller = mocker.MagicMock()
@@ -1359,11 +1378,11 @@ def test_DeleteSourceMessageBox_construct_message(mocker, source, session):
     expected_message = (
         "<big>Deleting the Source account for "
         "<b>{designation}</b> will also "
-        "delete {files} files and {messages} messages.</big> "
-        "<br> "
+        "delete {files} files, {replies} replies, and {messages} messages.</big>"
+        " <br> "
         "<small>This Source will no longer be able to correspond "
         "through the log-in tied to this account.</small>"
-    ).format(designation=source.journalist_designation, files=1, messages=2)
+    ).format(designation=source.journalist_designation, files=1, replies=1, messages=2)
     assert message == expected_message
 
 

tests/test_logic.py

@@ -5,6 +5,7 @@
 import arrow
 import os
 import pytest
+
 from sdclientapi import sdlocalobjects
 from tests import factory
 from securedrop_client import storage, db

tests/test_storage.py

@@ -4,13 +4,16 @@
 import pytest
 import os
 import uuid
-import securedrop_client.db
 from dateutil.parser import parse
+
+from sqlalchemy.orm.exc import NoResultFound
+
+import securedrop_client.db
 from securedrop_client.storage import get_local_sources, get_local_messages, get_local_replies, \
     get_remote_data, update_local_storage, update_sources, update_files, update_replies, \
     find_or_create_user, find_new_messages, find_new_replies, mark_file_as_downloaded, \
     mark_reply_as_downloaded, delete_single_submission_or_reply_on_disk, rename_file, \
-    get_local_files, find_new_files, mark_message_as_downloaded, \
+    get_local_files, find_new_files, mark_message_as_downloaded, source_exists, \
     set_object_decryption_status_with_content
 from securedrop_client import db
 from sdclientapi import Source, Submission, Reply
@@ -850,3 +853,26 @@ def test_rename_file_success(homedir):
     with open(os.path.join(homedir, 'data', trunc_new_filename)) as f:
         out = f.read()
     assert out == contents
+
+
+def test_source_exists_true(homedir, mocker):
+    '''
+    Check that method returns True if a source is return from the query.
+    '''
+    session = mocker.MagicMock()
+    source = make_remote_source()
+    source.uuid = 'test-source-uuid'
+    session.query().filter_by().one.return_value = source
+    assert source_exists(session, 'test-source-uuid')
+
+
+def test_source_exists_false(homedir, mocker):
+    '''
+    Check that method returns False if NoResultFound is thrown when we try to query the source.
+    '''
+    session = mocker.MagicMock()
+    source = mocker.MagicMock()
+    source.uuid = 'test-source-uuid'
+    session.query().filter_by().one.side_effect = NoResultFound()
+
+    assert not source_exists(session, 'test-source-uuid')