From a099fde9347a47070041988a321ae7faeab50779 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 19 Dec 2018 12:37:08 +0100 Subject: [PATCH 01/14] parallelize init tasks --- run.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/run.sh b/run.sh index 15ccb579d..7719414a6 100755 --- a/run.sh +++ b/run.sh @@ -26,9 +26,11 @@ chmod 0700 "$SDC_HOME" "$GPG_HOME" echo "Running app with home directory: $SDC_HOME" echo "" -gpg --homedir "$GPG_HOME" --allow-secret-key-import --import tests/files/securedrop.gpg.asc +gpg --homedir "$GPG_HOME" --allow-secret-key-import --import tests/files/securedrop.gpg.asc & # create the database and config for local testing -./create_dev_data.py "$SDC_HOME" +./create_dev_data.py "$SDC_HOME" & + +wait exec python -m securedrop_client --sdc-home "$SDC_HOME" --no-proxy $@ From abb9ee07a61fc3bf701d4c805c7bdeabb14e5884 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 19 Dec 2018 12:42:06 +0100 Subject: [PATCH 02/14] don't add placeholder title message --- securedrop_client/gui/main.py | 2 -- tests/gui/test_main.py | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 3047cc532..597a0d4d1 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -172,8 +172,6 @@ def show_conversation_for(self, source): """ conversation = ConversationView(self) conversation.setup(self.controller) - conversation.add_message('Source name: {}'.format( - source.journalist_designation)) # Display each conversation item in the source collection. for conversation_item in source.collection: diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index eef52dad8..1396cede5 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -236,8 +236,7 @@ def test_conversation_pending_message(mocker): w.show_conversation_for(mock_source) conv = mock_conview() - # once for source name, once for message - assert conv.add_message.call_count == 2 + assert conv.add_message.call_count == 1 assert conv.add_message.call_args == mocker.call("") From c27b58ce3aced4e80177d2f144a74ed0e4df6755 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 19 Dec 2018 13:27:17 +0100 Subject: [PATCH 03/14] added newlines to remove wall-of-text looking code blocks --- securedrop_client/app.py | 3 ++ securedrop_client/db.py | 8 ++++ securedrop_client/gui/main.py | 20 ++++++++-- securedrop_client/gui/widgets.py | 63 ++++++++++++++++++++++++++++++-- securedrop_client/logic.py | 20 +++++++++- securedrop_client/storage.py | 13 +++++++ 6 files changed, 119 insertions(+), 8 deletions(-) diff --git a/securedrop_client/app.py b/securedrop_client/app.py index 7b42c8b38..cb312a254 100644 --- a/securedrop_client/app.py +++ b/securedrop_client/app.py @@ -65,16 +65,19 @@ def configure_logging(sdc_home: str) -> None: log_fmt = ('%(asctime)s - %(name)s:%(lineno)d(%(funcName)s) ' '%(levelname)s: %(message)s') formatter = logging.Formatter(log_fmt) + # define log handlers such as for rotating log files handler = TimedRotatingFileHandler(log_file, when='midnight', backupCount=5, delay=0, encoding=ENCODING) handler.setFormatter(formatter) handler.setLevel(logging.DEBUG) + # set up primary log log = logging.getLogger() log.setLevel(logging.DEBUG) log.addHandler(handler) + # override excepthook to capture a log of catastrophic failures. sys.excepthook = excepthook diff --git a/securedrop_client/db.py b/securedrop_client/db.py index 19be3e019..3d8368d32 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -25,7 +25,9 @@ def make_engine(home: str): class Source(Base): + __tablename__ = 'sources' + # TODO - add number_of_docs id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) @@ -66,7 +68,9 @@ def collection(self): class Submission(Base): + __tablename__ = 'submissions' + id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) filename = Column(String(255), nullable=False) @@ -101,7 +105,9 @@ def __repr__(self): class Reply(Base): + __tablename__ = 'replies' + id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) source_id = Column(Integer, ForeignKey('sources.id')) @@ -133,7 +139,9 @@ def __repr__(self): class User(Base): + __tablename__ = 'users' + id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) username = Column(String(255), nullable=False, unique=True) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 597a0d4d1..452324866 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -53,19 +53,29 @@ def __init__(self, sdc_home: str): self.sdc_home = sdc_home self.setWindowTitle(_("SecureDrop Client {}").format(__version__)) self.setWindowIcon(load_icon(self.icon)) + self.widget = QWidget() widget_layout = QVBoxLayout() self.widget.setLayout(widget_layout) + self.setCentralWidget(self.widget) + self.tool_bar = ToolBar(self.widget) + self.main_view = MainView(self.widget) self.main_view.source_list.itemSelectionChanged.connect(self.on_source_changed) + widget_layout.addWidget(self.tool_bar, 1) widget_layout.addWidget(self.main_view, 6) - self.setCentralWidget(self.widget) - self.current_source = None # Tracks which source is shown + + # Cache a dict of source.uuid -> ConversationView + # We do this to not create/destroy widgets constantly (because it causes UI "flicker") self.conversations = {} - self.show() + + # Tracks which source is shown + self.current_source = None + self.autosize_window() + self.show() def setup(self, controller): """ @@ -74,9 +84,11 @@ def setup(self, controller): """ self.controller = controller # Reference the Client logic instance. self.tool_bar.setup(self, controller) + self.status_bar = QStatusBar(self) self.setStatusBar(self.status_bar) self.set_status('Started SecureDrop Client. Please sign in.', 20000) + self.login_dialog = LoginDialog(self) self.main_view.setup(self.controller) @@ -190,10 +202,12 @@ def show_conversation_for(self, source): container = QWidget() layout = QVBoxLayout() container.setLayout(layout) + source_profile = SourceProfileShortWidget(source, self.controller) layout.addWidget(source_profile) layout.addWidget(conversation) + self.main_view.update_view(container) def set_status(self, message, duration=5000): diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 006864d80..9d0bf868a 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -43,15 +43,20 @@ def __init__(self, parent): layout = QHBoxLayout(self) self.logo = QLabel() self.logo.setPixmap(load_image('header_logo.png')) + self.user_state = QLabel(_("Signed out.")) + self.login = QPushButton(_('Sign in')) self.login.clicked.connect(self.on_login_clicked) + self.logout = QPushButton(_('Sign out')) self.logout.clicked.connect(self.on_logout_clicked) self.logout.setVisible(False) + self.refresh = QPushButton(_('Refresh')) self.refresh.clicked.connect(self.on_refresh_clicked) self.refresh.setVisible(False) + layout.addWidget(self.logo) layout.addStretch() layout.addWidget(self.user_state) @@ -125,17 +130,23 @@ def __init__(self, parent): super().__init__(parent) self.layout = QHBoxLayout(self) self.setLayout(self.layout) + left_column = QWidget(parent=self) left_layout = QVBoxLayout() left_column.setLayout(left_layout) + self.status = QLabel(_('Waiting to refresh...')) self.error_status = QLabel('') self.error_status.setObjectName('error_label') + left_layout.addWidget(self.status) left_layout.addWidget(self.error_status) + self.source_list = SourceList(left_column) left_layout.addWidget(self.source_list) + self.layout.addWidget(left_column, 2) + self.view_holder = QWidget() self.view_layout = QVBoxLayout() self.view_holder.setLayout(self.view_layout) @@ -187,10 +198,13 @@ def update(self, sources): for source in sources: new_source = SourceWidget(self, source) new_source.setup(self.controller) + list_item = QListWidgetItem(self) list_item.setSizeHint(new_source.sizeHint()) + self.addItem(list_item) self.setItemWidget(list_item, new_source) + if current_maybe and (source.id == current_maybe.source.id): new_current_maybe = list_item @@ -222,6 +236,7 @@ def launch(self): QMessageBox.Cancel | QMessageBox.Yes, QMessageBox.Cancel ) + if reply == QMessageBox.Yes: logger.debug("Deleting source %s" % (self.source.uuid,)) self.controller.delete_source(self.source) @@ -234,6 +249,7 @@ def _construct_message(self, source): messages += 1 else: files += 1 + message = ( "Deleting the Source account for", "{} will also".format(source.journalist_designation,), @@ -257,23 +273,30 @@ def __init__(self, parent, source): """ super().__init__(parent) self.source = source + self.name = QLabel() + self.updated = QLabel() + layout = QVBoxLayout() self.setLayout(layout) + self.summary = QWidget(self) self.summary_layout = QHBoxLayout() self.summary.setLayout(self.summary_layout) + self.attached = load_svg('paperclip.svg') self.attached.setMaximumSize(16, 16) - self.name = QLabel() + self.summary_layout.addWidget(self.name) self.summary_layout.addStretch() self.summary_layout.addWidget(self.attached) + layout.addWidget(self.summary) - self.updated = QLabel() layout.addWidget(self.updated) + self.delete = load_svg('cross.svg') self.delete.setMaximumSize(16, 16) self.delete.mouseReleaseEvent = self.delete_source + self.summary_layout.addWidget(self.delete) self.update() @@ -342,33 +365,44 @@ def setup(self, controller): self.controller = controller self.setMinimumSize(600, 400) self.setWindowTitle(_('Sign in to SecureDrop')) + main_layout = QHBoxLayout() main_layout.addStretch() self.setLayout(main_layout) + form = QWidget() layout = QVBoxLayout() form.setLayout(layout) + main_layout.addWidget(form) main_layout.addStretch() + self.title = QLabel(_('

Sign in

')) self.title.setTextFormat(Qt.RichText) + self.instructions = QLabel(_('You may read all documents and messages ' 'shown here, without signing in. To ' 'correspond with a Source or to check ' 'the server for updates, you must sign ' 'in.')) self.instructions.setWordWrap(True) + self.username_label = QLabel(_('Username')) self.username_field = QLineEdit() + self.password_label = QLabel(_('Password')) self.password_field = QLineEdit() self.password_field.setEchoMode(QLineEdit.Password) + self.tfa_label = QLabel(_('Two-Factor Number')) self.tfa_field = QLineEdit() + self.submit = QPushButton(_('Sign in')) self.submit.clicked.connect(self.validate) + self.error_label = QLabel('') self.error_label.setObjectName('error_label') + layout.addStretch() layout.addWidget(self.title) layout.addWidget(self.instructions) @@ -432,6 +466,7 @@ def validate(self): self.setDisabled(False) self.error(_('Please use only numerals for the two factor number.')) return + self.controller.login(username, password, tfa_token) else: self.setDisabled(False) @@ -451,8 +486,10 @@ def __init__(self, text): super().__init__() layout = QVBoxLayout() self.setLayout(layout) + message = QLabel(html.escape(text, quote=False)) message.setWordWrap(True) + layout.addWidget(message) @@ -470,16 +507,21 @@ def __init__(self, message, align): super().__init__() layout = QHBoxLayout() label = SpeechBubble(message) + if align is not "left": # Float right... layout.addStretch(5) label.setStyleSheet(label.css + 'border-bottom-right-radius: 0px;') + layout.addWidget(label, 6) + if align is "left": # Add space on right hand side... layout.addStretch(5) label.setStyleSheet(label.css + 'border-bottom-left-radius: 0px;') + layout.setContentsMargins(0, 0, 0, 0) + self.setLayout(layout) self.setContentsMargins(0, 0, 0, 0) @@ -526,22 +568,28 @@ def __init__(self, source_db_object, submission_db_object, self.controller = controller self.source = source_db_object self.submission = submission_db_object + layout = QHBoxLayout() icon = QLabel() icon.setPixmap(load_image('file.png')) + if submission_db_object.is_downloaded: description = QLabel("Open") else: human_filesize = humanize_filesize(self.submission.size) description = QLabel("Download ({})".format(human_filesize)) + if align is not "left": # Float right... layout.addStretch(5) + layout.addWidget(icon) layout.addWidget(description, 5) + if align is "left": # Add space on right hand side... layout.addStretch(5) + self.setLayout(layout) def mouseReleaseEvent(self, e): @@ -574,6 +622,7 @@ def __init__(self, parent): self.scroll.setHorizontalScrollBarPolicy(Qt.ScrollBarAlwaysOff) self.scroll.setWidget(self.container) self.scroll.setWidgetResizable(True) + # Completely unintuitive way to ensure the view remains scrolled to the # bottom. sb = self.scroll.verticalScrollBar() @@ -624,7 +673,9 @@ def __init__(self, source, parent, controller): self.source = source self.controller = controller self.text = _("Delete source account") + super().__init__(self.text, parent) + self.messagebox = DeleteSourceMessageBox( parent, self.source, self.controller ) @@ -673,10 +724,13 @@ def __init__(self, source, controller): super().__init__() self.controller = controller self.source = source + ellipsis_icon = load_image("ellipsis.svg") self.setIcon(QIcon(ellipsis_icon)) + self.menu = SourceMenu(self.source, self.controller) self.setMenu(self.menu) + self.setPopupMode(QToolButton.InstantPopup) @@ -701,11 +755,14 @@ def __init__(self, source, controller): super().__init__() self.source = source self.controller = controller + self.layout = QHBoxLayout() self.setLayout(self.layout) + widgets = ( TitleLabel(self.source.journalist_designation), - SourceMenuButton(self.source, self.controller) + SourceMenuButton(self.source, self.controller), ) + for widget in widgets: self.layout.addWidget(widget) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 2f19c75b2..9ea29d98a 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -126,24 +126,28 @@ def setup(self): # The gui needs to reference this "controller" layer to call methods # triggered by UI events. self.gui.setup(self) + # If possible, update the UI with available sources. self.update_sources() + # Show the login dialog. self.gui.show_login() + # Create a timer to check for sync status every 30 seconds. self.sync_timer = QTimer() self.sync_timer.timeout.connect(self.update_sync) self.sync_timer.start(30000) + # Automagically sync with the API every 5 minutes. self.sync_update = QTimer() self.sync_update.timeout.connect(self.sync_api) self.sync_update.start(1000 * 60 * 5) # every 5 minutes. + # Use a QTimer to update the current conversation view such # that as downloads/decryption occur, the messages and replies # populate the view. self.conv_view_update = QTimer() - self.conv_view_update.timeout.connect( - self.update_conversation_view) + self.conv_view_update.timeout.connect(self.update_conversation_view) self.conv_view_update.start(1000 * 60 * 0.10) # every 6 seconds def call_api(self, function, callback, timeout, *args, current_object=None, @@ -163,23 +167,28 @@ def call_api(self, function, callback, timeout, *args, current_object=None, new_api_runner = APICallRunner(function, current_object, *args, **kwargs) new_api_runner.moveToThread(new_api_thread) + # handle completed call: copy response data, reset the # client, give the user-provided callback the response # data new_api_runner.call_finished.connect( lambda: self.completed_api_call(new_thread_id, callback)) + # we've started a timer. when that hits zero, call our # timeout function new_timer.timeout.connect( lambda: self.timeout_cleanup(new_thread_id, timeout)) + # when the thread starts, we want to run `call_api` on `api_runner` new_api_thread.started.connect(new_api_runner.call_api) + # Add the thread related objects to the api_threads dictionary. self.api_threads[new_thread_id] = { 'thread': new_api_thread, 'runner': new_api_runner, 'timer': new_timer, } + # Start the thread and related activity. new_api_thread.start() @@ -205,11 +214,13 @@ def completed_api_call(self, thread_id, user_callback): timer = thread_info['timer'] timer.stop() result_data = runner.result + # The callback may or may not have an associated current_object if runner.current_object: current_object = runner.current_object else: current_object = None + self.clean_thread(thread_id) if current_object: user_callback(result_data, current_object=current_object) @@ -252,10 +263,12 @@ def timeout_cleanup(self, thread_id, user_callback): if thread_id in self.api_threads: runner = self.api_threads[thread_id]['runner'] runner.i_timed_out = True + if runner.current_object: current_object = runner.current_object else: current_object = None + self.clean_thread(thread_id) if current_object: user_callback(current_object=current_object) @@ -283,6 +296,7 @@ def on_authenticate(self, result): self.gui.set_logged_in_as(self.api.username) self.start_message_thread() self.start_reply_thread() + # Clear the sidebar error status bar if a message was shown # to the user indicating they should log in. self.gui.update_error_status("") @@ -498,6 +512,7 @@ def on_file_open(self, submission_db_object): # Running on Qubes. command = "qvm-open-in-vm" args = ['$dispvm:sd-svs-disp', submission_filepath] + # QProcess (Qt) or Python's subprocess? Who cares? They do the # same thing. :-) process = QProcess(self) @@ -527,6 +542,7 @@ def on_file_download(self, source_db_object, message): sdk_object = sdclientapi.Reply(uuid=message.uuid) sdk_object.filename = message.filename sdk_object.source_uuid = source_db_object.uuid + self.set_status(_('Downloading {}'.format(sdk_object.filename))) self.call_api(func, self.on_file_downloaded, self.on_download_timeout, sdk_object, self.data_dir, diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index 21687b398..199e2fa9f 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -68,10 +68,12 @@ def get_remote_data(api): # Log any errors but allow the caller to handle the exception. logger.error(ex) raise(ex) + logger.info('Fetched {} remote sources.'.format(len(remote_sources))) logger.info('Fetched {} remote submissions.'.format( len(remote_submissions))) logger.info('Fetched {} remote replies.'.format(len(remote_replies))) + return (remote_sources, remote_submissions, remote_replies) @@ -85,6 +87,7 @@ def update_local_storage(session, remote_sources, remote_submissions, local_sources = get_local_sources(session) local_submissions = get_local_submissions(session) local_replies = get_local_replies(session) + update_sources(remote_sources, local_sources, session, data_dir) update_submissions(remote_submissions, local_submissions, session, data_dir) update_replies(remote_replies, local_replies, session, data_dir) @@ -114,6 +117,7 @@ def update_sources(remote_sources, local_sources, session, data_dir): local_source.document_count = source.number_of_documents local_source.is_starred = source.is_starred local_source.last_updated = parse(source.last_updated) + # Removing the UUID from local_uuids ensures this record won't be # deleted at the end of this function. local_uuids.remove(source.uuid) @@ -130,13 +134,16 @@ def update_sources(remote_sources, local_sources, session, data_dir): document_count=source.number_of_documents) session.add(ns) logger.info('Added new source {}'.format(source.uuid)) + # The uuids remaining in local_uuids do not exist on the remote server, so # delete the related records. for deleted_source in [s for s in local_sources if s.uuid in local_uuids]: for document in deleted_source.collection: delete_single_submission_or_reply_on_disk(document, data_dir) + session.delete(deleted_source) logger.info('Deleted source {}'.format(deleted_source.uuid)) + session.commit() @@ -157,6 +164,7 @@ def update_submissions(remote_submissions, local_submissions, session, data_dir) local_submission.size = submission.size local_submission.is_read = submission.is_read local_submission.download_url = submission.download_url + # Removing the UUID from local_uuids ensures this record won't be # deleted at the end of this function. local_uuids.remove(submission.uuid) @@ -171,6 +179,7 @@ def update_submissions(remote_submissions, local_submissions, session, data_dir) download_url=submission.download_url) session.add(ns) logger.info('Added new submission {}'.format(submission.uuid)) + # The uuids remaining in local_uuids do not exist on the remote server, so # delete the related records. for deleted_submission in [s for s in local_submissions @@ -178,6 +187,7 @@ def update_submissions(remote_submissions, local_submissions, session, data_dir) delete_single_submission_or_reply_on_disk(deleted_submission, data_dir) session.delete(deleted_submission) logger.info('Deleted submission {}'.format(deleted_submission.uuid)) + session.commit() @@ -201,6 +211,7 @@ def update_replies(remote_replies, local_replies, session, data_dir): local_reply.journalist_id = user.id local_reply.filename = reply.filename local_reply.size = reply.size + local_uuids.remove(reply.uuid) logger.info('Updated reply {}'.format(reply.uuid)) else: @@ -212,12 +223,14 @@ def update_replies(remote_replies, local_replies, session, data_dir): nr = Reply(reply.uuid, user, source, reply.filename, reply.size) session.add(nr) logger.info('Added new reply {}'.format(reply.uuid)) + # The uuids remaining in local_uuids do not exist on the remote server, so # delete the related records. for deleted_reply in [r for r in local_replies if r.uuid in local_uuids]: delete_single_submission_or_reply_on_disk(deleted_reply, data_dir) session.delete(deleted_reply) logger.info('Deleted reply {}'.format(deleted_reply.uuid)) + session.commit() From 5815adb702ec61818a442a3b27bca231b878ba2a Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 19 Dec 2018 13:39:49 +0100 Subject: [PATCH 04/14] cached the conversation views to prevent UI flicker --- securedrop_client/gui/main.py | 46 +++++++++++++++++--------------- securedrop_client/gui/widgets.py | 6 +++-- tests/gui/test_widgets.py | 4 +-- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 452324866..22ea6ab91 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -182,33 +182,37 @@ def show_conversation_for(self, source): Show conversation of messages and replies between a source and journalists. """ - conversation = ConversationView(self) - conversation.setup(self.controller) - # Display each conversation item in the source collection. - for conversation_item in source.collection: + conversation_container = self.conversations.get(source.uuid, None) - if conversation_item.filename.endswith('msg.gpg'): - self.add_item_content_or(conversation.add_message, - conversation_item, - "") - elif conversation_item.filename.endswith('reply.gpg'): - self.add_item_content_or(conversation.add_reply, - conversation_item, - "") - else: - conversation.add_file(source, conversation_item) + if conversation_container is None: + conversation = ConversationView(self) + conversation.setup(self.controller) - container = QWidget() - layout = QVBoxLayout() - container.setLayout(layout) + # Display each conversation item in the source collection. + for conversation_item in source.collection: - source_profile = SourceProfileShortWidget(source, self.controller) + if conversation_item.filename.endswith('msg.gpg'): + self.add_item_content_or(conversation.add_message, + conversation_item, + "") + elif conversation_item.filename.endswith('reply.gpg'): + self.add_item_content_or(conversation.add_reply, + conversation_item, + "") + else: + conversation.add_file(source, conversation_item) - layout.addWidget(source_profile) - layout.addWidget(conversation) + conversation_container = QWidget() + layout = QVBoxLayout() + conversation_container.setLayout(layout) - self.main_view.update_view(container) + source_profile = SourceProfileShortWidget(source, self.controller) + + layout.addWidget(source_profile) + layout.addWidget(conversation) + + self.main_view.set_conversation(conversation_container) def set_status(self, message, duration=5000): """ diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 9d0bf868a..826f91588 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -162,15 +162,17 @@ def setup(self, controller): def update_error_status(self, error=None): self.error_status.setText(html.escape(error)) - def update_view(self, widget): + def set_conversation(self, widget): """ Update the view holder to contain the referenced widget. """ old_widget = self.view_layout.takeAt(0) + if old_widget: old_widget.widget().setVisible(False) - widget.setVisible(True) + self.view_layout.addWidget(widget) + widget.setVisible(True) class SourceList(QListWidget): diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 6cc22c439..75104cc2c 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -117,7 +117,7 @@ def test_MainView_init(): assert isinstance(mv.view_holder, QWidget) -def test_MainView_update_view(mocker): +def test_MainView_show_conversation(mocker): """ Ensure the passed-in widget is added to the layout of the main view holder (i.e. that area of the screen on the right hand side). @@ -125,7 +125,7 @@ def test_MainView_update_view(mocker): mv = MainView(None) mv.view_layout = mocker.MagicMock() mock_widget = mocker.MagicMock() - mv.update_view(mock_widget) + mv.set_conversation(mock_widget) mv.view_layout.takeAt.assert_called_once_with(0) mv.view_layout.addWidget.assert_called_once_with(mock_widget) From e5e15d6894dc28ec2256f8d0e73c59b0c2b79a84 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 19 Dec 2018 14:09:51 +0100 Subject: [PATCH 05/14] move coversation display logic into the relevant widget --- securedrop_client/gui/main.py | 25 +-------------------- securedrop_client/gui/widgets.py | 37 +++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 22ea6ab91..56366d814 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -168,15 +168,6 @@ def on_source_changed(self): self.current_source = source_widget.source self.show_conversation_for(self.current_source) - def add_item_content_or(self, adder, item, default): - """ - Private helper function to add correct message to conversation widgets - """ - if item.is_downloaded is False: - adder(default) - else: - adder(get_data(self.sdc_home, item.filename)) - def show_conversation_for(self, source): """ Show conversation of messages and replies between a source and @@ -186,23 +177,9 @@ def show_conversation_for(self, source): conversation_container = self.conversations.get(source.uuid, None) if conversation_container is None: - conversation = ConversationView(self) + conversation = ConversationView(source, parent=self) conversation.setup(self.controller) - # Display each conversation item in the source collection. - for conversation_item in source.collection: - - if conversation_item.filename.endswith('msg.gpg'): - self.add_item_content_or(conversation.add_message, - conversation_item, - "") - elif conversation_item.filename.endswith('reply.gpg'): - self.add_item_content_or(conversation.add_reply, - conversation_item, - "") - else: - conversation.add_file(source, conversation_item) - conversation_container = QWidget() layout = QVBoxLayout() conversation_container.setLayout(layout) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 826f91588..5d4fb6269 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -612,8 +612,10 @@ class ConversationView(QWidget): Renders a conversation. """ - def __init__(self, parent): + def __init__(self, source_db_object, parent=None): super().__init__(parent) + self.source = source_db_object + self.container = QWidget() self.conversation_layout = QVBoxLayout() self.container.setLayout(self.conversation_layout) @@ -634,6 +636,39 @@ def __init__(self, parent): main_layout.addWidget(self.scroll) self.setLayout(main_layout) + self.update_conversation(self.source.collection) + + def update_conversation(self, collection: list) -> None: + # clear all old items + while True: + w = self.conversation_layout.takeAt(0) + if w: + del w + else: + break + + # add new items + for conversation_item in collection: + if conversation_item.filename.endswith('msg.gpg'): + self.add_item_content_or(self.add_message, + conversation_item, + "") + elif conversation_item.filename.endswith('reply.gpg'): + self.add_item_content_or(self.add_reply, + conversation_item, + "") + else: + self.add_file(self.source, conversation_item) + + def add_item_content_or(self, adder, item, default): + """ + Private helper function to add correct message to conversation widgets + """ + if item.is_downloaded is False: + adder(default) + else: + adder(item.content) + def setup(self, controller): """ Ensure there's a reference to program logic. From c5829de96dbc0e8fb712132c62265d836616df29 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 20 Dec 2018 13:58:06 +0100 Subject: [PATCH 06/14] added whitespace to tests for clarity --- tests/gui/test_widgets.py | 79 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 75104cc2c..80fde15d2 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -28,9 +28,12 @@ def test_ToolBar_setup(mocker): them becoming attributes of self. """ tb = ToolBar(None) + mock_window = mocker.MagicMock() mock_controller = mocker.MagicMock() + tb.setup(mock_window, mock_controller) + assert tb.window == mock_window assert tb.controller == mock_controller @@ -40,11 +43,14 @@ def test_ToolBar_set_logged_in_as(mocker): buttons, and refresh buttons, are in the correct state. """ tb = ToolBar(None) + tb.user_state = mocker.MagicMock() tb.login = mocker.MagicMock() tb.logout = mocker.MagicMock() tb.refresh = mocker.MagicMock() + tb.set_logged_in_as('test') + tb.user_state.setText.assert_called_once_with('Signed in as: test') tb.login.setVisible.assert_called_once_with(False) tb.logout.setVisible.assert_called_once_with(True) @@ -56,11 +62,14 @@ def test_ToolBar_set_logged_out(mocker): Ensure the UI reverts to the logged out state. """ tb = ToolBar(None) + tb.user_state = mocker.MagicMock() tb.login = mocker.MagicMock() tb.logout = mocker.MagicMock() tb.refresh = mocker.MagicMock() + tb.set_logged_out() + tb.user_state.setText.assert_called_once_with('Signed out.') tb.login.setVisible.assert_called_once_with(True) tb.logout.setVisible.assert_called_once_with(False) @@ -124,8 +133,10 @@ def test_MainView_show_conversation(mocker): """ mv = MainView(None) mv.view_layout = mocker.MagicMock() + mock_widget = mocker.MagicMock() mv.set_conversation(mock_widget) + mv.view_layout.takeAt.assert_called_once_with(0) mv.view_layout.addWidget.assert_called_once_with(mock_widget) @@ -137,8 +148,10 @@ def test_MainView_update_error_status(mocker): """ mv = MainView(None) expected_message = "this is the message to be displayed" + mv.error_status = mocker.MagicMock() mv.error_status.setText = mocker.MagicMock() + mv.update_error_status(error=expected_message) mv.error_status.setText.assert_called_once_with(expected_message) @@ -149,6 +162,7 @@ def test_SourceList_update(mocker): each passed-in source is created along with an associated QListWidgetItem. """ sl = SourceList(None) + sl.clear = mocker.MagicMock() sl.addItem = mocker.MagicMock() sl.setItemWidget = mocker.MagicMock() @@ -161,6 +175,7 @@ def test_SourceList_update(mocker): sources = ['a', 'b', 'c', ] sl.update(sources) + sl.clear.assert_called_once_with() assert mock_sw.call_count == len(sources) assert mock_lwi.call_count == len(sources) @@ -200,8 +215,10 @@ def test_SourceWidget_setup(mocker): """ mock_controller = mocker.MagicMock() mock_source = mocker.MagicMock() + sw = SourceWidget(None, mock_source) sw.setup(mock_controller) + assert sw.controller == mock_controller @@ -212,11 +229,14 @@ def test_SourceWidget_html_init(mocker): """ mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' + sw = SourceWidget(None, mock_source) sw.name = mocker.MagicMock() sw.summary_layout = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.load_svg') sw.update() + sw.name.setText.assert_called_once_with('foo <b>bar</b> baz') @@ -227,11 +247,14 @@ def test_SourceWidget_update_starred(mocker): mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.is_starred = True + sw = SourceWidget(None, mock_source) sw.name = mocker.MagicMock() sw.summary_layout = mocker.MagicMock() + mock_load = mocker.patch('securedrop_client.gui.widgets.load_svg') sw.update() + mock_load.assert_called_once_with('star_on.svg') sw.name.setText.assert_called_once_with('foo bar baz') @@ -243,11 +266,14 @@ def test_SourceWidget_update_unstarred(mocker): mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.is_starred = False + sw = SourceWidget(None, mock_source) sw.name = mocker.MagicMock() sw.summary_layout = mocker.MagicMock() + mock_load = mocker.patch('securedrop_client.gui.widgets.load_svg') sw.update() + mock_load.assert_called_once_with('star_off.svg') sw.name.setText.assert_called_once_with('foo bar baz') @@ -275,9 +301,11 @@ def test_SourceWidget_toggle_star(mocker): mock_controller = mocker.MagicMock() mock_source = mocker.MagicMock() event = mocker.MagicMock() + sw = SourceWidget(None, mock_source) sw.controller = mock_controller sw.controller.update_star = mocker.MagicMock() + sw.toggle_star(event) sw.controller.update_star.assert_called_once_with(mock_source) @@ -304,9 +332,11 @@ def test_SourceWidget_delete_source(mocker): def test_SourceWidget_delete_source_when_user_chooses_cancel(mocker): mock_message_box_question = mocker.MagicMock(QMessageBox.question) mock_message_box_question.return_value = QMessageBox.Cancel + mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.submissions = [] + submission_files = ( "submission_1-msg.gpg", "submission_2-msg.gpg", @@ -316,9 +346,11 @@ def test_SourceWidget_delete_source_when_user_chooses_cancel(mocker): submission = mocker.MagicMock() submission.filename = filename mock_source.submissions.append(submission) + mock_controller = mocker.MagicMock() sw = SourceWidget(None, mock_source) sw.controller = mock_controller + mocker.patch( "securedrop_client.gui.widgets.QMessageBox.question", mock_message_box_question, @@ -343,6 +375,7 @@ def test_LoginDialog_reset(mocker): Ensure the state of the login view is returned to the correct state. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field = mocker.MagicMock() @@ -350,7 +383,9 @@ def test_LoginDialog_reset(mocker): ld.tfa_field = mocker.MagicMock() ld.setDisabled = mocker.MagicMock() ld.error_label = mocker.MagicMock() + ld.reset() + ld.username_field.setText.assert_called_once_with('') ld.password_field.setText.assert_called_once_with('') ld.tfa_field.setText.assert_called_once_with('') @@ -375,6 +410,7 @@ def test_LoginDialog_validate_no_input(mocker): If the user doesn't provide input, tell them and give guidance. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field.text = mocker.MagicMock(return_value='') @@ -382,7 +418,9 @@ def test_LoginDialog_validate_no_input(mocker): ld.tfa_field.text = mocker.MagicMock(return_value='') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 2 assert ld.error.call_count == 1 @@ -393,6 +431,7 @@ def test_LoginDialog_validate_input_non_numeric_2fa(mocker): guidance. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field.text = mocker.MagicMock(return_value='foo') @@ -400,7 +439,9 @@ def test_LoginDialog_validate_input_non_numeric_2fa(mocker): ld.tfa_field.text = mocker.MagicMock(return_value='baz') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 2 assert ld.error.call_count == 1 assert mock_controller.login.call_count == 0 @@ -411,6 +452,7 @@ def test_LoginDialog_validate_too_short_username(mocker): If the username is too small, we show an informative error message. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field.text = mocker.MagicMock(return_value='he') @@ -418,7 +460,9 @@ def test_LoginDialog_validate_too_short_username(mocker): ld.tfa_field.text = mocker.MagicMock(return_value='123456') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 2 assert ld.error.call_count == 1 assert mock_controller.login.call_count == 0 @@ -429,6 +473,7 @@ def test_LoginDialog_validate_too_short_password(mocker): If the password is too small, we show an informative error message. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field.text = mocker.MagicMock(return_value='foo') @@ -436,7 +481,9 @@ def test_LoginDialog_validate_too_short_password(mocker): ld.tfa_field.text = mocker.MagicMock(return_value='123456') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 2 assert ld.error.call_count == 1 assert mock_controller.login.call_count == 0 @@ -452,12 +499,15 @@ def test_LoginDialog_validate_too_long_password(mocker): max_password_len = 128 too_long_password = 'a' * (max_password_len + 1) + ld.username_field.text = mocker.MagicMock(return_value='foo') ld.password_field.text = mocker.MagicMock(return_value=too_long_password) ld.tfa_field.text = mocker.MagicMock(return_value='123456') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 2 assert ld.error.call_count == 1 assert mock_controller.login.call_count == 0 @@ -468,6 +518,7 @@ def test_LoginDialog_validate_input_ok(mocker): Valid input from the user causes a call to the controller's login method. """ mock_controller = mocker.MagicMock() + ld = LoginDialog(None) ld.setup(mock_controller) ld.username_field.text = mocker.MagicMock(return_value='foo') @@ -475,7 +526,9 @@ def test_LoginDialog_validate_input_ok(mocker): ld.tfa_field.text = mocker.MagicMock(return_value='123456') ld.setDisabled = mocker.MagicMock() ld.error = mocker.MagicMock() + ld.validate() + assert ld.setDisabled.call_count == 1 assert ld.error.call_count == 0 mock_controller.login.assert_called_once_with('foo', 'nicelongpassword', '123456') @@ -663,8 +716,10 @@ def test_ConversationView_add_message(mocker): cv = ConversationView(None) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() + cv.add_message('hello') assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], MessageWidget) @@ -676,8 +731,10 @@ def test_ConversationView_add_reply(mocker): cv = ConversationView(None) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() + cv.add_reply('hello') assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], ReplyWidget) @@ -690,15 +747,18 @@ def test_ConversationView_add_downloaded_file(mocker): cv = ConversationView(None) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() + mock_source = mocker.MagicMock() mock_file = mocker.MagicMock() mock_file.is_downloaded = True mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel') mocker.patch('securedrop_client.gui.widgets.QHBoxLayout.addWidget') mocker.patch('securedrop_client.gui.widgets.FileWidget.setLayout') + cv.add_file(mock_source, mock_file) mock_label.assert_called_with("Open") assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], FileWidget) @@ -711,6 +771,7 @@ def test_ConversationView_add_not_downloaded_file(mocker): cv = ConversationView(None) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() + mock_source = mocker.MagicMock() mock_file = mocker.MagicMock() mock_file.is_downloaded = False @@ -718,9 +779,11 @@ def test_ConversationView_add_not_downloaded_file(mocker): mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel') mocker.patch('securedrop_client.gui.widgets.QHBoxLayout.addWidget') mocker.patch('securedrop_client.gui.widgets.FileWidget.setLayout') + cv.add_file(mock_source, mock_file) mock_label.assert_called_with("Download (123 bytes)") assert cv.conversation_layout.addWidget.call_count == 1 + cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], FileWidget) @@ -737,6 +800,7 @@ def test_DeleteSourceMessage_launch_when_user_chooses_cancel(mocker): mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.submissions = [] + submission_files = ( "submission_1-msg.gpg", "submission_2-msg.gpg", @@ -746,6 +810,7 @@ def test_DeleteSourceMessage_launch_when_user_chooses_cancel(mocker): submission = mocker.MagicMock() submission.filename = filename mock_source.submissions.append(submission) + mock_controller = mocker.MagicMock() delete_source_message_box = DeleteSourceMessageBox( None, mock_source, mock_controller @@ -754,6 +819,7 @@ def test_DeleteSourceMessage_launch_when_user_chooses_cancel(mocker): "securedrop_client.gui.widgets.QMessageBox.question", mock_message_box_question, ) + delete_source_message_box.launch() mock_controller.delete_source.assert_not_called() @@ -764,6 +830,7 @@ def test_DeleteSourceMssageBox_launch_when_user_chooses_yes(mocker): mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.submissions = [] + submission_files = ( "submission_1-msg.gpg", "submission_2-msg.gpg", @@ -773,6 +840,7 @@ def test_DeleteSourceMssageBox_launch_when_user_chooses_yes(mocker): submission = mocker.MagicMock() submission.filename = filename mock_source.submissions.append(submission) + mock_controller = mocker.MagicMock() delete_source_message_box = DeleteSourceMessageBox( None, mock_source, mock_controller @@ -781,8 +849,10 @@ def test_DeleteSourceMssageBox_launch_when_user_chooses_yes(mocker): "securedrop_client.gui.widgets.QMessageBox.question", mock_message_box_question, ) + delete_source_message_box.launch() mock_controller.delete_source.assert_called_once_with(mock_source) + message = ( "Deleting the Source account for " "foo bar baz will also " @@ -805,6 +875,7 @@ def test_DeleteSourceMessageBox_construct_message(mocker): mock_source = mocker.MagicMock() mock_source.journalist_designation = 'foo bar baz' mock_source.submissions = [] + submission_files = ( "submission_1-msg.gpg", "submission_2-msg.gpg", @@ -814,10 +885,12 @@ def test_DeleteSourceMessageBox_construct_message(mocker): submission = mocker.MagicMock() submission.filename = filename mock_source.submissions.append(submission) + delete_source_message_box = DeleteSourceMessageBox( None, mock_source, mock_controller ) message = delete_source_message_box._construct_message(mock_source) + expected_message = ( "Deleting the Source account for " "foo bar baz will also " @@ -825,8 +898,7 @@ def test_DeleteSourceMessageBox_construct_message(mocker): "
" "This Source will no longer be able to correspond " "through the log-in tied to this account." - ) - expected_message = expected_message.format(files=1, messages=2) + ).format(files=1, messages=2) assert message == expected_message @@ -848,6 +920,7 @@ def test_DeleteSourceAction_trigger(mocker): mock_delete_source_message_box.return_value = ( mock_delete_source_message_box_obj ) + with mocker.patch( 'securedrop_client.gui.widgets.DeleteSourceMessageBox', mock_delete_source_message_box @@ -870,6 +943,7 @@ def test_DeleteSource_from_source_menu_when_user_is_loggedout(mocker): mock_delete_source_message_box.return_value = ( mock_delete_source_message_box_obj ) + with mocker.patch( 'securedrop_client.gui.widgets.DeleteSourceMessageBox', mock_delete_source_message_box @@ -889,6 +963,7 @@ def test_DeleteSource_from_source_widget_when_user_is_loggedout(mocker): mock_delete_source_message_box.return_value = ( mock_delete_source_message_box_obj ) + with mocker.patch( 'securedrop_client.gui.widgets.DeleteSourceMessageBox', mock_delete_source_message_box From 429448b83df375dc6613d0c27b4bc3aa3db3303e Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 20 Dec 2018 14:09:03 +0100 Subject: [PATCH 07/14] updated conversation view tests --- tests/gui/test_widgets.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 80fde15d2..6a39549dd 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -680,11 +680,12 @@ def test_FileWidget_mousePressEvent_open(mocker): fw.controller.on_file_open.assert_called_once_with(submission) -def test_ConversationView_init(): +def test_ConversationView_init(mocker): """ Ensure the conversation view has a layout to add widgets to. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) assert isinstance(cv.conversation_layout, QVBoxLayout) @@ -692,7 +693,8 @@ def test_ConversationView_setup(mocker): """ Ensure the controller is set """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) mock_controller = mocker.MagicMock() cv.setup(mock_controller) assert cv.controller == mock_controller @@ -703,7 +705,8 @@ def test_ConversationView_move_to_bottom(mocker): Check the signal handler sets the correct value for the scrollbar to be the maximum possible value. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) cv.scroll = mocker.MagicMock() cv.move_to_bottom(0, 6789) cv.scroll.verticalScrollBar().setValue.assert_called_once_with(6789) @@ -713,7 +716,8 @@ def test_ConversationView_add_message(mocker): """ Adding a message results in a new MessageWidget added to the layout. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() @@ -728,7 +732,8 @@ def test_ConversationView_add_reply(mocker): """ Adding a reply results in a new ReplyWidget added to the layout. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() @@ -744,7 +749,8 @@ def test_ConversationView_add_downloaded_file(mocker): Adding a file results in a new FileWidget added to the layout with the proper QLabel. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() @@ -768,7 +774,8 @@ def test_ConversationView_add_not_downloaded_file(mocker): Adding a file results in a new FileWidget added to the layout with the proper QLabel. """ - cv = ConversationView(None) + mocked_source = mocker.MagicMock() + cv = ConversationView(mocked_source) cv.controller = mocker.MagicMock() cv.conversation_layout = mocker.MagicMock() From 1b7cb73b302a5bd506f8ee70d7cccf0920d52333 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 20 Dec 2018 14:23:50 +0100 Subject: [PATCH 08/14] updated tests on main conversation --- tests/gui/test_main.py | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 1396cede5..082190f38 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -185,7 +185,6 @@ def test_conversation_for(mocker): w = Window('mock') w.controller = mocker.MagicMock() w.main_view = mocker.MagicMock() - mock_conview = mocker.MagicMock() mock_source = mocker.MagicMock() mock_source.journalistic_designation = 'Testy McTestface' mock_file = mocker.MagicMock() @@ -196,16 +195,30 @@ def test_conversation_for(mocker): mock_reply.filename = '3-my-source-reply.gpg' mock_source.collection = [mock_file, mock_message, mock_reply] - mocker.patch('securedrop_client.gui.main.ConversationView', mock_conview) + mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') + mocked_add_reply = mocker.patch('securedrop_client.gui.main.ConversationView.add_reply') + mocked_add_file = mocker.patch('securedrop_client.gui.main.ConversationView.add_file') mocker.patch('securedrop_client.gui.main.QVBoxLayout') mocker.patch('securedrop_client.gui.main.QWidget') w.show_conversation_for(mock_source) - conv = mock_conview() - assert conv.add_message.call_count > 0 - assert conv.add_reply.call_count > 0 - assert conv.add_file.call_count > 0 + assert mocked_add_message.call_count > 0 + assert mocked_add_reply.call_count > 0 + assert mocked_add_file.call_count > 0 + + # check that showing the conversation a second time doesn't break anything + + # use new mocks to check the count again + mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') + mocked_add_reply = mocker.patch('securedrop_client.gui.main.ConversationView.add_reply') + mocked_add_file = mocker.patch('securedrop_client.gui.main.ConversationView.add_file') + + w.show_conversation_for(mock_source) + + assert mocked_add_message.call_count > 0 + assert mocked_add_reply.call_count > 0 + assert mocked_add_file.call_count > 0 def test_conversation_pending_message(mocker): @@ -217,7 +230,6 @@ def test_conversation_pending_message(mocker): w.controller = mocker.MagicMock() w.main_view = mocker.MagicMock() w._add_item_content_or = mocker.MagicMock() - mock_conview = mocker.MagicMock() mock_source = mocker.MagicMock() mock_source.journalistic_designation = 'Testy McTestface' @@ -229,15 +241,14 @@ def test_conversation_pending_message(mocker): mock_source.collection = [submission] - mocker.patch('securedrop_client.gui.main.ConversationView', mock_conview) + mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') mocker.patch('securedrop_client.gui.main.QVBoxLayout') mocker.patch('securedrop_client.gui.main.QWidget') w.show_conversation_for(mock_source) - conv = mock_conview() - assert conv.add_message.call_count == 1 - assert conv.add_message.call_args == mocker.call("") + assert mocked_add_message.call_count == 1 + assert mocked_add_message.call_args == mocker.call("") def test_set_status(mocker): From 4dfdafc719f771ec2f334a83c1035d3dec77e234 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 9 Jan 2019 14:33:46 +0100 Subject: [PATCH 09/14] fix errors in makefile --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 9d1806c54..96862ee05 100644 --- a/Makefile +++ b/Makefile @@ -3,11 +3,11 @@ SHELL := /bin/bash .PHONY: clean clean: ## Clean the workspace of generated resources - @rm -rf build dist *.egg-info .coverage .eggs docs/_build .pytest_cache lib htmlcov && \ + @rm -rf build dist *.egg-info .coverage .eggs docs/_build .pytest_cache lib htmlcov .cache && \ find . \( -name '*.py[co]' -o -name dropin.cache \) -delete && \ find . \( -name '*.bak' -o -name dropin.cache \) -delete && \ find . \( -name '*.tgz' -o -name dropin.cache \) -delete && \ - find . -name __pycache__ -print0 | xargs rm -rf + find . -name __pycache__ -print0 | xargs -0 rm -rf TESTS ?= tests TESTOPTS ?= -v @@ -34,7 +34,7 @@ check: clean lint test ## Run the full CI test suite # 6. Format columns with colon as delimiter. .PHONY: help help: ## Print this message and exit. - @printf "Makefile for developing and testing SecureDrop.\n" + @printf "Makefile for developing and testing the SecureDrop client.\n" @printf "Subcommands:\n\n" @awk 'BEGIN {FS = ":.*?## "} /^[0-9a-zA-Z_-]+:.*?## / {printf "\033[36m%s\033[0m : %s\n", $$1, $$2}' $(MAKEFILE_LIST) \ | sort \ From 75c78005e0e04787ba9541a7e82802877571bc21 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 9 Jan 2019 14:34:39 +0100 Subject: [PATCH 10/14] updated code to use signals/slots for decrytping/downloading messages --- securedrop_client/gui/main.py | 4 +- securedrop_client/gui/widgets.py | 73 ++++++++++++++++++---------- securedrop_client/logic.py | 81 +++++++++++++++++-------------- securedrop_client/message_sync.py | 29 +++++++++-- 4 files changed, 118 insertions(+), 69 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 56366d814..2016ed08c 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -26,7 +26,6 @@ ConversationView, SourceProfileShortWidget) from securedrop_client.resources import load_icon -from securedrop_client.storage import get_data logger = logging.getLogger(__name__) @@ -177,8 +176,7 @@ def show_conversation_for(self, source): conversation_container = self.conversations.get(source.uuid, None) if conversation_container is None: - conversation = ConversationView(source, parent=self) - conversation.setup(self.controller) + conversation = ConversationView(source, self.sdc_home, self.controller, parent=self) conversation_container = QWidget() layout = QVBoxLayout() diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 5d4fb6269..42af8bdc0 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -19,13 +19,15 @@ import logging import arrow import html -from PyQt5.QtCore import Qt +from PyQt5.QtCore import Qt, pyqtSlot from PyQt5.QtGui import QIcon from PyQt5.QtWidgets import QListWidget, QLabel, QWidget, QListWidgetItem, QHBoxLayout, \ QPushButton, QVBoxLayout, QLineEdit, QScrollArea, QDialog, QAction, QMenu, \ QMessageBox, QToolButton +from securedrop_client.logic import Client from securedrop_client.resources import load_svg, load_image +from securedrop_client.storage import get_data from securedrop_client.utils import humanize_filesize logger = logging.getLogger(__name__) @@ -484,15 +486,28 @@ class SpeechBubble(QWidget): css = "padding: 10px; border: 1px solid #999; border-radius: 20px;" - def __init__(self, text): + def __init__(self, message_id: str, text: str, update_signal) -> None: super().__init__() + self.message_id = message_id + layout = QVBoxLayout() self.setLayout(layout) - message = QLabel(html.escape(text, quote=False)) - message.setWordWrap(True) + self.message = QLabel(html.escape(text, quote=False)) + self.message.setWordWrap(True) - layout.addWidget(message) + layout.addWidget(self.message) + + update_signal.connect(self._update_text) + + @pyqtSlot(str, str) + def _update_text(self, message_id: str, text: str) -> None: + """ + Conditionally update this SpeechBubble's text if and only if the message_id of the emitted + signal matche the message_id of this speech bubble. + """ + if message_id == self.message_id: + self.message.setText(html.escape(text, quote=False)) class ConversationWidget(QWidget): @@ -500,7 +515,11 @@ class ConversationWidget(QWidget): Draws a message onto the screen. """ - def __init__(self, message, align): + def __init__(self, + message_id: str, + message: str, + update_signal, + align: str) -> None: """ Initialise with the message to display and some notion of which side of the conversation ("left" or "right" [anything else]) to which the @@ -508,7 +527,7 @@ def __init__(self, message, align): """ super().__init__() layout = QHBoxLayout() - label = SpeechBubble(message) + label = SpeechBubble(message_id, message, update_signal) if align is not "left": # Float right... @@ -533,8 +552,11 @@ class MessageWidget(ConversationWidget): Represents an incoming message from the source. """ - def __init__(self, message): - super().__init__(message, align="left") + def __init__(self, message_id: str, message: str, update_signal) -> None: + super().__init__(message_id, + message, + update_signal, + align="left") self.setStyleSheet(""" background-color: #EEE; """) @@ -545,8 +567,11 @@ class ReplyWidget(ConversationWidget): Represents a reply to a source. """ - def __init__(self, message): - super().__init__(message, align="right") + def __init__(self, message_id: str, message: str, update_signal) -> None: + super().__init__(message_id, + message, + update_signal, + align="right") self.setStyleSheet(""" background-color: #2299EE; """) @@ -612,9 +637,11 @@ class ConversationView(QWidget): Renders a conversation. """ - def __init__(self, source_db_object, parent=None): + def __init__(self, source_db_object, sdc_home: str, controller: Client, parent=None): super().__init__(parent) self.source = source_db_object + self.sdc_home = sdc_home + self.controller = controller self.container = QWidget() self.conversation_layout = QVBoxLayout() @@ -642,7 +669,7 @@ def update_conversation(self, collection: list) -> None: # clear all old items while True: w = self.conversation_layout.takeAt(0) - if w: + if w: # pragma: no cover del w else: break @@ -665,15 +692,9 @@ def add_item_content_or(self, adder, item, default): Private helper function to add correct message to conversation widgets """ if item.is_downloaded is False: - adder(default) + adder(item.uuid, default) else: - adder(item.content) - - def setup(self, controller): - """ - Ensure there's a reference to program logic. - """ - self.controller = controller + adder(item.uuid, get_data(self.sdc_home, item.filename)) def add_file(self, source_db_object, submission_db_object): """ @@ -690,17 +711,19 @@ def move_to_bottom(self, min_val, max_val): """ self.scroll.verticalScrollBar().setValue(max_val) - def add_message(self, message): + def add_message(self, message_id: str, message: str) -> None: """ Add a message from the source. """ - self.conversation_layout.addWidget(MessageWidget(message)) + self.conversation_layout.addWidget( + MessageWidget(message_id, message, self.controller.message_sync.message_downloaded)) - def add_reply(self, reply, files=None): + def add_reply(self, message_id: str, reply: str, files=None) -> None: """ Add a reply from a journalist. """ - self.conversation_layout.addWidget(ReplyWidget(reply)) + self.conversation_layout.addWidget( + ReplyWidget(message_id, reply, self.controller.reply_sync.reply_downloaded)) class DeleteSourceAction(QAction): diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 9ea29d98a..f19eb3961 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -96,22 +96,42 @@ def __init__(self, hostname, gui, session, various other layers of the application: the location of the SecureDrop proxy, the user interface and SqlAlchemy local storage respectively. """ - check_dir_permissions(home) - super().__init__() - self.hostname = hostname # Location of the SecureDrop server. - self.gui = gui # Reference to the UI window. - self.api = None # Reference to the API for secure drop proxy. - self.session = session # Reference to the SqlAlchemy session. - self.message_thread = None # thread responsible for fetching messages - self.reply_thread = None # thread responsible for fetching replies - self.home = home # used for finding DB in sync thread - self.api_threads = {} # Contains active threads calling the API. - self.sync_flag = os.path.join(home, 'sync_flag') - self.data_dir = os.path.join(self.home, 'data') # File data. - self.timer = None # call timeout timer + + # used for finding DB in sync thread + self.home = home + + # boolean flag for whether or not the client is operating behind a proxy self.proxy = proxy + + # Location of the SecureDrop server. + self.hostname = hostname + + # Reference to the UI window. + self.gui = gui + + # Reference to the API for secure drop proxy. + self.api = None + # Contains active threads calling the API. + self.api_threads = {} + + # Reference to the SqlAlchemy session. + self.session = session + + # thread responsible for fetching messages + self.message_thread = None + self.message_sync = MessageSync(self.api, self.home, self.proxy) + + # thread responsible for fetching replies + self.reply_thread = None + self.reply_sync = ReplySync(self.api, self.home, self.proxy) + + self.sync_flag = os.path.join(home, 'sync_flag') + + # File data. + self.data_dir = os.path.join(self.home, 'data') + self.gpg = GpgHelper(home, proxy) def setup(self): @@ -143,13 +163,6 @@ def setup(self): self.sync_update.timeout.connect(self.sync_api) self.sync_update.start(1000 * 60 * 5) # every 5 minutes. - # Use a QTimer to update the current conversation view such - # that as downloads/decryption occur, the messages and replies - # populate the view. - self.conv_view_update = QTimer() - self.conv_view_update.timeout.connect(self.update_conversation_view) - self.conv_view_update.start(1000 * 60 * 0.10) # every 6 seconds - def call_api(self, function, callback, timeout, *args, current_object=None, **kwargs): """ @@ -232,8 +245,8 @@ def start_message_thread(self): Starts the message-fetching thread in the background. """ if not self.message_thread: + self.message_sync.api = self.api self.message_thread = QThread() - self.message_sync = MessageSync(self.api, self.home, self.proxy) self.message_sync.moveToThread(self.message_thread) self.message_thread.started.connect(self.message_sync.run) self.message_thread.start() @@ -245,8 +258,8 @@ def start_reply_thread(self): Starts the reply-fetching thread in the background. """ if not self.reply_thread: + self.reply_sync.api = self.api self.reply_thread = QThread() - self.reply_sync = ReplySync(self.api, self.home, self.proxy) self.reply_sync.moveToThread(self.reply_thread) self.reply_thread.started.connect(self.reply_sync.run) self.reply_thread.start() @@ -406,8 +419,7 @@ def on_synced(self, result): except CryptoError: logger.warning('Failed to import key for source {}'.format(source.uuid)) - # TODO: show something in the conversation view? - # self.gui.show_conversation_for() + self.update_conversation_views() else: # How to handle a failure? Exceptions are already logged. Perhaps # a message in the UI? @@ -431,16 +443,14 @@ def update_sources(self): self.gui.show_sources(sources) self.update_sync() - def update_conversation_view(self): + def update_conversation_views(self): """ Updates the conversation view to reflect progress of the download and decryption of messages and replies. """ - # Redraw the conversation view if we have clicked on a source - # and the source has not been deleted. - if self.gui.current_source and self.gui.current_source in self.session: - self.session.refresh(self.gui.current_source) - self.gui.show_conversation_for(self.gui.current_source) + for conversation in self.gui.conversations.values(): + self.session.refresh(conversation.source) + conversation.update_conversation(conversation.source.collection) def on_update_star_complete(self, result): """ @@ -569,7 +579,9 @@ def on_file_downloaded(self, result, current_object): # Attempt to decrypt the file. self.gpg.decrypt_submission_or_reply( filepath_in_datadir, server_filename, is_doc=True) - except CryptoError: + except CryptoError as e: + logger.debug('Failed to decrypt file {}: {}'.format(server_filename, e)) + self.set_status("Failed to download and decrypt file, " "please try again.") # TODO: We should save the downloaded content, and just @@ -579,12 +591,9 @@ def on_file_downloaded(self, result, current_object): # Now that download and decrypt are done, mark the file as such. storage.mark_file_as_downloaded(file_uuid, self.session) - # Refresh the current source conversation, bearing in mind - # that the user may have navigated to another source. - self.gui.show_conversation_for(self.gui.current_source) - self.set_status( - 'Finished downloading {}'.format(current_object.filename)) + self.set_status('Finished downloading {}'.format(current_object.filename)) else: # The file did not download properly. + logger.debug('Failed to download file {}'.format(server_filename)) # Update the UI in some way to indicate a failure state. self.set_status("The file download failed. Please try again.") diff --git a/securedrop_client/message_sync.py b/securedrop_client/message_sync.py index b4afb13a4..e05fe2c87 100644 --- a/securedrop_client/message_sync.py +++ b/securedrop_client/message_sync.py @@ -22,11 +22,11 @@ import logging import sdclientapi.sdlocalobjects as sdkobjects -from PyQt5.QtCore import QObject +from PyQt5.QtCore import QObject, pyqtSignal from securedrop_client import storage from securedrop_client.crypto import GpgHelper from securedrop_client.db import make_engine - +from securedrop_client.storage import get_data from sqlalchemy.orm import sessionmaker @@ -58,11 +58,18 @@ class MessageSync(APISyncObject): Runs in the background, finding messages to download and downloading them. """ + """ + Signal emitted notifying that a message has been downloaded. The signal is a tuple of + (str, str) containing the message's UUID and the content of the message. + """ + message_downloaded = pyqtSignal([str, str]) + def __init__(self, api, home, is_qubes): super().__init__(api, home, is_qubes) def run(self, loop=True): while True: + logger.debug('Syncing messages.') submissions = storage.find_new_submissions(self.session) for db_submission in submissions: @@ -79,12 +86,15 @@ def run(self, loop=True): db_submission, self.api.download_submission, storage.mark_file_as_downloaded) - + self.message_downloaded.emit(db_submission.uuid, + get_data(self.home, db_submission.filename)) except Exception as e: logger.critical( "Exception while downloading submission! {}".format(e) ) + logger.debug('Completed message sync.') + if not loop: break else: @@ -96,16 +106,22 @@ class ReplySync(APISyncObject): Runs in the background, finding replies to download and downloading them. """ + """ + Signal emitted notifying that a reply has been downloaded. The signal is a tuple of + (str, str) containing the message's UUID and the content of the reply. + """ + reply_downloaded = pyqtSignal([str, str]) + def __init__(self, api, home, is_qubes): super().__init__(api, home, is_qubes) def run(self, loop=True): while True: + logger.debug('Syncing replies.') replies = storage.find_new_replies(self.session) for db_reply in replies: try: - # the API wants API objects. here in the client, # we have client objects. let's take care of that # here @@ -121,12 +137,15 @@ def run(self, loop=True): db_reply, self.api.download_reply, storage.mark_reply_as_downloaded) - + self.reply_downloaded.emit(db_reply.uuid, + get_data(self.home, db_reply.filename)) except Exception as e: logger.critical( "Exception while downloading reply! {}".format(e) ) + logger.debug('Completed reply sync.') + if not loop: break else: From c952eed831a310f3c26c6b8d1a656449eaad3c31 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Wed, 9 Jan 2019 14:34:43 +0100 Subject: [PATCH 11/14] updated tests for signals/slots --- tests/gui/test_main.py | 6 +- tests/gui/test_widgets.py | 127 +++++++++++++++++++++++++------------ tests/test_app.py | 6 +- tests/test_logic.py | 56 +++++----------- tests/test_message_sync.py | 56 ++++++++-------- 5 files changed, 136 insertions(+), 115 deletions(-) diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 082190f38..1b62c096d 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -5,6 +5,7 @@ from securedrop_client.gui.main import Window from securedrop_client.resources import load_icon from securedrop_client.db import Submission +from uuid import uuid4 app = QApplication([]) @@ -233,7 +234,8 @@ def test_conversation_pending_message(mocker): mock_source = mocker.MagicMock() mock_source.journalistic_designation = 'Testy McTestface' - submission = Submission(source=mock_source, uuid="test", size=123, + msg_uuid = str(uuid4()) + submission = Submission(source=mock_source, uuid=msg_uuid, size=123, filename="test.msg.gpg", download_url='http://test/test') @@ -248,7 +250,7 @@ def test_conversation_pending_message(mocker): w.show_conversation_for(mock_source) assert mocked_add_message.call_count == 1 - assert mocked_add_message.call_args == mocker.call("") + assert mocked_add_message.call_args == mocker.call(msg_uuid, "") def test_set_status(mocker): diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 6a39549dd..4e7fe7eb9 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -542,9 +542,33 @@ def test_SpeechBubble_init(mocker): mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel') mocker.patch('securedrop_client.gui.widgets.QVBoxLayout') mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout') + mock_signal = mocker.Mock() + mock_connect = mocker.Mock() + mock_signal.connect = mock_connect - SpeechBubble('hello') + SpeechBubble('mock id', 'hello', mock_signal) mock_label.assert_called_once_with('hello') + assert mock_connect.called + + +def test_SpeechBubble_update_text(mocker): + """ + Check that the calling the slot updates the text. + """ + mocker.patch('securedrop_client.gui.widgets.QVBoxLayout') + mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout') + mock_signal = mocker.MagicMock() + + msg_id = 'abc123' + sb = SpeechBubble(msg_id, 'hello', mock_signal) + + new_msg = 'new message' + sb._update_text(msg_id, new_msg) + assert sb.message.text() == new_msg + + newer_msg = 'an even newer message' + sb._update_text(msg_id + 'xxxxx', newer_msg) + assert sb.message.text() == new_msg def test_SpeechBubble_html_init(mocker): @@ -555,8 +579,9 @@ def test_SpeechBubble_html_init(mocker): mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel') mocker.patch('securedrop_client.gui.widgets.QVBoxLayout') mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout') + mock_signal = mocker.MagicMock() - SpeechBubble('hello') + SpeechBubble('mock id', 'hello', mock_signal) mock_label.assert_called_once_with('<b>hello</b>') @@ -565,48 +590,73 @@ def test_SpeechBubble_with_apostrophe_in_text(mocker): mock_label = mocker.patch('securedrop_client.gui.widgets.QLabel') mocker.patch('securedrop_client.gui.widgets.QVBoxLayout') mocker.patch('securedrop_client.gui.widgets.SpeechBubble.setLayout') + mock_signal = mocker.MagicMock() message = "I'm sure, you are reading my message." - SpeechBubble(message) + SpeechBubble('mock id', message, mock_signal) mock_label.assert_called_once_with(message) -def test_ConversationWidget_init_left(): +def test_ConversationWidget_init_left(mocker): """ Check the ConversationWidget is configured correctly for align-left. """ - cw = ConversationWidget('hello', align='left') + mock_signal = mocker.Mock() + mock_connect = mocker.Mock() + mock_signal.connect = mock_connect + + cw = ConversationWidget('mock id', 'hello', mock_signal, align='left') layout = cw.layout() + assert isinstance(layout.takeAt(0), QWidgetItem) assert isinstance(layout.takeAt(0), QSpacerItem) + assert mock_connect.called -def test_ConversationWidget_init_right(): +def test_ConversationWidget_init_right(mocker): """ Check the ConversationWidget is configured correctly for align-left. """ - cw = ConversationWidget('hello', align='right') + mock_signal = mocker.Mock() + mock_connect = mocker.Mock() + mock_signal.connect = mock_connect + + cw = ConversationWidget('mock id', 'hello', mock_signal, align='right') layout = cw.layout() + assert isinstance(layout.takeAt(0), QSpacerItem) assert isinstance(layout.takeAt(0), QWidgetItem) + assert mock_connect.called -def test_MessageWidget_init(): +def test_MessageWidget_init(mocker): """ Check the CSS is set as expected. """ - mw = MessageWidget('hello') + mock_signal = mocker.Mock() + mock_connected = mocker.Mock() + mock_signal.connect = mock_connected + + mw = MessageWidget('mock id', 'hello', mock_signal) ss = mw.styleSheet() + assert 'background-color' in ss + assert mock_connected.called -def test_ReplyWidget_init(): +def test_ReplyWidget_init(mocker): """ Check the CSS is set as expected. """ - rw = ReplyWidget('hello') + mock_signal = mocker.Mock() + mock_connected = mocker.Mock() + mock_signal.connect = mock_connected + + rw = ReplyWidget('mock id', 'hello', mock_signal) ss = rw.styleSheet() + assert 'background-color' in ss + assert mock_connected.called def test_FileWidget_init_left(mocker): @@ -680,78 +730,74 @@ def test_FileWidget_mousePressEvent_open(mocker): fw.controller.on_file_open.assert_called_once_with(submission) -def test_ConversationView_init(mocker): +def test_ConversationView_init(mocker, homedir): """ Ensure the conversation view has a layout to add widgets to. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) + mocked_controller = mocker.MagicMock() + cv = ConversationView(mocked_source, homedir, mocked_controller) assert isinstance(cv.conversation_layout, QVBoxLayout) -def test_ConversationView_setup(mocker): - """ - Ensure the controller is set - """ - mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) - mock_controller = mocker.MagicMock() - cv.setup(mock_controller) - assert cv.controller == mock_controller - - -def test_ConversationView_move_to_bottom(mocker): +def test_ConversationView_move_to_bottom(mocker, homedir): """ Check the signal handler sets the correct value for the scrollbar to be the maximum possible value. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) + mocked_controller = mocker.MagicMock() + + cv = ConversationView(mocked_source, homedir, mocked_controller) + cv.scroll = mocker.MagicMock() cv.move_to_bottom(0, 6789) cv.scroll.verticalScrollBar().setValue.assert_called_once_with(6789) -def test_ConversationView_add_message(mocker): +def test_ConversationView_add_message(mocker, homedir): """ Adding a message results in a new MessageWidget added to the layout. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) - cv.controller = mocker.MagicMock() + mocked_controller = mocker.MagicMock() + + cv = ConversationView(mocked_source, homedir, mocked_controller) cv.conversation_layout = mocker.MagicMock() - cv.add_message('hello') + cv.add_message('mock id', 'hello') assert cv.conversation_layout.addWidget.call_count == 1 cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], MessageWidget) -def test_ConversationView_add_reply(mocker): +def test_ConversationView_add_reply(mocker, homedir): """ Adding a reply results in a new ReplyWidget added to the layout. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) - cv.controller = mocker.MagicMock() + mocked_controller = mocker.MagicMock() + + cv = ConversationView(mocked_source, homedir, mocked_controller) cv.conversation_layout = mocker.MagicMock() - cv.add_reply('hello') + cv.add_reply('mock id', 'hello') assert cv.conversation_layout.addWidget.call_count == 1 cal = cv.conversation_layout.addWidget.call_args_list assert isinstance(cal[0][0][0], ReplyWidget) -def test_ConversationView_add_downloaded_file(mocker): +def test_ConversationView_add_downloaded_file(mocker, homedir): """ Adding a file results in a new FileWidget added to the layout with the proper QLabel. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) - cv.controller = mocker.MagicMock() + mocked_controller = mocker.MagicMock() + + cv = ConversationView(mocked_source, homedir, mocked_controller) cv.conversation_layout = mocker.MagicMock() mock_source = mocker.MagicMock() @@ -769,14 +815,15 @@ def test_ConversationView_add_downloaded_file(mocker): assert isinstance(cal[0][0][0], FileWidget) -def test_ConversationView_add_not_downloaded_file(mocker): +def test_ConversationView_add_not_downloaded_file(mocker, homedir): """ Adding a file results in a new FileWidget added to the layout with the proper QLabel. """ mocked_source = mocker.MagicMock() - cv = ConversationView(mocked_source) - cv.controller = mocker.MagicMock() + mocked_controller = mocker.MagicMock() + + cv = ConversationView(mocked_source, homedir, mocked_controller) cv.conversation_layout = mocker.MagicMock() mock_source = mocker.MagicMock() diff --git a/tests/test_app.py b/tests/test_app.py index ca8d5e6b3..593363503 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -234,7 +234,7 @@ def fake_known_args(): mock_start_app.assert_called_once_with(mock_args, mock_qt_args) -def test_signal_interception(mocker): +def test_signal_interception(mocker, homedir): # check that initializing an app calls configure_signal_handlers mocker.patch('securedrop_client.app.QApplication') mocker.patch('securedrop_client.app.prevent_second_instance') @@ -245,8 +245,10 @@ def test_signal_interception(mocker): mocker.patch('securedrop_client.logic.GpgHelper') mocker.patch('securedrop_client.app.configure_logging') mock_signal_handlers = mocker.patch('securedrop_client.app.configure_signal_handlers') + mock_args = mocker.Mock() + mock_args.sdc_home = homedir - start_app(mocker.MagicMock(), []) + start_app(mock_args, []) assert mock_signal_handlers.called # check that a signal interception calls quit on the app diff --git a/tests/test_logic.py b/tests/test_logic.py index bd8cd23f3..6310a8baa 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -680,59 +680,29 @@ def test_Client_update_sources(homedir, config, mocker): mock_gui.show_sources.assert_called_once_with(source_list) -def test_Client_update_conversation_view_current_source(homedir, config, mocker): +def test_Client_update_conversation_views(homedir, config, mocker): """ Ensure the UI displays the latest version of the messages/replies that have been downloaded/decrypted in the current conversation view. Using the `config` fixture to ensure the config is written to disk. """ - mock_gui = mocker.MagicMock() - mock_gui.current_source = 'teehee' - mock_gui.show_conversation_for = mocker.MagicMock() + mock_gui = mocker.Mock() + mock_conversation = mocker.MagicMock() + mock_update_conversation = mocker.MagicMock() + mock_conversation.update_conversation = mock_update_conversation + mock_gui.conversations = {'foo': mock_conversation} mock_session = mocker.MagicMock() # Since we use the set-like behavior of self.session # to check if the source is still persistent, let's mock that here - mock_session.__contains__ = mocker.MagicMock() - mock_session.__contains__.return_value = [mock_gui.current_source] + mock_session.__contains__ = mocker.Mock() + mock_session.__contains__.return_value = True mock_session.refresh = mocker.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, homedir) - cl.update_conversation_view() - mock_session.refresh.assert_called_with(mock_gui.current_source) - mock_gui.show_conversation_for.assert_called_once_with( - mock_gui.current_source) - - -def test_Client_update_conversation_deleted_source(homedir, config, mocker): - """ - Ensure the UI does not attempt to refresh and display a deleted - source. - Using the `config` fixture to ensure the config is written to disk. - """ - mock_gui = mocker.MagicMock() - mock_gui.current_source = 'teehee' - mock_gui.show_conversation_for = mocker.MagicMock() - mock_session = mocker.MagicMock() - mock_session.refresh = mocker.MagicMock() - cl = Client('http://localhost', mock_gui, mock_session, homedir) - cl.update_conversation_view() - mock_session.refresh.assert_not_called() - mock_gui.show_conversation_for.assert_not_called() - - -def test_Client_update_conversation_view_no_current_source(homedir, config, mocker): - """ - Ensure that if there is no current source (i.e. the user has not clicked - a source in the sidebar), the UI will not redraw the conversation view. - Using the `config` fixture to ensure the config is written to disk. - """ - mock_gui = mocker.MagicMock() - mock_gui.current_source = None - mock_session = mocker.MagicMock() - cl = Client('http://localhost', mock_gui, mock_session, homedir) - cl.update_conversation_view() - mock_gui.show_conversation_for.assert_not_called() + cl.update_conversation_views() + assert mock_session.refresh.called + assert mock_update_conversation.called def test_Client_unstars_a_source_if_starred(homedir, config, mocker): @@ -743,18 +713,22 @@ def test_Client_unstars_a_source_if_starred(homedir, config, mocker): mock_gui = mocker.MagicMock() mock_session = mocker.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, homedir) + source_db_object = mocker.MagicMock() source_db_object.uuid = mocker.MagicMock() source_db_object.is_starred = True + cl.call_api = mocker.MagicMock() cl.api = mocker.MagicMock() cl.api.remove_star = mocker.MagicMock() cl.on_update_star_complete = mocker.MagicMock() cl.on_sidebar_action_timeout = mocker.MagicMock() + source_sdk_object = mocker.MagicMock() mock_source = mocker.patch('sdclientapi.Source') mock_source.return_value = source_sdk_object cl.update_star(source_db_object) + cl.call_api.assert_called_once_with( cl.api.remove_star, cl.on_update_star_complete, cl.on_sidebar_action_timeout, source_sdk_object) diff --git a/tests/test_message_sync.py b/tests/test_message_sync.py index 467e19b15..b7159add6 100644 --- a/tests/test_message_sync.py +++ b/tests/test_message_sync.py @@ -30,20 +30,15 @@ def test_MessageSync_init(mocker): def test_MessageSync_run_success(mocker): submission = mocker.MagicMock() + submission.uuid = 'mock id' submission.download_url = "http://foo" submission.filename = "foo.gpg" - fh = mocker.MagicMock() - fh.name = "foo" - # mock the fetching of submissions mocker.patch('securedrop_client.storage.find_new_submissions', return_value=[submission]) - # mock the handling of the downloaded files - mocker.patch('shutil.move') - mocker.patch('os.unlink') - mocker.patch('tempfile.NamedTemporaryFile', return_value=fh) mocker.patch('securedrop_client.message_sync.storage.mark_file_as_downloaded') - mocker.patch('builtins.open', mocker.mock_open(read_data="blah")) + # don't create the signal + mocker.patch('securedrop_client.message_sync.pyqtSignal') # mock the GpgHelper creation since we don't have directories/keys setup mocker.patch('securedrop_client.message_sync.GpgHelper') @@ -54,9 +49,16 @@ def test_MessageSync_run_success(mocker): ms = MessageSync(api, home, is_qubes) ms.api.download_submission = mocker.MagicMock(return_value=(1234, "/home/user/downloads/foo")) + mock_message_downloaded = mocker.Mock() + mock_emit = mocker.Mock() + mock_message_downloaded.emit = mock_emit + mocker.patch.object(ms, 'message_downloaded', new=mock_message_downloaded) + # check that it runs without raising exceptions ms.run(False) + assert mock_emit.called + def test_MessageSync_exception(homedir, config, mocker): """ @@ -76,6 +78,8 @@ def test_MessageSync_exception(homedir, config, mocker): ms = MessageSync(api, str(homedir), is_qubes) mocker.patch.object(ms.gpg, 'decrypt_submission_or_reply', side_effect=CryptoError) + + # check that it runs without raising exceptions ms.run(False) @@ -84,17 +88,10 @@ def test_MessageSync_run_failure(mocker): submission.download_url = "http://foo" submission.filename = "foo.gpg" - fh = mocker.MagicMock() - fh.name = "foo" - # mock the fetching of submissions mocker.patch('securedrop_client.storage.find_new_submissions', return_value=[submission]) # mock the handling of the downloaded files - mocker.patch('shutil.move') - mocker.patch('os.unlink') mocker.patch('securedrop_client.message_sync.storage.mark_file_as_downloaded') - mocker.patch('tempfile.NamedTemporaryFile', return_value=fh) - mocker.patch('builtins.open', mocker.mock_open(read_data="blah")) # mock the GpgHelper creation since we don't have directories/keys setup mocker.patch('securedrop_client.message_sync.GpgHelper') @@ -105,30 +102,27 @@ def test_MessageSync_run_failure(mocker): ms = MessageSync(api, home, is_qubes) ms.api.download_submission = mocker.MagicMock(return_value=(1234, "/home/user/downloads/foo")) + # check that it runs without raising exceptions ms.run(False) def test_ReplySync_run_success(mocker): reply = mocker.MagicMock() + reply.uuid = 'mock id' reply.download_url = "http://foo" reply.filename = "foo.gpg" - fh = mocker.MagicMock() - fh.name = "foo" - api = mocker.MagicMock() home = "/home/user/.sd" is_qubes = True # mock the fetching of replies mocker.patch('securedrop_client.storage.find_new_replies', return_value=[reply]) + # don't create the signal + mocker.patch('securedrop_client.message_sync.pyqtSignal') # mock the handling of the replies - mocker.patch('shutil.move') - mocker.patch('os.unlink') - mocker.patch('securedrop_client.message_sync.storage.mark_file_as_downloaded') - mocker.patch('tempfile.NamedTemporaryFile', return_value=fh) + mocker.patch('securedrop_client.message_sync.storage.mark_reply_as_downloaded') mocker.patch('securedrop_client.message_sync.GpgHelper') - mocker.patch('builtins.open', mocker.mock_open(read_data="blah")) api = mocker.MagicMock() home = "/home/user/.sd" @@ -137,9 +131,16 @@ def test_ReplySync_run_success(mocker): ms = ReplySync(api, home, is_qubes) ms.api.download_reply = mocker.MagicMock(return_value=(1234, "/home/user/downloads/foo")) + mock_reply_downloaded = mocker.Mock() + mock_emit = mocker.Mock() + mock_reply_downloaded.emit = mock_emit + mocker.patch.object(ms, 'reply_downloaded', new=mock_reply_downloaded) + # check that it runs without raising exceptions ms.run(False) + assert mock_emit.called + def test_ReplySync_exception(mocker): """ @@ -157,6 +158,8 @@ def test_ReplySync_exception(mocker): mocker.patch("sdclientapi.sdlocalobjects.Reply", mocker.MagicMock(side_effect=Exception())) rs = ReplySync(api, home, is_qubes) + + # check that it runs without raise exceptions rs.run(False) @@ -165,18 +168,11 @@ def test_ReplySync_run_failure(mocker): reply.download_url = "http://foo" reply.filename = "foo.gpg" - fh = mocker.MagicMock() - fh.name = "foo" - # mock finding new replies mocker.patch('securedrop_client.storage.find_new_replies', return_value=[reply]) # mock handling the new reply - mocker.patch('shutil.move') - mocker.patch('os.unlink') mocker.patch('securedrop_client.message_sync.storage.mark_file_as_downloaded') - mocker.patch('tempfile.NamedTemporaryFile', return_value=fh) mocker.patch('securedrop_client.message_sync.GpgHelper') - mocker.patch('builtins.open', mocker.mock_open(read_data="blah")) api = mocker.MagicMock() home = "/home/user/.sd" From ee622bb05ac58368f6819d312b14e9e240b0dcf4 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 10 Jan 2019 12:31:32 +0100 Subject: [PATCH 12/14] create wrapper widget for all elements of a source's 'view' --- securedrop_client/gui/main.py | 19 ++++++------------- securedrop_client/gui/widgets.py | 21 ++++++++++++++++++++- securedrop_client/logic.py | 14 +++++++++++--- 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/securedrop_client/gui/main.py b/securedrop_client/gui/main.py index 2016ed08c..d24ee09a6 100644 --- a/securedrop_client/gui/main.py +++ b/securedrop_client/gui/main.py @@ -23,8 +23,7 @@ from PyQt5.QtWidgets import QMainWindow, QWidget, QVBoxLayout, QDesktopWidget, QStatusBar from securedrop_client import __version__ from securedrop_client.gui.widgets import (ToolBar, MainView, LoginDialog, - ConversationView, - SourceProfileShortWidget) + SourceConversationWrapper) from securedrop_client.resources import load_icon logger = logging.getLogger(__name__) @@ -66,7 +65,7 @@ def __init__(self, sdc_home: str): widget_layout.addWidget(self.tool_bar, 1) widget_layout.addWidget(self.main_view, 6) - # Cache a dict of source.uuid -> ConversationView + # Cache a dict of source.uuid -> SourceConversationWrapper # We do this to not create/destroy widgets constantly (because it causes UI "flicker") self.conversations = {} @@ -176,16 +175,10 @@ def show_conversation_for(self, source): conversation_container = self.conversations.get(source.uuid, None) if conversation_container is None: - conversation = ConversationView(source, self.sdc_home, self.controller, parent=self) - - conversation_container = QWidget() - layout = QVBoxLayout() - conversation_container.setLayout(layout) - - source_profile = SourceProfileShortWidget(source, self.controller) - - layout.addWidget(source_profile) - layout.addWidget(conversation) + conversation_container = SourceConversationWrapper(source, + self.sdc_home, + self.controller) + self.conversations[source.uuid] = conversation_container self.main_view.set_conversation(conversation_container) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 42af8bdc0..bdcbdf53b 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -25,6 +25,7 @@ QPushButton, QVBoxLayout, QLineEdit, QScrollArea, QDialog, QAction, QMenu, \ QMessageBox, QToolButton +from securedrop_client.db import Source from securedrop_client.logic import Client from securedrop_client.resources import load_svg, load_image from securedrop_client.storage import get_data @@ -637,7 +638,7 @@ class ConversationView(QWidget): Renders a conversation. """ - def __init__(self, source_db_object, sdc_home: str, controller: Client, parent=None): + def __init__(self, source_db_object: Source, sdc_home: str, controller: Client, parent=None): super().__init__(parent) self.source = source_db_object self.sdc_home = sdc_home @@ -726,6 +727,24 @@ def add_reply(self, message_id: str, reply: str, files=None) -> None: ReplyWidget(message_id, reply, self.controller.reply_sync.reply_downloaded)) +class SourceConversationWrapper(QWidget): + """ + Wrapper for a source's conversation including the chat window, profile tab, and other + per-soruce resources. + """ + + def __init__(self, source: Source, sdc_home: str, controller: Client, parent=None) -> None: + super().__init__(parent) + self.layout = QVBoxLayout() + self.setLayout(self.layout) + + self.conversation = ConversationView(source, sdc_home, controller, parent=self) + self.source_profile = SourceProfileShortWidget(source, controller) + + self.layout.addWidget(self.source_profile) + self.layout.addWidget(self.conversation) + + class DeleteSourceAction(QAction): """Use this action to delete the source record.""" diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index f19eb3961..25dee0dbc 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -404,6 +404,13 @@ def on_synced(self, result): remote_submissions, remote_replies, self.data_dir) + # clean up locally cached conversation views + remote_source_uuids = [s.uuid for s in remote_sources] + cached_sources = list(self.gui.conversations.keys()) + for cached_source in cached_sources: + if cached_source not in remote_source_uuids: + self.gui.conversations.pop(cached_source, None) + # Set last sync flag. with open(self.sync_flag, 'w') as f: f.write(arrow.now().format()) @@ -448,9 +455,10 @@ def update_conversation_views(self): Updates the conversation view to reflect progress of the download and decryption of messages and replies. """ - for conversation in self.gui.conversations.values(): - self.session.refresh(conversation.source) - conversation.update_conversation(conversation.source.collection) + for conversation_wrapper in self.gui.conversations.values(): + conv = conversation_wrapper.conversation + self.session.refresh(conv.source) + conv.update_conversation(conv.source.collection) def on_update_star_complete(self, result): """ From c11e751b239a9c632d3e3846cff45d9dd4f3f80a Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 10 Jan 2019 12:43:39 +0100 Subject: [PATCH 13/14] update tests for new conversation object structure --- tests/gui/test_main.py | 34 ++++++++++++++++++++++------------ tests/test_logic.py | 4 +++- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 1b62c096d..b0804d62b 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -196,11 +196,12 @@ def test_conversation_for(mocker): mock_reply.filename = '3-my-source-reply.gpg' mock_source.collection = [mock_file, mock_message, mock_reply] - mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') - mocked_add_reply = mocker.patch('securedrop_client.gui.main.ConversationView.add_reply') - mocked_add_file = mocker.patch('securedrop_client.gui.main.ConversationView.add_file') - mocker.patch('securedrop_client.gui.main.QVBoxLayout') - mocker.patch('securedrop_client.gui.main.QWidget') + mocked_add_message = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_message', + new=mocker.Mock()) + mocked_add_reply = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_reply', + new=mocker.Mock()) + mocked_add_file = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_file', + new=mocker.Mock()) w.show_conversation_for(mock_source) @@ -210,16 +211,25 @@ def test_conversation_for(mocker): # check that showing the conversation a second time doesn't break anything + # stop the old mockers + mocked_add_message.stop() + mocked_add_reply.stop() + mocked_add_file.stop() + # use new mocks to check the count again - mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') - mocked_add_reply = mocker.patch('securedrop_client.gui.main.ConversationView.add_reply') - mocked_add_file = mocker.patch('securedrop_client.gui.main.ConversationView.add_file') + mocked_add_message = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_message', + new=mocker.Mock()) + mocked_add_reply = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_reply', + new=mocker.Mock()) + mocked_add_file = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_file', + new=mocker.Mock()) w.show_conversation_for(mock_source) - assert mocked_add_message.call_count > 0 - assert mocked_add_reply.call_count > 0 - assert mocked_add_file.call_count > 0 + # because the conversation was cached, we don't call these functions again + assert mocked_add_message.call_count == 0 + assert mocked_add_reply.call_count == 0 + assert mocked_add_file.call_count == 0 def test_conversation_pending_message(mocker): @@ -243,7 +253,7 @@ def test_conversation_pending_message(mocker): mock_source.collection = [submission] - mocked_add_message = mocker.patch('securedrop_client.gui.main.ConversationView.add_message') + mocked_add_message = mocker.patch('securedrop_client.gui.widgets.ConversationView.add_message') mocker.patch('securedrop_client.gui.main.QVBoxLayout') mocker.patch('securedrop_client.gui.main.QWidget') diff --git a/tests/test_logic.py b/tests/test_logic.py index 6310a8baa..042421979 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -687,10 +687,12 @@ def test_Client_update_conversation_views(homedir, config, mocker): Using the `config` fixture to ensure the config is written to disk. """ mock_gui = mocker.Mock() + mock_conversation_wrapper = mocker.Mock() mock_conversation = mocker.MagicMock() + mock_conversation_wrapper.conversation = mock_conversation mock_update_conversation = mocker.MagicMock() mock_conversation.update_conversation = mock_update_conversation - mock_gui.conversations = {'foo': mock_conversation} + mock_gui.conversations = {'foo': mock_conversation_wrapper} mock_session = mocker.MagicMock() # Since we use the set-like behavior of self.session From a1ad28b3eacde2500ebc5e1047dd519a872c8d79 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Thu, 10 Jan 2019 13:04:24 +0100 Subject: [PATCH 14/14] ensure conversation is removed on source deletion --- tests/test_logic.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_logic.py b/tests/test_logic.py index 042421979..2dfd8c9c1 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -503,6 +503,31 @@ def test_Client_sync_api(homedir, config, mocker): cl.on_sync_timeout, cl.api) +def test_Client_on_synced_remove_stale_sources(homedir, config, mocker): + """ + On an API sync, if a source no longer exists, remove it from the GUI. + Using the `config` fixture to ensure the config is written to disk. + """ + mock_source_id = 'abc123' + mock_conv_wrapper = 'mock' + + gui = mocker.Mock() + gui.conversations = {mock_source_id: mock_conv_wrapper} + + mock_session = mocker.MagicMock() + cl = Client('http://localhost', gui, mock_session, homedir) + + mock_source = mocker.Mock() + mock_source.uuid = mock_source_id + + # not that the first item does *not* have the mock_source + api_res = ([], mocker.MagicMock(), mocker.MagicMock()) + cl.on_synced(api_res) + + # check that the uuid is not longer in the dict + assert mock_source_id not in gui.conversations + + def test_Client_last_sync_with_file(homedir, config, mocker): """ The flag indicating the time of the last sync with the API is stored in a