diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 4c1ecf6b9b..e6ff74006f 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -20,11 +20,13 @@ along with this program. If not, see . """ 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): """ diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index b8587d2940..3ca793088b 100644 --- a/securedrop_client/gui/widgets.py +++ b/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 = ( "Deleting the Source account for", "{} will also".format(source.journalist_designation,), - "delete {} files and {} messages.".format(files, messages), + "delete {} files, {} replies, and {} messages.".format(files, replies, messages), "
", "This Source will no longer be able to correspond", "through the log-in tied to this account.", @@ -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() diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index d4463313c3..75406c8740 100644 --- a/securedrop_client/logic.py +++ b/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__) diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index bbe398bb18..7c6fcfd201 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -20,10 +20,14 @@ along with this program. If not, see . """ 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 diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 4f8408f4dc..39539a0d23 100644 --- a/tests/gui/test_main.py +++ b/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): diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 4c29cbd11a..70d8eceb48 100644 --- a/tests/gui/test_widgets.py +++ b/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 = ( "Deleting the Source account for " "{designation} will also " - "delete {files} files and {messages} messages. " - "
" + "delete {files} files, {replies} replies, and {messages} messages." + "
" "This Source will no longer be able to correspond " "through the log-in tied to this account." - ).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 = ( "Deleting the Source account for " "{designation} will also " - "delete {files} files and {messages} messages. " - "
" + "delete {files} files, {replies} replies, and {messages} messages." + "
" "This Source will no longer be able to correspond " "through the log-in tied to this account." - ).format(designation=source.journalist_designation, files=1, messages=2) + ).format(designation=source.journalist_designation, files=1, replies=1, messages=2) assert message == expected_message diff --git a/tests/test_logic.py b/tests/test_logic.py index e2259d1ada..2fb2debbe3 100644 --- a/tests/test_logic.py +++ b/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 diff --git a/tests/test_storage.py b/tests/test_storage.py index 1c4502f9e8..5cd25ae874 100644 --- a/tests/test_storage.py +++ b/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')