From f57c191f725be5b07d84c21c357a3867c4c65667 Mon Sep 17 00:00:00 2001 From: "Nicholas H.Tollervey" Date: Tue, 17 Mar 2020 14:10:55 +0000 Subject: [PATCH 1/6] Replaces branch fix-blocked-ui-on-start. --- securedrop_client/gui/widgets.py | 115 ++++++++----- tests/gui/test_widgets.py | 283 +++++++++++++++---------------- 2 files changed, 203 insertions(+), 195 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index b0111f16e..776183cf3 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -679,6 +679,7 @@ def setup(self, controller): """ self.controller = controller self.source_list.setup(controller) + self.source_list.delete_source_by_uuid.connect(self.delete_conversation) def show_sources(self, sources: List[Source]): """ @@ -691,11 +692,7 @@ def show_sources(self, sources: List[Source]): else: self.empty_conversation_view.show_no_sources_message() self.empty_conversation_view.show() - - deleted_sources = self.source_list.update(sources) - for source_uuid in deleted_sources: - # Then call the function to remove the wrapper and its children. - self.delete_conversation(source_uuid) + self.source_list.update(sources) def on_source_changed(self): """ @@ -729,8 +726,6 @@ def delete_conversation(self, source_uuid: str) -> None: """ try: logger.debug('Deleting SourceConversationWrapper for {}'.format(source_uuid)) - conversation_wrapper = self.source_conversations[source_uuid] - conversation_wrapper.deleteLater() del self.source_conversations[source_uuid] except KeyError: logger.debug('No SourceConversationWrapper for {} to delete'.format(source_uuid)) @@ -886,6 +881,8 @@ class SourceList(QListWidget): } ''' + delete_source_by_uuid = pyqtSignal(str) + def __init__(self): super().__init__() @@ -910,50 +907,76 @@ def setup(self, controller): self.controller.file_ready.connect(self.set_snippet) self.controller.file_missing.connect(self.set_snippet) - def update(self, sources: List[Source]) -> List[str]: - """ - Update the list with the passed in list of sources. - """ - # Delete widgets that no longer exist in source list - source_uuids = [source.uuid for source in sources] - deleted_uuids = [] - for i in range(self.count()): - list_item = self.item(i) - list_widget = self.itemWidget(list_item) - - if list_widget and list_widget.source_uuid not in source_uuids: - if list_item.isSelected(): - self.setCurrentItem(None) - del self.source_widgets[list_widget.source_uuid] - deleted_uuids.append(list_widget.source_uuid) - self.takeItem(i) - list_widget.deleteLater() - - # Create new widgets for new sources - widget_uuids = [self.itemWidget(self.item(i)).source_uuid for i in range(self.count())] - for source in sources: - if source.uuid in widget_uuids: - try: - self.source_widgets[source.uuid].update() - except sqlalchemy.exc.InvalidRequestError as e: - logger.error( - "Could not update SourceWidget for source %s; deleting it. Error was: %s", - source.uuid, - e - ) - deleted_uuids.append(source.uuid) - self.source_widgets[source.uuid].deleteLater() - del self.source_widgets[list_widget.source_uuid] - else: + def update(self, sources: List[Source]): + """ + Reset and update the list with the passed in list of sources. + """ + # A flag to show if a source is currently selected (the current source + # doesn't have to be selected in a QListWidget -- it appears to default + # to whatever is in row 0, whether it's selected or not). + has_source_selected = False + if self.currentRow() > -1: + has_source_selected = self.item(self.currentRow()).isSelected() + current_source = self.get_current_source() + current_source_id = current_source and current_source.id + # Make a copy of the source_widgets that are currently displayed + # so that we can compare which widgets were removed. This means that + # the source was deleted, and we'll return this so we can also + # delete the corresponding SourceConversationWrapper. + existing_source_widgets = self.source_widgets.copy() + + # When we call clear() to delete all SourceWidgets, we should + # also clear the source_widgets dict. + self.clear() + self.source_widgets = {} + sources.reverse() + self.add_source(existing_source_widgets, sources, current_source_id, + has_source_selected) + + def add_source(self, existing_source_widgets, sources, current_source_id, + has_source_selected, slice_size=1): + """ + Add a slice of sources, ensure the currently selected source is + retained and, if necessary, reschedule the addition of more sources. + If no more sources are left, work out the deleted sources and emit + deletion signal for each source to delete. + """ + + def schedule_source_management(slice_size=slice_size): + if not sources: + # No more sources to add, discover the deleted sources and + # delete them. + deleted_uuids = list(set(existing_source_widgets.keys()) - + set(self.source_widgets.keys())) + for source_uuid in deleted_uuids: + self.delete_source_by_uuid.emit(source_uuid) + # Nothing more to do. + return + # Process the remaining "slice_size" number of sources. + sources_slice = sources[:slice_size] + for source in sources_slice: new_source = SourceWidget(self.controller, source) self.source_widgets[source.uuid] = new_source - - list_item = QListWidgetItem() - self.insertItem(0, list_item) + list_item = QListWidgetItem(self) list_item.setSizeHint(new_source.sizeHint()) + + self.addItem(list_item) self.setItemWidget(list_item, new_source) - return deleted_uuids + if source.id == current_source_id and has_source_selected: + self.setCurrentItem(list_item) + # ATTENTION! 32 is an arbitrary number arrived at via + # experimentation. It adds plenty of sources, but doesn't block + # for a noticable amount of time. + new_slice_size = min(32, slice_size * 2) + # Call add_source again for the remaining sources. + self.add_source(existing_source_widgets, sources[slice_size:], + current_source_id, has_source_selected, + new_slice_size) + + # Schedule the closure defined above in the next iteration of the + # Qt event loop (thus unblocking the UI). + QTimer.singleShot(1, schedule_source_management) def get_current_source(self): source_item = self.currentItem() diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 745c58326..99c76f255 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -10,7 +10,8 @@ from PyQt5.QtGui import QFocusEvent, QKeyEvent, QMovie from PyQt5.QtTest import QTest from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \ - QLineEdit + QLineEdit, QListWidgetItem +import sqlalchemy import sqlalchemy.orm.exc from sqlalchemy.orm import attributes, scoped_session, sessionmaker @@ -472,6 +473,7 @@ def test_MainView_setup(mocker): assert mv.controller == controller mv.source_list.setup.assert_called_once_with(controller) + mv.source_list.delete_source_by_uuid.connect.assert_called_once_with(mv.delete_conversation) def test_MainView_show_sources_with_none_selected(mocker): @@ -504,25 +506,6 @@ def test_MainView_show_sources_with_no_sources_at_all(mocker): mv.empty_conversation_view.show.assert_called_once_with() -def test_MainView_show_sources_when_sources_are_deleted(mocker): - """ - Ensure that show_sources also deletes the SourceConversationWrapper for a deleted source. - """ - mv = MainView(None) - mv.source_list = mocker.MagicMock() - mv.empty_conversation_view = mocker.MagicMock() - mv.source_list.update = mocker.MagicMock(return_value=[]) - mv.delete_conversation = mocker.MagicMock() - - mv.show_sources([1, 2, 3, 4]) - - mv.source_list.update = mocker.MagicMock(return_value=[4]) - - mv.show_sources([1, 2, 3]) - - mv.delete_conversation.assert_called_once_with(4) - - def test_MainView_delete_conversation_when_conv_wrapper_exists(mocker): """ Ensure SourceConversationWrapper is deleted if it exists. @@ -538,7 +521,6 @@ def test_MainView_delete_conversation_when_conv_wrapper_exists(mocker): mv.delete_conversation('123') - conversation_wrapper.deleteLater.assert_called_once_with() assert mv.source_conversations == {} @@ -719,7 +701,13 @@ def test_SourceList_get_current_source(mocker): sl = SourceList() sl.controller = mocker.MagicMock() sources = [factory.Source(), factory.Source()] - sl.update(sources) + for s in sources: + new_source = SourceWidget(sl.controller, s) + sl.source_widgets[s.uuid] = new_source + list_item = QListWidgetItem(sl) + list_item.setSizeHint(new_source.sizeHint()) + sl.addItem(list_item) + sl.setItemWidget(list_item, new_source) sl.setCurrentItem(sl.itemAt(1, 0)) # select source2 current_source = sl.get_current_source() @@ -735,169 +723,146 @@ def test_SourceList_update_adds_new_sources(mocker): sl = SourceList() sl.clear = mocker.MagicMock() - sl.insertItem = mocker.MagicMock() - sl.takeItem = mocker.MagicMock() + sl.add_source = mocker.MagicMock() sl.setItemWidget = mocker.MagicMock() - sl.controller = mocker.MagicMock() - sl.setCurrentItem = mocker.MagicMock() - - mock_sw = mocker.MagicMock() - mock_lwi = mocker.MagicMock() - mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) - mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) - + sl.currentRow = mocker.MagicMock(return_value=0) + sl.item = mocker.MagicMock() + sl.item().isSelected.return_value = True sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] sl.update(sources) - - assert mock_sw.call_count == len(sources) - assert mock_lwi.call_count == len(sources) - assert sl.insertItem.call_count == len(sources) - assert sl.setItemWidget.call_count == len(sources) - assert len(sl.source_widgets) == len(sources) - assert sl.setCurrentItem.call_count == 0 - sl.clear.assert_not_called() - sl.takeItem.assert_not_called() - mock_sw.deleteLater.assert_not_called() + sl.clear.assert_called_once_with() + sl.add_source.assert_called_once_with({}, sources, None, True) -def test_SourceList_update_when_source_deleted(mocker, session, session_maker, homedir): +def test_SourceList_add_source_starts_timer(mocker, session_maker, homedir): """ - Test that SourceWidget.update raises an exception when its source has been deleted. - - When SourceList.update calls SourceWidget.update and that - SourceWidget's source has been deleted, SourceList.update should - catch the resulting excpetion, delete the SourceWidget and add its - source UUID to the list of deleted source UUIDs. + When the add_source method is called it schedules the addition of a source + to the source list via a single-shot QTimer. """ - mock_gui = mocker.MagicMock() - controller = logic.Controller('http://localhost', mock_gui, session_maker, homedir) - - # create the source in another session - source = factory.Source() - session.add(source) - session.commit() - - # construct the SourceWidget with the source fetched in its - # controller's session - oss = controller.session.query(db.Source).filter_by(id=source.id).one() - - # add it to the SourceList sl = SourceList() - sl.setup(controller) - deleted_uuids = sl.update([oss]) - assert not deleted_uuids - assert len(sl.source_widgets) == 1 - - # now delete it - session.delete(source) - session.commit() - - # and finally verify that updating raises an exception, causing - # the SourceWidget to be deleted - deleted_uuids = sl.update([source]) - assert len(deleted_uuids) == 1 - assert source.uuid in deleted_uuids - assert len(sl.source_widgets) == 0 + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, None, False) + assert mock_timer.singleShot.call_count == 1 -def test_SourceList_update_maintains_selection(mocker): +def test_SourceList_add_source_closure_adds_sources(mocker): """ - Maintains the selected item if present in new list + The closure (function created within the add_source function) behaves in + the expected way given the context of the call to add_source. """ sl = SourceList() sl.controller = mocker.MagicMock() - sources = [factory.Source(), factory.Source()] - sl.update(sources) - - sl.setCurrentItem(sl.itemAt(0, 0)) # select the first source - sl.update(sources) - - assert sl.currentItem() - assert sl.itemWidget(sl.currentItem()).source.id == sources[0].id - - sl.setCurrentItem(sl.itemAt(1, 0)) # select the second source - sl.update(sources) - - assert sl.currentItem() - assert sl.itemWidget(sl.currentItem()).source.id == sources[1].id - + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() -def test_SourceList_update_with_pre_selected_source_maintains_selection(mocker): - """ - Check that an existing source widget that is selected remains selected. + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, 1, False) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + assert mock_sw.call_count == 1 + assert mock_lwi.call_count == 1 + assert sl.addItem.call_count == 1 + assert sl.setItemWidget.call_count == 1 + assert len(sl.source_widgets) == 1 + assert sl.setCurrentItem.call_count == 0 + sl.add_source.assert_called_once_with({}, sources[1:], 1, False, 2) + + +def test_SourceList_add_source_closure_sets_current_item(mocker): + """ + The closure (function created within the add_source function) ensures that + if an item being added to the UI is the currently selected item and is + already selected in the UI, it is marked as such. """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(), factory.Source()]) - second_item = sl.itemAt(1, 0) - sl.setCurrentItem(second_item) # select the second source - mocker.patch.object(second_item, 'isSelected', return_value=True) - - sl.update([factory.Source(), factory.Source()]) - - assert second_item.isSelected() is True - + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() -def test_SourceList_update_removes_selected_item_results_in_no_current_selection(mocker): - """ - Check that no items are currently selected if the currently selected item is deleted. + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) + mock_source = mocker.MagicMock() + mock_source.id = 1 + sources = [mock_source, mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, 1, True) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + assert sl.setCurrentItem.call_count == 1 + + +def test_SourceList_add_source_closure_deletes_sources_when_finished(mocker): + """ + The closure (function created within the add_source function) ensures that + if an item being added to the UI is the currently selected item and is + already selected in the UI, it is marked as such. """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' + existing_sources = { + "uuid1": mocker.MagicMock() + } - assert not sl.currentItem() + sl.source_widgets = { + "uuid2": mocker.MagicMock() + } + sl.delete_source_by_uuid = mocker.MagicMock() -def test_SourceList_update_removes_item_from_end_of_list(mocker): - """ - Check that the item is removed from the source list and dict if the source no longer exists. - """ - sl = SourceList() - sl.controller = mocker.MagicMock() - sl.update([ - factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) - assert sl.count() == 3 - sl.update([factory.Source(uuid='newer'), factory.Source(uuid='newest')]) - assert sl.count() == 2 - assert sl.itemWidget(sl.item(0)).source.uuid == 'newest' - assert sl.itemWidget(sl.item(1)).source.uuid == 'newer' - assert len(sl.source_widgets) == 2 + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source(existing_sources, [], 1, True) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + sl.delete_source_by_uuid.emit.assert_called_once_with("uuid1") -def test_SourceList_update_removes_item_from_middle_of_list(mocker): +def test_SourceList_update_removes_selected_item_results_in_no_current_selection(mocker): """ - Check that the item is removed from the source list and dict if the source no longer exists. + Check that no items are currently selected if the currently selected item is deleted. """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([ - factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) - assert sl.count() == 3 - sl.update([factory.Source(uuid='new'), factory.Source(uuid='newest')]) - assert sl.count() == 2 - assert sl.itemWidget(sl.item(0)).source.uuid == 'newest' - assert sl.itemWidget(sl.item(1)).source.uuid == 'new' - assert len(sl.source_widgets) == 2 + sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) + sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' + sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' -def test_SourceList_update_removes_item_from_beginning_of_list(mocker): - """ - Check that the item is removed from the source list and dict if the source no longer exists. - """ - sl = SourceList() - sl.controller = mocker.MagicMock() - sl.update([ - factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) - assert sl.count() == 3 - sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) - assert sl.count() == 2 - assert sl.itemWidget(sl.item(0)).source.uuid == 'newer' - assert sl.itemWidget(sl.item(1)).source.uuid == 'new' - assert len(sl.source_widgets) == 2 + assert not sl.currentItem() def test_SourceList_set_snippet(mocker): @@ -960,6 +925,26 @@ def test_SourceWidget_update_attachment_icon(mocker): assert sw.paperclip.isHidden() +def test_SourceWidget_update_raises_InvalidRequestError(mocker): + """ + If the source no longer exists in the local data store, ensure the expected + exception is logged and re-raised. + """ + controller = mocker.MagicMock() + source = factory.Source(document_count=1) + sw = SourceWidget(controller, source) + ex = sqlalchemy.exc.InvalidRequestError() + controller.session.refresh.side_effect = ex + mock_logger = mocker.MagicMock() + mocker.patch( + "securedrop_client.gui.widgets.logger", + mock_logger, + ) + with pytest.raises(sqlalchemy.exc.InvalidRequestError): + sw.update() + assert mock_logger.error.call_count == 1 + + def test_SourceWidget_set_snippet(mocker): """ Snippets are set as expected. From ccc62bcecfabfd635dcaacfe6f658e05e115d3f7 Mon Sep 17 00:00:00 2001 From: "Nicholas H.Tollervey" Date: Wed, 25 Mar 2020 09:59:45 +0000 Subject: [PATCH 2/6] Rebase conflicts --- securedrop_client/gui/widgets.py | 82 ++++++++++++++-- tests/gui/test_widgets.py | 158 +++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+), 7 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 731924065..f04c73a6b 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -696,6 +696,7 @@ def setup(self, controller): """ self.controller = controller self.source_list.setup(controller) + self.source_list.delete_source_by_uuid.connect(self.delete_conversation) def show_sources(self, sources: List[Source]): """ @@ -708,11 +709,7 @@ def show_sources(self, sources: List[Source]): else: self.empty_conversation_view.show_no_sources_message() self.empty_conversation_view.show() - - deleted_sources = self.source_list.update(sources) - for source_uuid in deleted_sources: - # Then call the function to remove the wrapper and its children. - self.delete_conversation(source_uuid) + self.source_list.update(sources) def on_source_changed(self): """ @@ -746,8 +743,6 @@ def delete_conversation(self, source_uuid: str) -> None: """ try: logger.debug('Deleting SourceConversationWrapper for {}'.format(source_uuid)) - conversation_wrapper = self.source_conversations[source_uuid] - conversation_wrapper.deleteLater() del self.source_conversations[source_uuid] except KeyError: logger.debug('No SourceConversationWrapper for {} to delete'.format(source_uuid)) @@ -903,6 +898,8 @@ class SourceList(QListWidget): } ''' + delete_source_by_uuid = pyqtSignal(str) + def __init__(self): super().__init__() @@ -977,6 +974,77 @@ def update(self, sources: List[Source]) -> List[str]: return deleted_uuids + def initial_update(self, sources: List[Source]): + """ + Reset and update the list with the passed in list of sources. + """ + # A flag to show if a source is currently selected (the current source + # doesn't have to be selected in a QListWidget -- it appears to default + # to whatever is in row 0, whether it's selected or not). + has_source_selected = False + if self.currentRow() > -1: + has_source_selected = self.item(self.currentRow()).isSelected() + current_source = self.get_current_source() + current_source_id = current_source and current_source.id + # Make a copy of the source_widgets that are currently displayed + # so that we can compare which widgets were removed. This means that + # the source was deleted, and we'll return this so we can also + # delete the corresponding SourceConversationWrapper. + existing_source_widgets = self.source_widgets.copy() + + # When we call clear() to delete all SourceWidgets, we should + # also clear the source_widgets dict. + self.clear() + self.source_widgets = {} + sources.reverse() + self.add_source(existing_source_widgets, sources, current_source_id, + has_source_selected) + + def add_source(self, existing_source_widgets, sources, current_source_id, + has_source_selected, slice_size=1): + """ + Add a slice of sources, ensure the currently selected source is + retained and, if necessary, reschedule the addition of more sources. + If no more sources are left, work out the deleted sources and emit + deletion signal for each source to delete. + """ + + def schedule_source_management(slice_size=slice_size): + if not sources: + # No more sources to add, discover the deleted sources and + # delete them. + deleted_uuids = list(set(existing_source_widgets.keys()) - + set(self.source_widgets.keys())) + for source_uuid in deleted_uuids: + self.delete_source_by_uuid.emit(source_uuid) + # Nothing more to do. + return + # Process the remaining "slice_size" number of sources. + sources_slice = sources[:slice_size] + for source in sources_slice: + new_source = SourceWidget(self.controller, source) + self.source_widgets[source.uuid] = new_source + list_item = QListWidgetItem(self) + list_item.setSizeHint(new_source.sizeHint()) + + self.addItem(list_item) + self.setItemWidget(list_item, new_source) + + if source.id == current_source_id and has_source_selected: + self.setCurrentItem(list_item) + # ATTENTION! 32 is an arbitrary number arrived at via + # experimentation. It adds plenty of sources, but doesn't block + # for a noticable amount of time. + new_slice_size = min(32, slice_size * 2) + # Call add_source again for the remaining sources. + self.add_source(existing_source_widgets, sources[slice_size:], + current_source_id, has_source_selected, + new_slice_size) + + # Schedule the closure defined above in the next iteration of the + # Qt event loop (thus unblocking the UI). + QTimer.singleShot(1, schedule_source_management) + def get_selected_source(self): if not self.selectedItems(): return None diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index e4eb17fe7..9aae43402 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -11,6 +11,7 @@ from PyQt5.QtTest import QTest from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \ QLineEdit +import sqlalchemy import sqlalchemy.orm.exc from sqlalchemy.orm import attributes, scoped_session, sessionmaker @@ -479,6 +480,7 @@ def test_MainView_setup(mocker): assert mv.controller == controller mv.source_list.setup.assert_called_once_with(controller) + mv.source_list.delete_source_by_uuid.connect.assert_called_once_with(mv.delete_conversation) def test_MainView_show_sources_with_none_selected(mocker): @@ -770,6 +772,25 @@ def test_SourceList_update_adds_new_sources(mocker): mock_sw.deleteLater.assert_not_called() +def test_SourceList_initial_update_adds_new_sources(mocker): + """ + Check a new SourceWidget for each passed-in source is created and no widgets are cleared or + removed. + """ + sl = SourceList() + + sl.clear = mocker.MagicMock() + sl.add_source = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.currentRow = mocker.MagicMock(return_value=0) + sl.item = mocker.MagicMock() + sl.item().isSelected.return_value = True + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + sl.initial_update(sources) + sl.clear.assert_called_once_with() + sl.add_source.assert_called_once_with({}, sources, None, True) + + def test_SourceList_update_when_source_deleted(mocker, session, session_maker, homedir): """ Test that SourceWidget.update raises an exception when its source has been deleted. @@ -810,6 +831,19 @@ def test_SourceList_update_when_source_deleted(mocker, session, session_maker, h assert len(sl.source_widgets) == 0 +def test_SourceList_add_source_starts_timer(mocker, session_maker, homedir): + """ + When the add_source method is called it schedules the addition of a source + to the source list via a single-shot QTimer. + """ + sl = SourceList() + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, None, False) + assert mock_timer.singleShot.call_count == 1 + + def test_SourceList_update_when_source_deleted_crash(mocker, session, session_maker, homedir): """ When SourceList.update calls SourceWidget.update and that @@ -949,6 +983,110 @@ def test_SourceList_update_removes_item_from_beginning_of_list(mocker): assert len(sl.source_widgets) == 2 +def test_SourceList_add_source_closure_adds_sources(mocker): + """ + The closure (function created within the add_source function) behaves in + the expected way given the context of the call to add_source. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() + + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, 1, False) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + assert mock_sw.call_count == 1 + assert mock_lwi.call_count == 1 + assert sl.addItem.call_count == 1 + assert sl.setItemWidget.call_count == 1 + assert len(sl.source_widgets) == 1 + assert sl.setCurrentItem.call_count == 0 + sl.add_source.assert_called_once_with({}, sources[1:], 1, False, 2) + + +def test_SourceList_add_source_closure_sets_current_item(mocker): + """ + The closure (function created within the add_source function) ensures that + if an item being added to the UI is the currently selected item and is + already selected in the UI, it is marked as such. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() + + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) + mock_source = mocker.MagicMock() + mock_source.id = 1 + sources = [mock_source, mocker.MagicMock(), mocker.MagicMock(), ] + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source({}, sources, 1, True) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + assert sl.setCurrentItem.call_count == 1 + + +def test_SourceList_add_source_closure_deletes_sources_when_finished(mocker): + """ + The closure (function created within the add_source function) ensures that + if an item being added to the UI is the currently selected item and is + already selected in the UI, it is marked as such. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() + + sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' + sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' + existing_sources = { + "uuid1": mocker.MagicMock() + } + + sl.source_widgets = { + "uuid2": mocker.MagicMock() + } + + sl.delete_source_by_uuid = mocker.MagicMock() + + mock_timer = mocker.MagicMock() + with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): + sl.add_source(existing_sources, [], 1, True) + # Now grab the function created within add_source: + inner_fn = mock_timer.singleShot.call_args_list[0][0][1] + # Ensure add_source is mocked to avoid recursion in the test and so we + # can spy on how it's called. + sl.add_source = mocker.MagicMock() + # Call the inner function (as if the timer had completed). + inner_fn() + sl.delete_source_by_uuid.emit.assert_called_once_with("uuid1") + + def test_SourceList_set_snippet(mocker): """ Handle the emitted event in the expected manner. @@ -1042,6 +1180,26 @@ def test_SourceWidget_update_attachment_icon(mocker): assert sw.paperclip.isHidden() +def test_SourceWidget_update_raises_InvalidRequestError(mocker): + """ + If the source no longer exists in the local data store, ensure the expected + exception is logged and re-raised. + """ + controller = mocker.MagicMock() + source = factory.Source(document_count=1) + sw = SourceWidget(controller, source) + ex = sqlalchemy.exc.InvalidRequestError() + controller.session.refresh.side_effect = ex + mock_logger = mocker.MagicMock() + mocker.patch( + "securedrop_client.gui.widgets.logger", + mock_logger, + ) + with pytest.raises(sqlalchemy.exc.InvalidRequestError): + sw.update() + assert mock_logger.error.call_count == 1 + + def test_SourceWidget_set_snippet(mocker, session_maker, session, homedir): """ Snippets are set as expected. From 67378062c7aabc64cad853a819cad15f517532f0 Mon Sep 17 00:00:00 2001 From: "Nicholas H.Tollervey" Date: Wed, 25 Mar 2020 14:58:05 +0000 Subject: [PATCH 3/6] Final fixes to this branch so chunked update only happens if no sources exist in source list. --- securedrop_client/gui/widgets.py | 644 ++++++++++------- tests/gui/test_widgets.py | 1104 +++++++++++++++++++++--------- 2 files changed, 1178 insertions(+), 570 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 776183cf3..45ee0ab28 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -22,12 +22,12 @@ import sys from gettext import gettext as _ -from typing import Dict, List, Union # noqa: F401 +from typing import Dict, List, Union, Optional # noqa: F401 from uuid import uuid4 from PyQt5.QtCore import Qt, pyqtSlot, pyqtSignal, QEvent, QTimer, QSize, pyqtBoundSignal, \ QObject, QPoint from PyQt5.QtGui import QIcon, QPalette, QBrush, QColor, QFont, QLinearGradient, QKeySequence, \ - QCursor, QKeyEvent, QCloseEvent + QCursor, QKeyEvent, QPixmap from PyQt5.QtWidgets import QApplication, QListWidget, QLabel, QWidget, QListWidgetItem, \ QHBoxLayout, QVBoxLayout, QLineEdit, QScrollArea, QDialog, QAction, QMenu, QMessageBox, \ QToolButton, QSizePolicy, QPlainTextEdit, QStatusBar, QGraphicsDropShadowEffect, QPushButton, \ @@ -382,6 +382,9 @@ def update_message(self, message: str, duration: int) -> None: continuously show message. """ self.status_bar.showMessage(message, duration) + new_width = self.fontMetrics().horizontalAdvance(message) + self.status_bar.setMinimumWidth(new_width) + self.status_bar.reformat() if duration != 0: self.status_timer.start(duration) @@ -448,17 +451,20 @@ def __init__(self): # Login button self.login_button = LoginButton() + # User button + self.user_button = UserButton() + # User icon - self.user_icon = QLabel() + self.user_icon = UserIconLabel() self.user_icon.setObjectName('user_icon') # Set css id self.user_icon.setFixedSize(QSize(30, 30)) self.user_icon.setAlignment(Qt.AlignCenter) self.user_icon_font = QFont() self.user_icon_font.setLetterSpacing(QFont.AbsoluteSpacing, 0.58) self.user_icon.setFont(self.user_icon_font) - - # User button - self.user_button = UserButton() + self.user_icon.clicked.connect(self.user_button.click) + # Set cursor. + self.user_icon.setCursor(QCursor(Qt.PointingHandCursor)) # Add widgets to user auth layout layout.addWidget(self.login_button, 1) @@ -488,6 +494,17 @@ def hide(self): self.login_button.show() +class UserIconLabel(QLabel): + """ + Makes a label clickable. (For the label containing the user icon.) + """ + + clicked = pyqtSignal() + + def mousePressEvent(self, e): + self.clicked.emit() + + class UserButton(SvgPushButton): """An menu button for the journalist menu @@ -692,14 +709,23 @@ def show_sources(self, sources: List[Source]): else: self.empty_conversation_view.show_no_sources_message() self.empty_conversation_view.show() - self.source_list.update(sources) + + if self.source_list.source_widgets: + # The source list already contains sources. + deleted_sources = self.source_list.update(sources) + for source_uuid in deleted_sources: + # Then call the function to remove the wrapper and its children. + self.delete_conversation(source_uuid) + else: + # We have an empty source list, so do an initial update. + self.source_list.initial_update(sources) def on_source_changed(self): """ Show conversation for the currently-selected source if it hasn't been deleted. If the current source no longer exists, clear the conversation for that source. """ - source = self.source_list.get_current_source() + source = self.source_list.get_selected_source() if not source: return @@ -726,6 +752,8 @@ def delete_conversation(self, source_uuid: str) -> None: """ try: logger.debug('Deleting SourceConversationWrapper for {}'.format(source_uuid)) + conversation_wrapper = self.source_conversations[source_uuid] + conversation_wrapper.deleteLater() del self.source_conversations[source_uuid] except KeyError: logger.debug('No SourceConversationWrapper for {} to delete'.format(source_uuid)) @@ -889,7 +917,7 @@ def __init__(self): # Set id and styles. self.setObjectName('sourcelist') self.setStyleSheet(self.CSS) - self.setFixedWidth(525) + self.setFixedWidth(540) self.setUniformItemSizes(True) # Set layout. @@ -907,49 +935,71 @@ def setup(self, controller): self.controller.file_ready.connect(self.set_snippet) self.controller.file_missing.connect(self.set_snippet) - def update(self, sources: List[Source]): - """ - Reset and update the list with the passed in list of sources. - """ - # A flag to show if a source is currently selected (the current source - # doesn't have to be selected in a QListWidget -- it appears to default - # to whatever is in row 0, whether it's selected or not). - has_source_selected = False - if self.currentRow() > -1: - has_source_selected = self.item(self.currentRow()).isSelected() - current_source = self.get_current_source() - current_source_id = current_source and current_source.id - # Make a copy of the source_widgets that are currently displayed - # so that we can compare which widgets were removed. This means that - # the source was deleted, and we'll return this so we can also - # delete the corresponding SourceConversationWrapper. - existing_source_widgets = self.source_widgets.copy() - - # When we call clear() to delete all SourceWidgets, we should - # also clear the source_widgets dict. - self.clear() - self.source_widgets = {} + def update(self, sources: List[Source]) -> List[str]: + """ + Update the list with the passed in list of sources. + """ + # Delete widgets that no longer exist in source list + source_uuids = [source.uuid for source in sources] + deleted_uuids = [] + for i in range(self.count()): + list_item = self.item(i) + list_widget = self.itemWidget(list_item) + + if list_widget and list_widget.source_uuid not in source_uuids: + if list_item.isSelected(): + self.setCurrentItem(None) + + try: + del self.source_widgets[list_widget.source_uuid] + except KeyError: + pass + + self.takeItem(i) + deleted_uuids.append(list_widget.source_uuid) + list_widget.deleteLater() + + # Create new widgets for new sources + widget_uuids = [self.itemWidget(self.item(i)).source_uuid for i in range(self.count())] + for source in sources: + if source.uuid in widget_uuids: + try: + self.source_widgets[source.uuid].update() + except sqlalchemy.exc.InvalidRequestError as e: + logger.error( + "Could not update SourceWidget for source %s; deleting it. Error was: %s", + source.uuid, + e + ) + deleted_uuids.append(source.uuid) + self.source_widgets[source.uuid].deleteLater() + del self.source_widgets[list_widget.source_uuid] + else: + new_source = SourceWidget(self.controller, source) + self.source_widgets[source.uuid] = new_source + + list_item = QListWidgetItem() + self.insertItem(0, list_item) + list_item.setSizeHint(new_source.sizeHint()) + self.setItemWidget(list_item, new_source) + + return deleted_uuids + + def initial_update(self, sources: List[Source]): + """ + Initialise the list with the passed in list of sources. + """ sources.reverse() - self.add_source(existing_source_widgets, sources, current_source_id, - has_source_selected) + self.add_source(sources) - def add_source(self, existing_source_widgets, sources, current_source_id, - has_source_selected, slice_size=1): + def add_source(self, sources, slice_size=1): """ - Add a slice of sources, ensure the currently selected source is - retained and, if necessary, reschedule the addition of more sources. - If no more sources are left, work out the deleted sources and emit - deletion signal for each source to delete. + Add a slice of sources, and if necessary, reschedule the addition of + more sources. """ def schedule_source_management(slice_size=slice_size): if not sources: - # No more sources to add, discover the deleted sources and - # delete them. - deleted_uuids = list(set(existing_source_widgets.keys()) - - set(self.source_widgets.keys())) - for source_uuid in deleted_uuids: - self.delete_source_by_uuid.emit(source_uuid) # Nothing more to do. return # Process the remaining "slice_size" number of sources. @@ -962,37 +1012,55 @@ def schedule_source_management(slice_size=slice_size): self.addItem(list_item) self.setItemWidget(list_item, new_source) - - if source.id == current_source_id and has_source_selected: - self.setCurrentItem(list_item) # ATTENTION! 32 is an arbitrary number arrived at via # experimentation. It adds plenty of sources, but doesn't block # for a noticable amount of time. new_slice_size = min(32, slice_size * 2) # Call add_source again for the remaining sources. - self.add_source(existing_source_widgets, sources[slice_size:], - current_source_id, has_source_selected, - new_slice_size) + self.add_source(sources[slice_size:], new_slice_size) # Schedule the closure defined above in the next iteration of the # Qt event loop (thus unblocking the UI). QTimer.singleShot(1, schedule_source_management) - def get_current_source(self): - source_item = self.currentItem() + def get_selected_source(self): + if not self.selectedItems(): + return None + + source_item = self.selectedItems()[0] source_widget = self.itemWidget(source_item) - if source_widget and source_exists(self.controller.session, source_widget.source.uuid): + if source_widget and source_exists(self.controller.session, source_widget.source_uuid): return source_widget.source - def set_snippet(self, source_uuid, message_uuid, content): - """ - Given a UUID of a source, if the referenced message is the latest - message, then update the source's preview snippet to the referenced - content. - """ - source_widget = self.source_widgets.get(source_uuid) + def get_source_widget(self, source_uuid: str) -> Optional[QListWidget]: + ''' + First try to get the source widget from the cache, then look for it in the SourceList. + ''' + try: + source_widget = self.source_widgets[source_uuid] + return source_widget + except KeyError: + pass + + for i in range(self.count()): + list_item = self.item(i) + source_widget = self.itemWidget(list_item) + if source_widget and source_widget.source_uuid == source_uuid: + return source_widget + + return None + + @pyqtSlot(str, str, str) + def set_snippet(self, source_uuid: str, collection_item_uuid: str, content: str) -> None: + ''' + Set the source widget's preview snippet with the supplied content. + + Note: The signal's `collection_item_uuid` is not needed for setting the preview snippet. It + is used by other signal handlers. + ''' + source_widget = self.get_source_widget(source_uuid) if source_widget: - source_widget.set_snippet(source_uuid, message_uuid, content) + source_widget.set_snippet(source_uuid, content) class SourceWidget(QWidget): @@ -1069,11 +1137,11 @@ def __init__(self, controller: Controller, source: Source): self.controller = controller self.controller.source_deleted.connect(self._on_source_deleted) + self.controller.source_deletion_failed.connect(self._on_source_deletion_failed) # Store source self.source_uuid = source.uuid self.source = source - self.source_uuid = source.uuid # Set styles self.setStyleSheet(self.CSS) @@ -1099,7 +1167,7 @@ def __init__(self, controller: Controller, source: Source): gutter_layout = QVBoxLayout(self.gutter) gutter_layout.setContentsMargins(0, 0, 0, 0) gutter_layout.setSpacing(0) - self.star = StarToggleButton(self.controller, self.source) + self.star = StarToggleButton(self.controller, self.source_uuid, source.is_starred) gutter_layout.addWidget(self.star) gutter_layout.addStretch() @@ -1163,31 +1231,28 @@ def update(self): self.controller.session.refresh(self.source) self.timestamp.setText(_(arrow.get(self.source.last_updated).format('DD MMM'))) self.name.setText(self.source.journalist_designation) - self.set_snippet(self.source.uuid) + + if not self.source.collection: + self.set_snippet(self.source_uuid, '') + else: + last_collection_obj = self.source.collection[-1] + self.set_snippet(self.source_uuid, str(last_collection_obj)) + if self.source.document_count == 0: self.paperclip.hide() - self.star.update() + self.star.update(self.source.is_starred) except sqlalchemy.exc.InvalidRequestError as e: - logger.error( - "Could not update SourceWidget for source %s: %s", - self.source.uuid, - e - ) + logger.error(f"Could not update SourceWidget for source {self.source_uuid}: {e}") raise - def set_snippet(self, source, uuid=None, content=None): + def set_snippet(self, source_uuid: str, content: str): """ - Update the preview snippet only if the new message is for the - referenced source and there's a source collection. If a uuid and - content are passed then use these, otherwise default to whatever the - latest item in the conversation might be. + Update the preview snippet if the source_uuid matches our own. """ - if source == self.source.uuid and self.source.collection: - last_collection_object = self.source.collection[-1] - if uuid and uuid == last_collection_object.uuid and content: - self.preview.setText(content) - else: - self.preview.setText(str(last_collection_object)) + if source_uuid != self.source_uuid: + return + + self.preview.setText(content) def delete_source(self, event): if self.controller.api is None: @@ -1205,6 +1270,14 @@ def _on_source_deleted(self, source_uuid: str): self.preview.hide() self.waiting_delete_confirmation.show() + @pyqtSlot(str) + def _on_source_deletion_failed(self, source_uuid: str): + if self.source_uuid == source_uuid: + self.waiting_delete_confirmation.hide() + self.gutter.show() + self.metadata.show() + self.preview.show() + class StarToggleButton(SvgToggleButton): """ @@ -1217,137 +1290,146 @@ class StarToggleButton(SvgToggleButton): } ''' - def __init__(self, controller: Controller, source: Source): - super().__init__( - on='star_on.svg', - off='star_off.svg', - svg_size=QSize(16, 16)) + def __init__(self, controller: Controller, source_uuid: str, is_starred: bool): + super().__init__(on='star_on.svg', off='star_off.svg', svg_size=QSize(16, 16)) self.controller = controller - self.source = source + self.source_uuid = source_uuid + self.is_starred = is_starred + self.pending_count = 0 + self.wait_until_next_sync = False + self.controller.authentication_state.connect(self.on_authentication_changed) + self.controller.star_update_failed.connect(self.on_star_update_failed) + self.controller.star_update_successful.connect(self.on_star_update_successful) self.installEventFilter(self) self.setObjectName('star_button') self.setStyleSheet(self.css) self.setFixedSize(QSize(20, 20)) - self.controller.authentication_state.connect(self.on_authentication_changed) - self.on_authentication_changed(self.controller.is_authenticated) + self.pressed.connect(self.on_pressed) + self.setCheckable(True) + self.setChecked(self.is_starred) - def disable(self): - """ - Disable the widget. + if not self.controller.is_authenticated: + self.disable_toggle() - Disable toggle by setting checkable to False. Unfortunately, - disabling toggle doesn't freeze state, rather it always - displays the off state when a user tries to toggle. In order - to save on state we update the icon's off state image to - display on (hack). + def disable_toggle(self): """ - self.disable_api_call() - self.setCheckable(False) - if self.source.is_starred: - self.set_icon(on='star_on.svg', off='star_on.svg') + Unset `checkable` so that the star cannot be toggled. - try: - while True: - self.pressed.disconnect(self.on_toggle_offline) - except Exception as e: - logger.warning("Could not disconnect on_toggle_offline from self.pressed: %s", e) - - try: - while True: - self.toggled.disconnect(self.on_toggle) - except Exception as e: - logger.warning("Could not disconnect on_toggle from self.toggled: %s", e) + Disconnect the `pressed` signal from previous handler and connect it to the offline handler. + """ + self.pressed.disconnect() + self.pressed.connect(self.on_pressed_offline) - self.pressed.connect(self.on_toggle_offline) + # If the source is starred, we must update the icon so that the off state continues to show + # the source as starred. We could instead disable the button, which will continue to show + # the star as checked, but Qt will also gray out the star, which we don't want. + if self.is_starred: + self.set_icon(on='star_on.svg', off='star_on.svg') + self.setCheckable(False) - def enable(self): + def enable_toggle(self): """ Enable the widget. - """ - self.enable_api_call() - self.setCheckable(True) - self.set_icon(on='star_on.svg', off='star_off.svg') - self.setChecked(self.source.is_starred) - try: - while True: - self.toggled.disconnect(self.on_toggle) - except Exception: - pass + Disconnect the pressed signal from previous handler, set checkable so that the star can be + toggled, and connect to the online toggle handler. - try: - while True: - self.pressed.disconnect(self.on_toggle_offline) - except Exception: - pass - - self.toggled.connect(self.on_toggle) + Note: We must update the icon in case it was modified after being disabled. + """ + self.pressed.disconnect() + self.pressed.connect(self.on_pressed) + self.setCheckable(True) + self.set_icon(on='star_on.svg', off='star_off.svg') # Undo icon change from disable_toggle def eventFilter(self, obj, event): - checkable = self.isCheckable() + """ + If the button is checkable then we show a hover state. + """ + if not self.isCheckable(): + return QObject.event(obj, event) + t = event.type() - if t == QEvent.HoverEnter and checkable: + if t == QEvent.HoverEnter: self.setIcon(load_icon('star_hover.svg')) elif t == QEvent.HoverLeave: - if checkable: - self.set_icon(on='star_on.svg', off='star_off.svg') - else: - if self.source.is_starred: - self.set_icon(on='star_on.svg', off='star_on.svg') + self.set_icon(on='star_on.svg', off='star_off.svg') + return QObject.event(obj, event) - def on_authentication_changed(self, authenticated: bool): + @pyqtSlot(bool) + def on_authentication_changed(self, authenticated: bool) -> None: """ Set up handlers based on whether or not the user is authenticated. Connect to 'pressed' event instead of 'toggled' event when not authenticated because toggling will be disabled. """ if authenticated: - self.enable() + self.pending_count = 0 + self.enable_toggle() + self.setChecked(self.is_starred) else: - self.disable() + self.disable_toggle() - def on_toggle(self): + @pyqtSlot() + def on_pressed(self) -> None: """ Tell the controller to make an API call to update the source's starred field. """ - if self.is_api_call_enabled: - self.controller.update_star(self.source, self.on_update) + self.controller.update_star(self.source_uuid, self.isChecked()) + self.is_starred = not self.is_starred + self.pending_count = self.pending_count + 1 + self.wait_until_next_sync = True - def on_toggle_offline(self): + @pyqtSlot() + def on_pressed_offline(self) -> None: """ Show error message when not authenticated. """ self.controller.on_action_requiring_login() - def on_update(self, result): + @pyqtSlot(bool) + def update(self, is_starred: bool) -> None: """ - The result is a uuid for the source and boolean flag for the new state - of the star. + If star was updated via the Journalist Interface or by another instance of the client, then + self.is_starred will not match the server and will need to be updated. """ - enabled = result[1] - self.source.is_starred = enabled - self.controller.session.commit() - self.controller.update_sources() - self.setChecked(enabled) + if not self.controller.is_authenticated: + return - def update(self): + # Wait until ongoing star jobs are finished before checking if it matches with the server + if self.pending_count > 0: + return + + # Wait until next sync to avoid the possibility of updating the star with outdated source + # information in case the server just received the star request. + if self.wait_until_next_sync: + self.wait_until_next_sync = False + return + + if self.is_starred != is_starred: + self.is_starred = is_starred + self.setChecked(self.is_starred) + + @pyqtSlot(str, bool) + def on_star_update_failed(self, source_uuid: str, is_starred: bool) -> None: """ - Update the star to reflect its source's current state. + If the star update failed to update on the server, toggle back to previous state. """ - self.controller.session.refresh(self.source) - self.disable_api_call() - self.setChecked(self.source.is_starred) - self.enable_api_call() - - def disable_api_call(self): - self.is_api_call_enabled = False + if self.source_uuid == source_uuid: + self.is_starred = is_starred + self.pending_count = self.pending_count - 1 + QTimer.singleShot(250, lambda: self.setChecked(self.is_starred)) - def enable_api_call(self): - self.is_api_call_enabled = True + @pyqtSlot(str) + def on_star_update_successful(self, source_uuid: str) -> None: + """ + If the star update succeeded, set pending to False so the sync can update the star field + """ + if self.source_uuid == source_uuid: + self.pending_count = self.pending_count - 1 class DeleteSourceMessageBox: @@ -1355,6 +1437,7 @@ class DeleteSourceMessageBox: def __init__(self, source, controller): self.source = source + self.source_uuid = source.uuid self.controller = controller def launch(self): @@ -1370,7 +1453,7 @@ def launch(self): None, "", _(message), QMessageBox.Cancel | QMessageBox.Yes, QMessageBox.Cancel) if reply == QMessageBox.Yes: - logger.debug("Deleting source %s" % (self.source.uuid,)) + logger.debug(f'Deleting source {self.source_uuid}') self.controller.delete_source(self.source) def _construct_message(self, source: Source) -> str: @@ -1606,6 +1689,9 @@ def __init__(self, parent): self.parent = parent super().__init__(self.parent) + # Set modal + self.setModal(True) + # Set css id self.setObjectName('login_dialog') @@ -2204,6 +2290,13 @@ def __init__( file_ready_signal.connect(self._on_file_downloaded, type=Qt.QueuedConnection) file_missing.connect(self._on_file_missing, type=Qt.QueuedConnection) + def update_file_size(self): + try: + self.file_size.setText(humanize_filesize(self.file.size)) + except Exception as e: + logger.error(f"Could not update file size on FileWidget: {e}") + self.file_size.setText("") + def eventFilter(self, obj, event): t = event.type() if t == QEvent.MouseButtonPress: @@ -2227,6 +2320,7 @@ def _set_file_state(self): self.middot.show() self.print_button.show() self.file_name.show() + self.update_file_size() else: logger.debug('Changing file {} state to not downloaded'.format(self.uuid)) self.download_button.setText(_('DOWNLOAD')) @@ -2271,8 +2365,8 @@ def _on_export_clicked(self): if not self.controller.downloaded_file_exists(self.file): return - dialog = ExportDialog(self.controller, self.uuid, self.file.filename) - dialog.exec() + self.export_dialog = ExportDialog(self.controller, self.uuid, self.file.filename) + self.export_dialog.show() @pyqtSlot() def _on_print_clicked(self): @@ -2296,7 +2390,7 @@ def _on_left_click(self): if self.file.is_decrypted: # Open the already downloaded and decrypted file. self.controller.on_file_open(self.file) - else: + elif not self.downloading: if self.controller.api: self.start_button_animation() # Download the file. @@ -2328,25 +2422,17 @@ def stop_button_animation(self): self._set_file_state() -class FramelessDialog(QDialog): +class ModalDialog(QDialog): CSS = ''' - #frameless_dialog { + #modal { min-width: 800px; max-width: 800px; min-height: 300px; max-height: 800px; background-color: #fff; - border: 1px solid #2a319d; } - #close_button { - border: none; - font-family: 'Source Sans Pro'; - font-weight: 600; - font-size: 12px; - color: #2a319d; - } - #header_icon { + #header_icon, #header_spinner { min-width: 80px; max-width: 80px; min-height: 64px; @@ -2412,37 +2498,9 @@ class FramelessDialog(QDialog): def __init__(self): parent = QApplication.activeWindow() super().__init__(parent) - - # Used to determine whether or not the popup is being closed from our own internal close - # method or from clicking outside of the popup. - # - # This is a workaround for for frameless - # modals not working as expected on QubesOS, i.e. the following flags are ignored in Qubes: - # self.setWindowFlags(Qt.FramelessWindowHint) - # self.setWindowFlags(Qt.CustomizeWindowHint) - self.internal_close_event_emitted = False - - self.setObjectName('frameless_dialog') + self.setObjectName('modal') self.setStyleSheet(self.CSS) - self.setWindowFlags(Qt.Popup) - - # Set drop shadow effect - effect = QGraphicsDropShadowEffect(self) - effect.setOffset(0, 1) - effect.setBlurRadius(8) - effect.setColor(QColor('#aa000000')) - self.setGraphicsEffect(effect) - self.update() - - # Custom titlebar for close button - titlebar = QWidget() - titlebar_layout = QVBoxLayout() - titlebar.setLayout(titlebar_layout) - close_button = SvgPushButton('delete_close.svg', svg_size=QSize(10, 10)) - close_button.setObjectName('close_button') - close_button.setText('CLOSE') - close_button.clicked.connect(self.close) - titlebar_layout.addWidget(close_button, alignment=Qt.AlignRight) + self.setModal(True) # Header for icon and task title header_container = QWidget() @@ -2450,9 +2508,16 @@ def __init__(self): header_container.setLayout(header_container_layout) self.header_icon = SvgLabel('blank.svg', svg_size=QSize(64, 64)) self.header_icon.setObjectName('header_icon') + self.header_spinner = QPixmap() + self.header_spinner_label = QLabel() + self.header_spinner_label.setObjectName("header_spinner") + self.header_spinner_label.setMinimumSize(64, 64) + self.header_spinner_label.setVisible(False) + self.header_spinner_label.setPixmap(self.header_spinner) self.header = QLabel() self.header.setObjectName('header') header_container_layout.addWidget(self.header_icon) + header_container_layout.addWidget(self.header_spinner_label) header_container_layout.addWidget(self.header, alignment=Qt.AlignCenter) header_container_layout.addStretch() @@ -2483,9 +2548,11 @@ def __init__(self): window_buttons.setLayout(button_layout) self.cancel_button = QPushButton(_('CANCEL')) self.cancel_button.clicked.connect(self.close) + self.cancel_button.setAutoDefault(False) self.continue_button = QPushButton(_('CONTINUE')) self.continue_button.setObjectName('primary_button') self.continue_button.setDefault(True) + self.continue_button.setIconSize(QSize(21, 21)) button_box = QDialogButtonBox(Qt.Horizontal) button_box.setObjectName('button_box') button_box.addButton(self.cancel_button, QDialogButtonBox.ActionRole) @@ -2496,7 +2563,6 @@ def __init__(self): # Main widget layout layout = QVBoxLayout(self) self.setLayout(layout) - layout.addWidget(titlebar) layout.addWidget(header_container) layout.addWidget(self.header_line) layout.addWidget(self.error_details) @@ -2504,42 +2570,68 @@ def __init__(self): layout.addStretch() layout.addWidget(window_buttons) - def closeEvent(self, event: QCloseEvent): - # ignore any close event that doesn't come from our custom close method - if not self.internal_close_event_emitted: - event.ignore() + # Activestate animation. + self.button_animation = load_movie("activestate-wide.gif") + self.button_animation.setScaledSize(QSize(32, 32)) + self.button_animation.frameChanged.connect(self.animate_activestate) + + # Header animation. + self.header_animation = load_movie("header_animation.gif") + self.header_animation.setScaledSize(QSize(64, 64)) + self.header_animation.frameChanged.connect(self.animate_header) def keyPressEvent(self, event: QKeyEvent): - # Since the dialog sets the Qt.Popup window flag (in order to achieve a frameless dialog - # window in Qubes), the default behavior is to close the dialog when the Enter or Return - # key is clicked, which we override here. if (event.key() == Qt.Key_Enter or event.key() == Qt.Key_Return): if self.cancel_button.hasFocus(): self.cancel_button.click() else: self.continue_button.click() - return + event.ignore() # Don't allow Enter to close dialog super().keyPressEvent(event) - def close(self): - self.internal_close_event_emitted = True - super().close() + def animate_activestate(self): + self.continue_button.setIcon(QIcon(self.button_animation.currentPixmap())) - def center_dialog(self): - active_window = QApplication.activeWindow() - if not active_window: - return - application_window_size = active_window.geometry() - dialog_size = self.geometry() - x = application_window_size.x() - y = application_window_size.y() - x_center = (application_window_size.width() - dialog_size.width()) / 2 - y_center = (application_window_size.height() - dialog_size.height()) / 2 - self.move(x + x_center, y + y_center) + def animate_header(self): + self.header_spinner_label.setPixmap(self.header_animation.currentPixmap()) + + def start_animate_activestate(self): + self.button_animation.start() + self.continue_button.setText("") + self.continue_button.setMinimumSize(QSize(142, 43)) + css = """ + background-color: #f1f1f6; + color: #fff; + border: 2px solid #f1f1f6; + margin: 0px 0px 0px 12px; + height: 40px; + padding-left: 20px; + padding-right: 20px; + """ + self.continue_button.setStyleSheet(css) + self.error_details.setStyleSheet("color: #ff66C4") + + def start_animate_header(self): + self.header_icon.setVisible(False) + self.header_spinner_label.setVisible(True) + self.header_animation.start() + def stop_animate_activestate(self): + self.continue_button.setIcon(QIcon()) + self.button_animation.stop() + self.continue_button.setText(_('CONTINUE')) + css = "background-color: #2a319d; color: #fff; border: 2px solid #2a319d;" + self.continue_button.setStyleSheet(css) + self.error_details.setStyleSheet("color: #ff0064") -class PrintDialog(FramelessDialog): + def stop_animate_header(self): + self.header_icon.setVisible(True) + self.header_spinner_label.setVisible(False) + self.header_animation.stop() + + +class PrintDialog(ModalDialog): FILENAME_WIDTH_PX = 260 @@ -2558,23 +2650,26 @@ def __init__(self, controller: Controller, file_uuid: str, file_name: str): # Connect parent signals to slots self.continue_button.setEnabled(False) - - self.header_icon.update_image('printer.svg', svg_size=QSize(64, 64)) + self.continue_button.clicked.connect(self._run_preflight) # Dialog content self.starting_header = _( 'Preparing to print:' '
' '{}'.format(self.file_name)) - self.insert_usb_header = _('Insert USB printer') - self.error_header = _('Unable to print') + self.ready_header = _( + 'Ready to print:' + '
' + '{}'.format(self.file_name)) + self.insert_usb_header = _('Connect USB printer') + self.error_header = _('Printing failed') self.starting_message = _( '

Managing printout risks

' - 'QR-Codes and visible web addresses' + 'QR codes and web addresses' '
' - 'Never open web addresses or scan QR codes contained in printed documents without ' - 'taking security precautions. If you are unsure how to manage this risk, please ' - 'contact your administrator.' + 'Never type in and open web addresses or scan QR codes contained in printed ' + 'documents without taking security precautions. If you are unsure how to ' + 'manage this risk, please contact your administrator.' '

' 'Printer dots' '
' @@ -2585,6 +2680,7 @@ def __init__(self, controller: Controller, file_uuid: str, file_name: str): self.generic_error_message = _('See your administrator for help.') self._show_starting_instructions() + self.start_animate_header() self._run_preflight() def _show_starting_instructions(self): @@ -2592,7 +2688,6 @@ def _show_starting_instructions(self): self.body.setText(self.starting_message) self.error_details.hide() self.adjustSize() - self.center_dialog() def _show_insert_usb_message(self): self.continue_button.clicked.disconnect() @@ -2601,7 +2696,6 @@ def _show_insert_usb_message(self): self.body.setText(self.insert_usb_message) self.error_details.hide() self.adjustSize() - self.center_dialog() def _show_generic_error_message(self): self.continue_button.clicked.disconnect() @@ -2611,7 +2705,6 @@ def _show_generic_error_message(self): self.body.setText('{}: {}'.format(self.error_status, self.generic_error_message)) self.error_details.hide() self.adjustSize() - self.center_dialog() @pyqtSlot() def _run_preflight(self): @@ -2625,16 +2718,22 @@ def _print_file(self): @pyqtSlot() def _on_preflight_success(self): # If the continue button is disabled then this is the result of a background preflight check + self.stop_animate_header() + self.header_icon.update_image('printer.svg', svg_size=QSize(64, 64)) + self.header.setText(self.ready_header) if not self.continue_button.isEnabled(): self.continue_button.clicked.disconnect() self.continue_button.clicked.connect(self._print_file) self.continue_button.setEnabled(True) + self.continue_button.setFocus() return self._print_file() @pyqtSlot(object) def _on_preflight_failure(self, error: ExportError): + self.stop_animate_header() + self.header_icon.update_image('printer.svg', svg_size=QSize(64, 64)) self.error_status = error.status # If the continue button is disabled then this is the result of a background preflight check if not self.continue_button.isEnabled(): @@ -2645,6 +2744,7 @@ def _on_preflight_failure(self, error: ExportError): self.continue_button.clicked.connect(self._show_generic_error_message) self.continue_button.setEnabled(True) + self.continue_button.setFocus() else: if error.status == ExportStatus.PRINTER_NOT_FOUND.value: self._show_insert_usb_message() @@ -2652,7 +2752,7 @@ def _on_preflight_failure(self, error: ExportError): self._show_generic_error_message() -class ExportDialog(FramelessDialog): +class ExportDialog(ModalDialog): PASSPHRASE_FORM_CSS = ''' #passphrase_form QLabel { @@ -2692,31 +2792,34 @@ def __init__(self, controller: Controller, file_uuid: str, file_name: str): # Connect parent signals to slots self.continue_button.setEnabled(False) - - self.header_icon.update_image('savetodisk.svg', QSize(64, 64)) + self.continue_button.clicked.connect(self._run_preflight) # Dialog content self.starting_header = _( 'Preparing to export:' '
' '{}'.format(self.file_name)) + self.ready_header = _( + 'Ready to export:' + '
' + '{}'.format(self.file_name)) self.insert_usb_header = _('Insert encrypted USB drive') self.passphrase_header = _('Enter passphrase for USB drive') self.success_header = _('Export successful') - self.error_header = _('Unable to export') + self.error_header = _('Export failed') self.starting_message = _( - '

Proceed with caution when exporting files

' + '

Understand the risks before exporting files

' 'Malware' '
' - 'This workstation lets you open documents securely. If you open documents on another ' + 'This workstation lets you open files securely. If you open files on another ' 'computer, any embedded malware may spread to your computer or network. If you are ' - 'unsure how to manage this risk, please print the document, or contact your ' + 'unsure how to manage this risk, please print the file, or contact your ' 'administrator.' '

' 'Anonymity' '
' - 'Documents submitted by sources may contain information or hidden metadata that ' - 'identifies who they are. To protect your sources, please consider redacting documents ' + 'Files submitted by sources may contain information or hidden metadata that ' + 'identifies who they are. To protect your sources, please consider redacting files ' 'before working with them on network-connected computers.') self.exporting_message = _('Exporting: {}'.format(self.file_name)) self.insert_usb_message = _( @@ -2757,13 +2860,13 @@ def __init__(self, controller: Controller, file_uuid: str, file_name: str): self.passphrase_form.hide() self._show_starting_instructions() + self.start_animate_header() self._run_preflight() def _show_starting_instructions(self): self.header.setText(self.starting_header) self.body.setText(self.starting_message) self.adjustSize() - self.center_dialog() def _show_passphrase_request_message(self): self.continue_button.clicked.disconnect() @@ -2775,7 +2878,6 @@ def _show_passphrase_request_message(self): self.body.hide() self.passphrase_form.show() self.adjustSize() - self.center_dialog() def _show_passphrase_request_message_again(self): self.continue_button.clicked.disconnect() @@ -2788,7 +2890,6 @@ def _show_passphrase_request_message_again(self): self.error_details.show() self.passphrase_form.show() self.adjustSize() - self.center_dialog() def _show_success_message(self): self.continue_button.clicked.disconnect() @@ -2802,7 +2903,6 @@ def _show_success_message(self): self.header_line.show() self.body.show() self.adjustSize() - self.center_dialog() def _show_insert_usb_message(self): self.continue_button.clicked.disconnect() @@ -2815,7 +2915,6 @@ def _show_insert_usb_message(self): self.header_line.show() self.body.show() self.adjustSize() - self.center_dialog() def _show_insert_encrypted_usb_message(self): self.continue_button.clicked.disconnect() @@ -2829,7 +2928,6 @@ def _show_insert_encrypted_usb_message(self): self.error_details.show() self.body.show() self.adjustSize() - self.center_dialog() def _show_generic_error_message(self): self.continue_button.clicked.disconnect() @@ -2842,7 +2940,6 @@ def _show_generic_error_message(self): self.header_line.show() self.body.show() self.adjustSize() - self.center_dialog() @pyqtSlot() def _run_preflight(self): @@ -2850,29 +2947,40 @@ def _run_preflight(self): @pyqtSlot() def _export_file(self, checked: bool = False): + self.start_animate_activestate() + self.passphrase_field.setDisabled(True) self.controller.export_file_to_usb_drive(self.file_uuid, self.passphrase_field.text()) @pyqtSlot() def _on_preflight_success(self): # If the continue button is disabled then this is the result of a background preflight check + self.stop_animate_header() + self.header_icon.update_image('savetodisk.svg', QSize(64, 64)) + self.header.setText(self.ready_header) if not self.continue_button.isEnabled(): self.continue_button.clicked.disconnect() self.continue_button.clicked.connect(self._show_passphrase_request_message) self.continue_button.setEnabled(True) + self.continue_button.setFocus() return self._show_passphrase_request_message() @pyqtSlot(object) def _on_preflight_failure(self, error: ExportError): + self.stop_animate_header() + self.header_icon.update_image('savetodisk.svg', QSize(64, 64)) self._update_dialog(error.status) @pyqtSlot() def _on_export_success(self): + self.stop_animate_activestate() self._show_success_message() @pyqtSlot(object) def _on_export_failure(self, error: ExportError): + self.stop_animate_activestate() + self.passphrase_field.setDisabled(False) self._update_dialog(error.status) def _update_dialog(self, error_status: str): @@ -2890,6 +2998,7 @@ def _update_dialog(self, error_status: str): self.continue_button.clicked.connect(self._show_generic_error_message) self.continue_button.setEnabled(True) + self.continue_button.setFocus() else: if self.error_status == ExportStatus.BAD_PASSPHRASE.value: self._show_passphrase_request_message_again() @@ -3136,6 +3245,7 @@ def __init__(self, source: Source, controller: Controller) -> None: self.source_uuid = source.uuid controller.source_deleted.connect(self._on_source_deleted) + controller.source_deletion_failed.connect(self._on_source_deletion_failed) # Set layout layout = QVBoxLayout() @@ -3173,6 +3283,14 @@ def _on_source_deleted(self, source_uuid: str): self.reply_box.hide() self.waiting_delete_confirmation.show() + @pyqtSlot(str) + def _on_source_deletion_failed(self, source_uuid: str): + if self.source_uuid == source_uuid: + self.waiting_delete_confirmation.hide() + self.conversation_title_bar.show() + self.conversation_view.show() + self.reply_box.show() + class ReplyBoxWidget(QWidget): """ @@ -3309,7 +3427,7 @@ def _on_authentication_changed(self, authenticated: bool) -> None: try: self.update_authentication_state(authenticated) except sqlalchemy.orm.exc.ObjectDeletedError: - logger.error( + logger.debug( "On authentication change, ReplyBoxWidget found its source had been deleted." ) self.destroy() @@ -3331,7 +3449,7 @@ def _on_synced(self, data: str) -> None: else: self.refocus_after_sync = False except sqlalchemy.orm.exc.ObjectDeletedError: - logger.error("During sync, ReplyBoxWidget found its source had been deleted.") + logger.debug("During sync, ReplyBoxWidget found its source had been deleted.") self.destroy() diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 99c76f255..8df68d823 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -10,7 +10,7 @@ from PyQt5.QtGui import QFocusEvent, QKeyEvent, QMovie from PyQt5.QtTest import QTest from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \ - QLineEdit, QListWidgetItem + QLineEdit import sqlalchemy import sqlalchemy.orm.exc from sqlalchemy.orm import attributes, scoped_session, sessionmaker @@ -22,8 +22,8 @@ DeleteSourceMessageBox, DeleteSourceAction, SourceMenu, TopPane, LeftPane, SyncIcon, \ ErrorStatusBar, ActivityStatusBar, UserProfile, UserButton, UserMenu, LoginButton, \ ReplyBoxWidget, ReplyTextEdit, SourceConversationWrapper, StarToggleButton, LoginOfflineLink, \ - LoginErrorBar, EmptyConversationView, FramelessDialog, ExportDialog, PrintDialog, \ - PasswordEdit, SourceProfileShortWidget + LoginErrorBar, EmptyConversationView, ModalDialog, ExportDialog, PrintDialog, \ + PasswordEdit, SourceProfileShortWidget, UserIconLabel from tests import factory @@ -384,6 +384,13 @@ def test_UserProfile_hide(mocker): up.login_button.show.assert_called_once_with() +def test_UserIconLabel_clicks(mocker): + uil = UserIconLabel() + uil.clicked = mocker.MagicMock() + uil.mousePressEvent(None) + uil.clicked.emit.assert_called_once_with() + + def test_UserButton_setup(mocker): ub = UserButton() ub.menu = mocker.MagicMock() @@ -491,6 +498,20 @@ def test_MainView_show_sources_with_none_selected(mocker): mv.empty_conversation_view.show.assert_called_once_with() +def test_MainView_show_sources_from_cold_start(mocker): + """ + Ensure the initial_update is called if no sources exist in the UI. + """ + mv = MainView(None) + mv.source_list = mocker.MagicMock() + mv.source_list.source_widgets = [] + mv.empty_conversation_view = mocker.MagicMock() + + mv.show_sources([]) + + mv.source_list.initial_update.assert_called_once_with([]) + + def test_MainView_show_sources_with_no_sources_at_all(mocker): """ Ensure the sources list is passed to the source list widget to be updated. @@ -506,6 +527,25 @@ def test_MainView_show_sources_with_no_sources_at_all(mocker): mv.empty_conversation_view.show.assert_called_once_with() +def test_MainView_show_sources_when_sources_are_deleted(mocker): + """ + Ensure that show_sources also deletes the SourceConversationWrapper for a deleted source. + """ + mv = MainView(None) + mv.source_list = mocker.MagicMock() + mv.empty_conversation_view = mocker.MagicMock() + mv.source_list.update = mocker.MagicMock(return_value=[]) + mv.delete_conversation = mocker.MagicMock() + + mv.show_sources([1, 2, 3, 4]) + + mv.source_list.update = mocker.MagicMock(return_value=[4]) + + mv.show_sources([1, 2, 3]) + + mv.delete_conversation.assert_called_once_with(4) + + def test_MainView_delete_conversation_when_conv_wrapper_exists(mocker): """ Ensure SourceConversationWrapper is deleted if it exists. @@ -521,6 +561,7 @@ def test_MainView_delete_conversation_when_conv_wrapper_exists(mocker): mv.delete_conversation('123') + conversation_wrapper.deleteLater.assert_called_once_with() assert mv.source_conversations == {} @@ -545,7 +586,7 @@ def test_MainView_on_source_changed(mocker): mv = MainView(None) mv.set_conversation = mocker.MagicMock() mv.source_list = mocker.MagicMock() - mv.source_list.get_current_source = mocker.MagicMock(return_value=factory.Source()) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=factory.Source()) mv.controller = mocker.MagicMock(is_authenticated=True) mocker.patch('securedrop_client.gui.widgets.source_exists', return_value=True) scw = mocker.MagicMock() @@ -553,7 +594,7 @@ def test_MainView_on_source_changed(mocker): mv.on_source_changed() - mv.source_list.get_current_source.assert_called_once_with() + mv.source_list.get_selected_source.assert_called_once_with() mv.set_conversation.assert_called_once_with(scw) @@ -564,11 +605,11 @@ def test_MainView_on_source_changed_when_source_no_longer_exists(mocker): mv = MainView(None) mv.set_conversation = mocker.MagicMock() mv.source_list = mocker.MagicMock() - mv.source_list.get_current_source = mocker.MagicMock(return_value=None) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=None) mv.on_source_changed() - mv.source_list.get_current_source.assert_called_once_with() + mv.source_list.get_selected_source.assert_called_once_with() mv.set_conversation.assert_not_called() @@ -588,7 +629,7 @@ def test_MainView_on_source_changed_updates_conversation_view(mocker, session): r = factory.Reply(source=s, filename='0-mock-reply.gpg') session.add(r) session.commit() - mv.source_list.get_current_source = mocker.MagicMock(return_value=s) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=s) add_message_fn = mocker.patch( 'securedrop_client.gui.widgets.ConversationView.add_message', new=mocker.Mock()) add_reply_fn = mocker.patch( @@ -624,7 +665,7 @@ def test_MainView_on_source_changed_SourceConversationWrapper_is_preserved(mocke return_value=None) # We expect on the first call, SourceConversationWrapper.__init__ should be called. - mv.source_list.get_current_source = mocker.MagicMock(return_value=source) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=source) mv.on_source_changed() assert mv.set_conversation.call_count == 1 assert source_conversation_init.call_count == 1 @@ -636,7 +677,7 @@ def test_MainView_on_source_changed_SourceConversationWrapper_is_preserved(mocke # Now click on another source (source2). Since this is the first time we have clicked # on source2, we expect on the first call, SourceConversationWrapper.__init__ should be # called. - mv.source_list.get_current_source = mocker.MagicMock(return_value=source2) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=source2) mv.on_source_changed() assert mv.set_conversation.call_count == 1 assert source_conversation_init.call_count == 1 @@ -647,7 +688,7 @@ def test_MainView_on_source_changed_SourceConversationWrapper_is_preserved(mocke # But if we click back (call on_source_changed again) to the source, # its SourceConversationWrapper should _not_ be recreated. - mv.source_list.get_current_source = mocker.MagicMock(return_value=source) + mv.source_list.get_selected_source = mocker.MagicMock(return_value=source) conversation_wrapper = mv.source_conversations[source.uuid] conversation_wrapper.conversation_view = mocker.MagicMock() conversation_wrapper.conversation_view.update_conversation = mocker.MagicMock() @@ -694,23 +735,20 @@ def test_EmptyConversationView_show_no_source_selected_message(mocker): assert not ecv.no_source_selected.isHidden() -def test_SourceList_get_current_source(mocker): +def test_SourceList_get_selected_source(mocker): """ Maintains the selected item if present in new list """ sl = SourceList() sl.controller = mocker.MagicMock() sources = [factory.Source(), factory.Source()] - for s in sources: - new_source = SourceWidget(sl.controller, s) - sl.source_widgets[s.uuid] = new_source - list_item = QListWidgetItem(sl) - list_item.setSizeHint(new_source.sizeHint()) - sl.addItem(list_item) - sl.setItemWidget(list_item, new_source) + sl.update(sources) + + assert sl.get_selected_source() is None + sl.setCurrentItem(sl.itemAt(1, 0)) # select source2 - current_source = sl.get_current_source() + current_source = sl.get_selected_source() assert current_source.id == sources[1].id @@ -722,6 +760,39 @@ def test_SourceList_update_adds_new_sources(mocker): """ sl = SourceList() + sl.clear = mocker.MagicMock() + sl.insertItem = mocker.MagicMock() + sl.takeItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.controller = mocker.MagicMock() + sl.setCurrentItem = mocker.MagicMock() + + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) + + sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] + sl.update(sources) + + assert mock_sw.call_count == len(sources) + assert mock_lwi.call_count == len(sources) + assert sl.insertItem.call_count == len(sources) + assert sl.setItemWidget.call_count == len(sources) + assert len(sl.source_widgets) == len(sources) + assert sl.setCurrentItem.call_count == 0 + sl.clear.assert_not_called() + sl.takeItem.assert_not_called() + mock_sw.deleteLater.assert_not_called() + + +def test_SourceList_initial_update_adds_new_sources(mocker): + """ + Check a new SourceWidget for each passed-in source is created and no widgets are cleared or + removed. + """ + sl = SourceList() + sl.clear = mocker.MagicMock() sl.add_source = mocker.MagicMock() sl.setItemWidget = mocker.MagicMock() @@ -729,9 +800,48 @@ def test_SourceList_update_adds_new_sources(mocker): sl.item = mocker.MagicMock() sl.item().isSelected.return_value = True sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] - sl.update(sources) - sl.clear.assert_called_once_with() - sl.add_source.assert_called_once_with({}, sources, None, True) + sl.initial_update(sources) + sl.add_source.assert_called_once_with(sources) + + +def test_SourceList_update_when_source_deleted(mocker, session, session_maker, homedir): + """ + Test that SourceWidget.update raises an exception when its source has been deleted. + + When SourceList.update calls SourceWidget.update and that + SourceWidget's source has been deleted, SourceList.update should + catch the resulting excpetion, delete the SourceWidget and add its + source UUID to the list of deleted source UUIDs. + """ + mock_gui = mocker.MagicMock() + controller = logic.Controller('http://localhost', mock_gui, session_maker, homedir) + + # create the source in another session + source = factory.Source() + session.add(source) + session.commit() + + # construct the SourceWidget with the source fetched in its + # controller's session + oss = controller.session.query(db.Source).filter_by(id=source.id).one() + + # add it to the SourceList + sl = SourceList() + sl.setup(controller) + deleted_uuids = sl.update([oss]) + assert not deleted_uuids + assert len(sl.source_widgets) == 1 + + # now delete it + session.delete(source) + session.commit() + + # and finally verify that updating raises an exception, causing + # the SourceWidget to be deleted + deleted_uuids = sl.update([source]) + assert len(deleted_uuids) == 1 + assert source.uuid in deleted_uuids + assert len(sl.source_widgets) == 0 def test_SourceList_add_source_starts_timer(mocker, session_maker, homedir): @@ -743,10 +853,149 @@ def test_SourceList_add_source_starts_timer(mocker, session_maker, homedir): sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] mock_timer = mocker.MagicMock() with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): - sl.add_source({}, sources, None, False) + sl.add_source(sources) assert mock_timer.singleShot.call_count == 1 +def test_SourceList_update_when_source_deleted_crash(mocker, session, session_maker, homedir): + """ + When SourceList.update calls SourceWidget.update and that + SourceWidget has been deleted from the dict on the SourceList, + it should handle the exception and delete the list widget. + """ + mock_gui = mocker.MagicMock() + controller = logic.Controller('http://localhost', mock_gui, session_maker, homedir) + + # create the source in another session + source = factory.Source() + session.add(source) + session.commit() + + # construct the SourceWidget with the source fetched in its + # controller's session + oss = controller.session.query(db.Source).filter_by(id=source.id).one() + + # add it to the SourceList + sl = SourceList() + sl.setup(controller) + deleted_uuids = sl.update([oss]) + assert not deleted_uuids + assert len(sl.source_widgets) == 1 + assert sl.count() == 1 + + # Remove source_widget from dict + sl.source_widgets.pop(oss.uuid) + + # now delete it + session.delete(source) + session.commit() + + # and finally verify that updating does not throw an exception, and + # all widgets are removed from the list view. + deleted_uuids = sl.update([]) + assert sl.count() == 0 + + +def test_SourceList_update_maintains_selection(mocker): + """ + Maintains the selected item if present in new list + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sources = [factory.Source(), factory.Source()] + sl.update(sources) + + sl.setCurrentItem(sl.itemAt(0, 0)) # select the first source + sl.update(sources) + + assert sl.currentItem() + assert sl.itemWidget(sl.currentItem()).source.id == sources[0].id + + sl.setCurrentItem(sl.itemAt(1, 0)) # select the second source + sl.update(sources) + + assert sl.currentItem() + assert sl.itemWidget(sl.currentItem()).source.id == sources[1].id + + +def test_SourceList_update_with_pre_selected_source_maintains_selection(mocker): + """ + Check that an existing source widget that is selected remains selected. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.update([factory.Source(), factory.Source()]) + second_item = sl.itemAt(1, 0) + sl.setCurrentItem(second_item) # select the second source + mocker.patch.object(second_item, 'isSelected', return_value=True) + + sl.update([factory.Source(), factory.Source()]) + + assert second_item.isSelected() is True + + +def test_SourceList_update_removes_selected_item_results_in_no_current_selection(mocker): + """ + Check that no items are currently selected if the currently selected item is deleted. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) + + sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' + sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' + + assert not sl.currentItem() + + +def test_SourceList_update_removes_item_from_end_of_list(mocker): + """ + Check that the item is removed from the source list and dict if the source no longer exists. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.update([ + factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) + assert sl.count() == 3 + sl.update([factory.Source(uuid='newer'), factory.Source(uuid='newest')]) + assert sl.count() == 2 + assert sl.itemWidget(sl.item(0)).source.uuid == 'newest' + assert sl.itemWidget(sl.item(1)).source.uuid == 'newer' + assert len(sl.source_widgets) == 2 + + +def test_SourceList_update_removes_item_from_middle_of_list(mocker): + """ + Check that the item is removed from the source list and dict if the source no longer exists. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.update([ + factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) + assert sl.count() == 3 + sl.update([factory.Source(uuid='new'), factory.Source(uuid='newest')]) + assert sl.count() == 2 + assert sl.itemWidget(sl.item(0)).source.uuid == 'newest' + assert sl.itemWidget(sl.item(1)).source.uuid == 'new' + assert len(sl.source_widgets) == 2 + + +def test_SourceList_update_removes_item_from_beginning_of_list(mocker): + """ + Check that the item is removed from the source list and dict if the source no longer exists. + """ + sl = SourceList() + sl.controller = mocker.MagicMock() + sl.update([ + factory.Source(uuid='new'), factory.Source(uuid='newer'), factory.Source(uuid='newest')]) + assert sl.count() == 3 + sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) + assert sl.count() == 2 + assert sl.itemWidget(sl.item(0)).source.uuid == 'newer' + assert sl.itemWidget(sl.item(1)).source.uuid == 'new' + assert len(sl.source_widgets) == 2 + + def test_SourceList_add_source_closure_adds_sources(mocker): """ The closure (function created within the add_source function) behaves in @@ -765,7 +1014,7 @@ def test_SourceList_add_source_closure_adds_sources(mocker): sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ] mock_timer = mocker.MagicMock() with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): - sl.add_source({}, sources, 1, False) + sl.add_source(sources, 1) # Now grab the function created within add_source: inner_fn = mock_timer.singleShot.call_args_list[0][0][1] # Ensure add_source is mocked to avoid recursion in the test and so we @@ -779,14 +1028,13 @@ def test_SourceList_add_source_closure_adds_sources(mocker): assert sl.setItemWidget.call_count == 1 assert len(sl.source_widgets) == 1 assert sl.setCurrentItem.call_count == 0 - sl.add_source.assert_called_once_with({}, sources[1:], 1, False, 2) + sl.add_source.assert_called_once_with(sources[1:], 2) -def test_SourceList_add_source_closure_sets_current_item(mocker): +def test_SourceList_add_source_closure_exits_on_no_more_sources(mocker): """ - The closure (function created within the add_source function) ensures that - if an item being added to the UI is the currently selected item and is - already selected in the UI, it is marked as such. + The closure (function created within the add_source function) behaves in + the expected way given the context of the call to add_source. """ sl = SourceList() sl.controller = mocker.MagicMock() @@ -798,84 +1046,70 @@ def test_SourceList_add_source_closure_sets_current_item(mocker): mock_lwi = mocker.MagicMock() mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw) mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi) - mock_source = mocker.MagicMock() - mock_source.id = 1 - sources = [mock_source, mocker.MagicMock(), mocker.MagicMock(), ] + sources = [] mock_timer = mocker.MagicMock() with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): - sl.add_source({}, sources, 1, True) + sl.add_source(sources, 1) # Now grab the function created within add_source: inner_fn = mock_timer.singleShot.call_args_list[0][0][1] # Ensure add_source is mocked to avoid recursion in the test and so we # can spy on how it's called. sl.add_source = mocker.MagicMock() # Call the inner function (as if the timer had completed). - inner_fn() - assert sl.setCurrentItem.call_count == 1 + assert inner_fn() is None + assert mock_sw.call_count == 0 + assert mock_lwi.call_count == 0 + assert sl.addItem.call_count == 0 + assert sl.setItemWidget.call_count == 0 + assert len(sl.source_widgets) == 0 + assert sl.setCurrentItem.call_count == 0 + assert sl.add_source.call_count == 0 -def test_SourceList_add_source_closure_deletes_sources_when_finished(mocker): +def test_SourceList_set_snippet(mocker): """ - The closure (function created within the add_source function) ensures that - if an item being added to the UI is the currently selected item and is - already selected in the UI, it is marked as such. + Handle the emitted event in the expected manner. """ sl = SourceList() - sl.controller = mocker.MagicMock() - sl.addItem = mocker.MagicMock() - sl.setItemWidget = mocker.MagicMock() - sl.setCurrentItem = mocker.MagicMock() - - sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' - sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' - existing_sources = { - "uuid1": mocker.MagicMock() - } - + mock_widget = mocker.MagicMock() sl.source_widgets = { - "uuid2": mocker.MagicMock() + "a_uuid": mock_widget, } + sl.set_snippet("a_uuid", "msg_uuid", "msg_content") + mock_widget.set_snippet.assert_called_once_with("a_uuid", "msg_content") - sl.delete_source_by_uuid = mocker.MagicMock() - mock_timer = mocker.MagicMock() - with mocker.patch("securedrop_client.gui.widgets.QTimer", mock_timer): - sl.add_source(existing_sources, [], 1, True) - # Now grab the function created within add_source: - inner_fn = mock_timer.singleShot.call_args_list[0][0][1] - # Ensure add_source is mocked to avoid recursion in the test and so we - # can spy on how it's called. - sl.add_source = mocker.MagicMock() - # Call the inner function (as if the timer had completed). - inner_fn() - sl.delete_source_by_uuid.emit.assert_called_once_with("uuid1") +def test_SourceList_get_source_widget(mocker): + sl = SourceList() + sl.controller = mocker.MagicMock() + mock_source = factory.Source(uuid='mock_uuid') + sl.update([mock_source]) + sl.source_widgets = {} + source_widget = sl.get_source_widget('mock_uuid') -def test_SourceList_update_removes_selected_item_results_in_no_current_selection(mocker): - """ - Check that no items are currently selected if the currently selected item is deleted. - """ + assert source_widget.source_uuid == 'mock_uuid' + assert source_widget == sl.itemWidget(sl.item(0)) + + +def test_SourceList_get_source_widget_does_not_exist(mocker): sl = SourceList() sl.controller = mocker.MagicMock() - sl.update([factory.Source(uuid='new'), factory.Source(uuid='newer')]) + mock_source = factory.Source(uuid='mock_uuid') + sl.update([mock_source]) + sl.source_widgets = {} - sl.setCurrentItem(sl.itemAt(0, 0)) # select source with uuid='newer' - sl.update([factory.Source(uuid='new')]) # delete source with uuid='newer' + source_widget = sl.get_source_widget('uuid_for_source_not_in_list') - assert not sl.currentItem() + assert source_widget is None -def test_SourceList_set_snippet(mocker): - """ - Handle the emitted event in the expected manner. - """ +def test_SourceList_get_source_widget_if_one_exists_in_cache(mocker): sl = SourceList() - mock_widget = mocker.MagicMock() - sl.source_widgets = { - "a_uuid": mock_widget, - } - sl.set_snippet("a_uuid", "msg_uuid", "msg_content") - mock_widget.set_snippet.assert_called_once_with("a_uuid", "msg_uuid", "msg_content") + mock_source_widget = factory.Source(uuid='mock_uuid') + sl.source_widgets = {'mock_uuid': mock_source_widget} + source_widget = sl.get_source_widget('mock_uuid') + assert source_widget == mock_source_widget def test_SourceWidget_init(mocker): @@ -945,19 +1179,32 @@ def test_SourceWidget_update_raises_InvalidRequestError(mocker): assert mock_logger.error.call_count == 1 -def test_SourceWidget_set_snippet(mocker): +def test_SourceWidget_set_snippet(mocker, session_maker, session, homedir): """ Snippets are set as expected. """ - controller = mocker.MagicMock() - source = mocker.MagicMock() - source.uuid = "a_uuid" - source.journalist_designation = "Testy McTestface" - msg = factory.Message(content="abcdefg") - source.collection = [msg, ] + mock_gui = mocker.MagicMock() + controller = logic.Controller('http://localhost', mock_gui, session_maker, homedir) + source = factory.Source(document_count=1) + f = factory.File(source=source) + session.add(f) + session.add(source) + session.commit() + sw = SourceWidget(controller, source) - sw.set_snippet(source.uuid, msg.uuid, msg.content) - assert sw.preview.text() == "abcdefg" + sw.set_snippet(source.uuid, f.filename) + assert sw.preview.text() == f.filename + + # check when a different source is specified + sw.set_snippet("not-the-source-uuid", "something new") + assert sw.preview.text() == f.filename + + source_uuid = source.uuid + session.delete(source) + session.commit() + + # check when the source has been deleted that it catches sqlalchemy.exc.InvalidRequestError + sw.set_snippet(source_uuid, "something new") def test_SourceWidget_update_truncate_latest_msg(mocker): @@ -1034,6 +1281,32 @@ def test_SourceWidget__on_source_deleted_wrong_uuid(mocker, session, source): assert sw.waiting_delete_confirmation.isHidden() +def test_SourceWidget__on_source_deletion_failed(mocker, session, source): + controller = mocker.MagicMock() + sw = SourceWidget(controller, factory.Source(uuid='123')) + sw._on_source_deleted('123') + + sw._on_source_deletion_failed('123') + + assert not sw.gutter.isHidden() + assert not sw.metadata.isHidden() + assert not sw.preview.isHidden() + assert sw.waiting_delete_confirmation.isHidden() + + +def test_SourceWidget__on_source_deletion_failed_wrong_uuid(mocker, session, source): + controller = mocker.MagicMock() + sw = SourceWidget(controller, factory.Source(uuid='123')) + sw._on_source_deleted('123') + + sw._on_source_deletion_failed('321') + + assert sw.gutter.isHidden() + assert sw.metadata.isHidden() + assert sw.preview.isHidden() + assert not sw.waiting_delete_confirmation.isHidden() + + def test_SourceWidget_uses_SecureQLabel(mocker): """ Ensure the source widget preview uses SecureQLabel and is not injectable @@ -1055,65 +1328,158 @@ def test_SourceWidget_uses_SecureQLabel(mocker): def test_StarToggleButton_init_source_starred(mocker): controller = mocker.MagicMock() - source = factory.Source() - source.is_starred = True + source = factory.Source(is_starred=True) - stb = StarToggleButton(controller, source) + stb = StarToggleButton(controller, source.uuid, source.is_starred) - assert stb.source == source + assert stb.source_uuid == source.uuid assert stb.isChecked() is True def test_StarToggleButton_init_source_unstarred(mocker): controller = mocker.MagicMock() - source = factory.Source() - source.is_starred = False + source = factory.Source(is_starred=False) - stb = StarToggleButton(controller, source) + stb = StarToggleButton(controller, source.uuid, source.is_starred) - assert stb.source == source + assert stb.source_uuid == source.uuid assert stb.isChecked() is False -def test_StarToggleButton_eventFilter(mocker): +def test_StarToggleButton_eventFilter_when_checked(mocker): """ - Ensure the hover events are handled properly. + Ensure the hover events are handled properly when star is checked and online. """ controller = mocker.MagicMock() - stb = StarToggleButton(controller=controller, source=mocker.MagicMock()) + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', True) + stb.pressed = mocker.MagicMock() stb.setIcon = mocker.MagicMock() stb.set_icon = mocker.MagicMock() + load_icon_fn = mocker.patch("securedrop_client.gui.widgets.load_icon") + # Hover enter test_event = QEvent(QEvent.HoverEnter) stb.eventFilter(stb, test_event) assert stb.setIcon.call_count == 1 + load_icon_fn.assert_called_once_with('star_hover.svg') + # Hover leave test_event = QEvent(QEvent.HoverLeave) stb.eventFilter(stb, test_event) stb.set_icon.assert_called_once_with(on='star_on.svg', off='star_off.svg') - # Hover leave when disabled - stb.disable() + # Authentication change + stb.on_authentication_changed(authenticated=True) + assert stb.isCheckable() is True + stb.pressed.disconnect.assert_called_once_with() + stb.pressed.connect.assert_called_once_with(stb.on_pressed) + + +def test_StarToggleButton_eventFilter_when_not_checked(mocker): + """ + Ensure the hover events are handled properly when star is checked and online. + """ + controller = mocker.MagicMock() + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', False) + stb.pressed = mocker.MagicMock() + stb.setIcon = mocker.MagicMock() + stb.set_icon = mocker.MagicMock() + load_icon_fn = mocker.patch("securedrop_client.gui.widgets.load_icon") + + # Hover enter + test_event = QEvent(QEvent.HoverEnter) + stb.eventFilter(stb, test_event) + assert stb.setIcon.call_count == 1 + load_icon_fn.assert_called_once_with('star_hover.svg') + + # Hover leave test_event = QEvent(QEvent.HoverLeave) stb.eventFilter(stb, test_event) + stb.set_icon.assert_called_once_with(on='star_on.svg', off='star_off.svg') + + # Authentication change + stb.on_authentication_changed(authenticated=True) + assert stb.isCheckable() is True + stb.pressed.disconnect.assert_called_once_with() + stb.pressed.connect.assert_called_once_with(stb.on_pressed) + + +def test_StarToggleButton_eventFilter_when_checked_and_offline(mocker): + """ + Ensure the hover events do not change the icon when offline and that the star icon is set to + off='star_on.svg' when checked and offline. + """ + controller = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', True) + stb.pressed = mocker.MagicMock() + stb.setIcon = mocker.MagicMock() + stb.set_icon = mocker.MagicMock() + load_icon_fn = mocker.patch("securedrop_client.gui.widgets.load_icon") + + # Authentication change + stb.on_authentication_changed(authenticated=False) + assert stb.isCheckable() is False stb.set_icon.assert_called_with(on='star_on.svg', off='star_on.svg') + stb.pressed.disconnect.assert_called_once_with() + stb.pressed.connect.assert_called_once_with(stb.on_pressed_offline) + + # Hover enter + test_event = QEvent(QEvent.HoverEnter) + stb.eventFilter(stb, test_event) + stb.setIcon.assert_not_called() + load_icon_fn.assert_not_called() + + # Hover leave + test_event = QEvent(QEvent.HoverLeave) + stb.eventFilter(stb, test_event) + stb.setIcon.assert_not_called() + + +def test_StarToggleButton_eventFilter_when_not_checked_and_offline(mocker): + """ + Ensure the hover events do not change the icon when offline and that the star icon is set to + off='star_on.svg' when unchecked and offline. + """ + controller = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', False) + stb.pressed = mocker.MagicMock() + stb.setIcon = mocker.MagicMock() + stb.set_icon = mocker.MagicMock() + load_icon_fn = mocker.patch("securedrop_client.gui.widgets.load_icon") + + # Authentication change + stb.on_authentication_changed(authenticated=False) + assert stb.isCheckable() is False + stb.pressed.disconnect.assert_called_once_with() + stb.pressed.connect.assert_called_once_with(stb.on_pressed_offline) + + # Hover enter + test_event = QEvent(QEvent.HoverEnter) + stb.eventFilter(stb, test_event) + stb.setIcon.assert_not_called() + load_icon_fn.assert_not_called() + + # Hover leave + test_event = QEvent(QEvent.HoverLeave) + stb.eventFilter(stb, test_event) + stb.setIcon.assert_not_called() def test_StarToggleButton_on_authentication_changed_while_authenticated_and_checked(mocker): """ - If on_authentication_changed is set up correctly, then calling toggle on a checked button should - result in the button being unchecked. + If on_authentication_changed is set up correctly, then toggling a checked button should result + in the button being unchecked. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - stb = StarToggleButton(controller, source=source) - stb.setChecked(True) - stb.on_toggle = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', True) + stb.on_pressed = mocker.MagicMock() stb.on_authentication_changed(authenticated=True) - stb.toggle() + stb.click() - assert stb.on_toggle.called is True + stb.on_pressed.assert_called_once_with() assert stb.isChecked() is False @@ -1123,97 +1489,231 @@ def test_StarToggleButton_on_authentication_changed_while_authenticated_and_not_ should result in the button being unchecked. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - source.is_starred = False - stb = StarToggleButton(controller, source=source) - stb.setChecked(False) - stb.on_toggle = mocker.MagicMock() - assert stb.isChecked() is False - + stb = StarToggleButton(controller, 'mock_uuid', False) + stb.on_pressed = mocker.MagicMock() stb.on_authentication_changed(authenticated=True) - assert stb.isChecked() is False - stb.toggle() + stb.click() - assert stb.on_toggle.called is True + stb.on_pressed.assert_called_once_with() assert stb.isChecked() is True -def test_StarToggleButton_on_authentication_changed_while_offline_mode(mocker): +def test_StarToggleButton_on_authentication_changed_while_offline_mode_and_not_checked(mocker): """ Ensure on_authentication_changed is set up correctly for offline mode. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - stb = StarToggleButton(controller, source=source) - stb.on_toggle_offline = mocker.MagicMock() - stb.on_toggle = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', False) + stb.on_pressed_offline = mocker.MagicMock() + stb.on_pressed = mocker.MagicMock() + stb.on_authentication_changed(authenticated=False) + + stb.click() + + stb.on_pressed_offline.assert_called_once_with() + stb.on_pressed.assert_not_called() + assert stb.isChecked() is False + +def test_StarToggleButton_on_authentication_changed_while_offline_mode_and_checked(mocker): + """ + Ensure on_authentication_changed is set up correctly for offline mode. + """ + controller = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', True) + stb.on_pressed_offline = mocker.MagicMock() + stb.on_pressed = mocker.MagicMock() stb.on_authentication_changed(authenticated=False) + stb.click() - assert stb.on_toggle_offline.called is True - assert stb.on_toggle.called is False + stb.on_pressed_offline.assert_called_once_with() + stb.on_pressed.assert_not_called() + assert stb.isCheckable() is False + assert stb.is_starred is True -def test_StarToggleButton_on_toggle(mocker): +def test_StarToggleButton_on_pressed_toggles_to_starred(mocker): """ - Ensure correct star icon images are loaded for the enabled button. + Ensure pressing the star button toggles from unstarred to starred. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - stb = StarToggleButton(controller, source) + stb = StarToggleButton(controller, 'mock_uuid', False) + + stb.click() + + stb.controller.update_star.assert_called_once_with('mock_uuid', False) + assert stb.isChecked() + + +def test_StarToggleButton_on_pressed_toggles_to_unstarred(mocker): + """ + Ensure pressing the star button toggles from starred to unstarred. + """ + controller = mocker.MagicMock() + stb = StarToggleButton(controller, 'mock_uuid', True) - stb.on_toggle() + stb.click() - stb.controller.update_star.assert_called_once_with(source, stb.on_update) + stb.controller.update_star.assert_called_once_with('mock_uuid', True) + assert not stb.isChecked() -def test_StarToggleButton_on_toggle_offline(mocker): +def test_StarToggleButton_on_pressed_offline(mocker): """ Ensure toggle is disabled when offline. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - stb = StarToggleButton(controller, source) + controller.is_authenticated = False + stb = StarToggleButton(controller, 'mock_uuid', True) + + stb.click() - stb.on_toggle_offline() stb.controller.on_action_requiring_login.assert_called_once_with() -def test_StarToggleButton_on_toggle_offline_when_checked(mocker): +def test_StarToggleButton_on_pressed_offline_when_checked(mocker): """ Ensure correct star icon images are loaded for the disabled button. """ controller = mocker.MagicMock() - source = mocker.MagicMock() - source.is_starred = True - stb = StarToggleButton(controller, source) + controller.is_authenticated = False + source = factory.Source(is_starred=True) + stb = StarToggleButton(controller, source.uuid, source.is_starred) set_icon_fn = mocker.patch('securedrop_client.gui.SvgToggleButton.set_icon') - # go offline stb.on_authentication_changed(False) assert stb.isCheckable() is False set_icon_fn.assert_called_with(on='star_on.svg', off='star_on.svg') - stb.on_toggle_offline() + stb.click() stb.controller.on_action_requiring_login.assert_called_once_with() -def test_StarToggleButton_on_update(mocker): +def test_StarToggleButton_update(mocker): """ - Ensure the on_update callback updates the state of the source and UI - element to the current "enabled" state. + Ensure update syncs the star state with the server if there are no pending jobs and we're not + waiting until the next sync (in order to avoid the "ghost" issue where update is called with an + outdated state between a star job finishing and a sync). """ controller = mocker.MagicMock() - source = mocker.MagicMock() - source.is_starred = True - stb = StarToggleButton(controller, source) - stb.setChecked = mocker.MagicMock() - stb.on_update(("uuid", False)) - assert source.is_starred is False - stb.controller.update_sources.assert_called_once_with() - stb.setChecked.assert_called_once_with(False) + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', True) + + # Should not change because we wait until next sync + stb.pending_count = 0 + stb.wait_until_next_sync = True + stb.update(False) + assert stb.isChecked() is True + stb.update(True) + assert stb.isChecked() is True + + # Should update to match value provided by update because there are no pending star jobs and + # wait_until_next_sync is False, meaning a sync already occurred after the star job finished + stb.setChecked(True) + stb.pending_count = 0 + stb.wait_until_next_sync = False + stb.update(False) + assert stb.isChecked() is False + stb.update(True) + assert stb.isChecked() is True + + # Should not change because there are pending star jobs + stb.setChecked(True) + stb.pending_count = 1 + stb.wait_until_next_sync = True + stb.update(False) + assert stb.isChecked() is True + stb.update(True) + assert stb.isChecked() is True + # Still should not change because there are pending star jobs + stb.wait_until_next_sync = False + stb.update(False) + assert stb.isChecked() is True + stb.update(True) + assert stb.isChecked() is True + + +def test_StarToggleButton_update_when_not_authenticated(mocker): + """ + Ensure the button state does not change if the user is not authenticated. + """ + controller = mocker.MagicMock() + controller.is_authenticated = False + source = factory.Source(is_starred=True) + stb = StarToggleButton(controller, source.uuid, source.is_starred) + + # Button stays checked + stb.update(False) + assert stb.is_starred is True + + # Button stays unchecked + stb.is_starred = False + stb.update(True) + assert stb.is_starred is False + + +def test_StarToggleButton_on_star_update_failed(mocker): + ''' + Ensure the button is toggled to the state provided in the failure handler and that the pending + count is decremented if the source uuid matches. + ''' + controller = mocker.MagicMock() + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', False) + + stb.click() + assert stb.is_starred is True + assert stb.pending_count == 1 + stb.on_star_update_failed('mock_uuid', is_starred=False) + assert stb.is_starred is False + assert stb.pending_count == 0 + + +def test_StarToggleButton_on_star_update_failed_for_non_matching_source_uuid(mocker): + ''' + Ensure the button is not toggled and that the pending count stays the same if the source uuid + does not match. + ''' + controller = mocker.MagicMock() + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', False) + + stb.click() + assert stb.is_starred is True + assert stb.pending_count == 1 + stb.on_star_update_failed('some_other_uuid', is_starred=False) + assert stb.is_starred is True + assert stb.pending_count == 1 + + +def test_StarToggleButton_on_star_update_successful(mocker): + ''' + Ensure that the pending count is decremented if the source uuid matches. + ''' + controller = mocker.MagicMock() + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', True) + + stb.click() + assert stb.pending_count == 1 + stb.on_star_update_successful('mock_uuid') + assert stb.pending_count == 0 + + +def test_StarToggleButton_on_star_update_successful_for_non_matching_source_uuid(mocker): + ''' + Ensure that the pending count is not decremented if the source uuid does not match. + ''' + controller = mocker.MagicMock() + controller.is_authenticated = True + stb = StarToggleButton(controller, 'mock_uuid', True) + + stb.click() + assert stb.pending_count == 1 + stb.on_star_update_successful('some_other_uuid') + assert stb.pending_count == 1 def test_LoginDialog_setup(mocker, i18n): @@ -1732,6 +2232,31 @@ def test_FileWidget_on_left_click_download(mocker, session, source): db.File, file_.uuid) +def test_FileWidget_on_left_click_downloading_in_progress(mocker, session, source): + """ + Left click on download when file is not downloaded but is in progress + downloading should not trigger a download. + """ + file_ = factory.File(source=source['source'], + is_downloaded=False, + is_decrypted=None) + session.add(file_) + session.commit() + + mock_get_file = mocker.MagicMock(return_value=file_) + mock_controller = mocker.MagicMock(get_file=mock_get_file) + + fw = FileWidget(file_.uuid, mock_controller, mocker.MagicMock(), mocker.MagicMock(), 0) + fw.downloading = True + fw.download_button = mocker.MagicMock() + mock_get_file.assert_called_once_with(file_.uuid) + mock_get_file.reset_mock() + + fw._on_left_click() + mock_get_file.call_count == 0 + mock_controller.on_submission_download.call_count == 0 + + def test_FileWidget_start_button_animation(mocker, session, source): """ Ensure widget state is updated when this method is called. @@ -2009,36 +2534,29 @@ def test_FileWidget__on_print_clicked_missing_file(mocker, session, source): dialog.assert_not_called() -def test_FramelessDialog_closeEvent(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() - dialog.internal_close_event_emitted = True - close_event = QEvent(QEvent.Close) - close_event.ignore = mocker.MagicMock() - - dialog.closeEvent(close_event) - - close_event.ignore.assert_not_called() - +def test_FileWidget_update_file_size_with_deleted_file( + mocker, homedir, config, session_maker, source +): + mock_gui = mocker.MagicMock() + controller = logic.Controller('http://localhost', mock_gui, session_maker, homedir) -def test_FramelessDialog_closeEvent_ignored_if_not_a_close_event_from_custom_close_buttons(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() - close_event = QEvent(QEvent.Close) - close_event.ignore = mocker.MagicMock() + file = factory.File(source=source['source'], is_downloaded=True) + controller.session.add(file) + controller.session.commit() - dialog.closeEvent(close_event) + fw = FileWidget(file.uuid, controller, mocker.MagicMock(), mocker.MagicMock(), 0) - close_event.ignore.assert_called_once_with() + with mocker.patch( + "securedrop_client.gui.widgets.humanize_filesize", + side_effect=Exception("boom!") + ): + fw.update_file_size() + assert fw.file_size.text() == "" @pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return]) -def test_FramelessDialog_keyPressEvent_does_not_close_on_enter_or_return(mocker, key): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() +def test_ModalDialog_keyPressEvent_does_not_close_on_enter_or_return(mocker, key): + dialog = ModalDialog() dialog.close = mocker.MagicMock() event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier) @@ -2048,10 +2566,8 @@ def test_FramelessDialog_keyPressEvent_does_not_close_on_enter_or_return(mocker, @pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return]) -def test_FramelessDialog_keyPressEvent_cancel_on_enter_when_focused(mocker, key): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() +def test_ModalDialog_keyPressEvent_cancel_on_enter_when_focused(mocker, key): + dialog = ModalDialog() dialog.cancel_button.click = mocker.MagicMock() dialog.cancel_button.hasFocus = mocker.MagicMock(return_value=True) @@ -2062,10 +2578,8 @@ def test_FramelessDialog_keyPressEvent_cancel_on_enter_when_focused(mocker, key) @pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return]) -def test_FramelessDialog_keyPressEvent_continue_on_enter(mocker, key): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() +def test_ModalDialog_keyPressEvent_continue_on_enter(mocker, key): + dialog = ModalDialog() dialog.continue_button.click = mocker.MagicMock() event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier) @@ -2075,10 +2589,8 @@ def test_FramelessDialog_keyPressEvent_continue_on_enter(mocker, key): @pytest.mark.parametrize("key", [Qt.Key_Alt, Qt.Key_A]) -def test_FramelessDialog_keyPressEvent_does_not_close_for_other_keys(mocker, key): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() +def test_ModalDialog_keyPressEvent_does_not_close_for_other_keys(mocker, key): + dialog = ModalDialog() dialog.close = mocker.MagicMock() event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier) @@ -2087,41 +2599,65 @@ def test_FramelessDialog_keyPressEvent_does_not_close_for_other_keys(mocker, key dialog.close.assert_not_called() -def test_FramelessDialog_close(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() - - assert dialog.internal_close_event_emitted is False - - dialog.close() - - assert dialog.internal_close_event_emitted is True - - -def test_FramelessDialog_center_dialog(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) - dialog = FramelessDialog() - dialog.move = mocker.MagicMock() - - dialog.center_dialog() - - dialog.move.call_count == 1 - - -def test_FramelessDialog_center_dialog_with_no_active_window(mocker): - dialog = FramelessDialog() - dialog.move = mocker.MagicMock() - - dialog.center_dialog() +def test_ModalDialog_animation_of_activestate(mocker): + dialog = ModalDialog() + assert dialog.button_animation + dialog.button_animation.start = mocker.MagicMock() + dialog.button_animation.stop = mocker.MagicMock() + dialog.continue_button = mocker.MagicMock() - dialog.move.assert_not_called() + # Check the animation frame is updated as expected. + dialog.animate_activestate() + assert dialog.continue_button.setIcon.call_count == 1 + dialog.continue_button.reset_mock() + + # Check starting the animated state works as expected. + dialog.start_animate_activestate() + dialog.button_animation.start.assert_called_once_with() + dialog.continue_button.setText.assert_called_once_with("") + assert dialog.continue_button.setMinimumSize.call_count == 1 + assert dialog.continue_button.setStyleSheet.call_count == 1 + + dialog.continue_button.reset_mock() + + # Check stopping the animated state works as expected. + dialog.stop_animate_activestate() + dialog.button_animation.stop.assert_called_once_with() + dialog.continue_button.setText.assert_called_once_with("CONTINUE") + assert dialog.continue_button.setIcon.call_count == 1 + assert dialog.continue_button.setStyleSheet.call_count == 1 + + +def test_ModalDialog_animation_of_header(mocker): + dialog = ModalDialog() + assert dialog.header_animation + dialog.header_animation.start = mocker.MagicMock() + dialog.header_animation.stop = mocker.MagicMock() + dialog.header_icon.setVisible = mocker.MagicMock() + dialog.header_spinner_label.setVisible = mocker.MagicMock() + dialog.header_spinner_label.setPixmap = mocker.MagicMock() + + # Check the animation frame is updated as expected. + dialog.animate_header() + assert dialog.header_spinner_label.setPixmap.call_count == 1 + + # Check starting the animated state works as expected. + dialog.start_animate_header() + dialog.header_animation.start.assert_called_once_with() + dialog.header_icon.setVisible.assert_called_once_with(False) + dialog.header_spinner_label.setVisible.assert_called_once_with(True) + + dialog.header_icon.setVisible.reset_mock() + dialog.header_spinner_label.setVisible.reset_mock() + + # Check stopping the animated state works as expected. + dialog.stop_animate_header() + dialog.header_animation.stop.assert_called_once_with() + dialog.header_icon.setVisible.assert_called_once_with(True) + dialog.header_spinner_label.setVisible.assert_called_once_with(False) def test_ExportDialog_init(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) _show_starting_instructions_fn = mocker.patch( 'securedrop_client.gui.widgets.ExportDialog._show_starting_instructions') @@ -2132,8 +2668,6 @@ def test_ExportDialog_init(mocker): def test_ExportDialog_init_sanitizes_filename(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) secure_qlabel = mocker.patch('securedrop_client.gui.widgets.SecureQLabel') mocker.patch('securedrop_client.gui.widgets.QVBoxLayout.addWidget') filename = '' @@ -2144,8 +2678,6 @@ def test_ExportDialog_init_sanitizes_filename(mocker): def test_ExportDialog__show_starting_instructions(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_starting_instructions() @@ -2155,18 +2687,18 @@ def test_ExportDialog__show_starting_instructions(mocker): '
' \ 'mock.jpg' assert dialog.body.text() == \ - '

Proceed with caution when exporting files

' \ + '

Understand the risks before exporting files

' \ 'Malware' \ '
' \ - 'This workstation lets you open documents securely. If you open documents on another ' \ + 'This workstation lets you open files securely. If you open files on another ' \ 'computer, any embedded malware may spread to your computer or network. If you are ' \ - 'unsure how to manage this risk, please print the document, or contact your ' \ + 'unsure how to manage this risk, please print the file, or contact your ' \ 'administrator.' \ '

' \ 'Anonymity' \ '
' \ - 'Documents submitted by sources may contain information or hidden metadata that ' \ - 'identifies who they are. To protect your sources, please consider redacting documents ' \ + 'Files submitted by sources may contain information or hidden metadata that ' \ + 'identifies who they are. To protect your sources, please consider redacting files ' \ 'before working with them on network-connected computers.' assert not dialog.header.isHidden() assert not dialog.header_line.isHidden() @@ -2178,8 +2710,6 @@ def test_ExportDialog__show_starting_instructions(mocker): def test_ExportDialog___show_passphrase_request_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_passphrase_request_message() @@ -2195,8 +2725,6 @@ def test_ExportDialog___show_passphrase_request_message(mocker): def test_ExportDialog__show_passphrase_request_message_again(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_passphrase_request_message_again() @@ -2214,8 +2742,6 @@ def test_ExportDialog__show_passphrase_request_message_again(mocker): def test_ExportDialog__show_success_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_success_message() @@ -2233,8 +2759,6 @@ def test_ExportDialog__show_success_message(mocker): def test_ExportDialog__show_insert_usb_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_insert_usb_message() @@ -2253,8 +2777,6 @@ def test_ExportDialog__show_insert_usb_message(mocker): def test_ExportDialog__show_insert_encrypted_usb_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_insert_encrypted_usb_message() @@ -2275,14 +2797,12 @@ def test_ExportDialog__show_insert_encrypted_usb_message(mocker): def test_ExportDialog__show_generic_error_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog.error_status = 'mock_error_status' dialog._show_generic_error_message() - assert dialog.header.text() == 'Unable to export' + assert dialog.header.text() == 'Export failed' assert dialog.body.text() == 'mock_error_status: See your administrator for help.' assert not dialog.header.isHidden() assert not dialog.header_line.isHidden() @@ -2294,8 +2814,6 @@ def test_ExportDialog__show_generic_error_message(mocker): def test_ExportDialog__export_file(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) controller = mocker.MagicMock() controller.export_file_to_usb_drive = mocker.MagicMock() dialog = ExportDialog(controller, 'mock_uuid', 'mock.jpg') @@ -2307,8 +2825,6 @@ def test_ExportDialog__export_file(mocker): def test_ExportDialog__on_preflight_success(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_passphrase_request_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2323,8 +2839,6 @@ def test_ExportDialog__on_preflight_success(mocker): def test_ExportDialog__on_preflight_success_when_continue_enabled(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_passphrase_request_message = mocker.MagicMock() dialog.continue_button.setEnabled(True) @@ -2335,8 +2849,6 @@ def test_ExportDialog__on_preflight_success_when_continue_enabled(mocker): def test_ExportDialog__on_preflight_success_enabled_after_preflight_success(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') assert not dialog.continue_button.isEnabled() dialog._on_preflight_success() @@ -2344,8 +2856,6 @@ def test_ExportDialog__on_preflight_success_enabled_after_preflight_success(mock def test_ExportDialog__on_preflight_success_enabled_after_preflight_failure(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') assert not dialog.continue_button.isEnabled() dialog._on_preflight_failure(mocker.MagicMock()) @@ -2353,8 +2863,6 @@ def test_ExportDialog__on_preflight_success_enabled_after_preflight_failure(mock def test_ExportDialog__on_preflight_failure(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._update_dialog = mocker.MagicMock() @@ -2365,8 +2873,6 @@ def test_ExportDialog__on_preflight_failure(mocker): def test_ExportDialog__on_export_success(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_success_message = mocker.MagicMock() @@ -2376,8 +2882,6 @@ def test_ExportDialog__on_export_success(mocker): def test_ExportDialog__on_export_failure(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._update_dialog = mocker.MagicMock() @@ -2388,8 +2892,6 @@ def test_ExportDialog__on_export_failure(mocker): def test_ExportDialog__update_dialog_when_status_is_USB_NOT_CONNECTED(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_insert_usb_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2407,8 +2909,6 @@ def test_ExportDialog__update_dialog_when_status_is_USB_NOT_CONNECTED(mocker): def test_ExportDialog__update_dialog_when_status_is_BAD_PASSPHRASE(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_passphrase_request_message_again = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2427,8 +2927,6 @@ def test_ExportDialog__update_dialog_when_status_is_BAD_PASSPHRASE(mocker): def test_ExportDialog__update_dialog_when_status_DISK_ENCRYPTION_NOT_SUPPORTED_ERROR(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_insert_encrypted_usb_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2447,8 +2945,6 @@ def test_ExportDialog__update_dialog_when_status_DISK_ENCRYPTION_NOT_SUPPORTED_E def test_ExportDialog__update_dialog_when_status_is_CALLED_PROCESS_ERROR(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_generic_error_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2469,8 +2965,6 @@ def test_ExportDialog__update_dialog_when_status_is_CALLED_PROCESS_ERROR(mocker) def test_ExportDialog__update_dialog_when_status_is_unknown(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = ExportDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_generic_error_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2491,8 +2985,6 @@ def test_ExportDialog__update_dialog_when_status_is_unknown(mocker): def test_PrintDialog_init(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) _show_starting_instructions_fn = mocker.patch( 'securedrop_client.gui.widgets.PrintDialog._show_starting_instructions') @@ -2502,8 +2994,6 @@ def test_PrintDialog_init(mocker): def test_PrintDialog_init_sanitizes_filename(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) secure_qlabel = mocker.patch('securedrop_client.gui.widgets.SecureQLabel') filename = '' @@ -2513,8 +3003,6 @@ def test_PrintDialog_init_sanitizes_filename(mocker): def test_PrintDialog__show_starting_instructions(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._show_starting_instructions() @@ -2525,11 +3013,11 @@ def test_PrintDialog__show_starting_instructions(mocker): 'mock.jpg' assert dialog.body.text() == \ '

Managing printout risks

' \ - 'QR-Codes and visible web addresses' \ + 'QR codes and web addresses' \ '
' \ - 'Never open web addresses or scan QR codes contained in printed documents without ' \ - 'taking security precautions. If you are unsure how to manage this risk, please ' \ - 'contact your administrator.' \ + 'Never type in and open web addresses or scan QR codes contained in printed ' \ + 'documents without taking security precautions. If you are unsure how to ' \ + 'manage this risk, please contact your administrator.' \ '

' \ 'Printer dots' \ '
' \ @@ -2545,13 +3033,11 @@ def test_PrintDialog__show_starting_instructions(mocker): def test_PrintDialog__show_insert_usb_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_insert_usb_message() - assert dialog.header.text() == 'Insert USB printer' + assert dialog.header.text() == 'Connect USB printer' assert dialog.body.text() == 'Please connect your printer to a USB port.' assert not dialog.header.isHidden() assert not dialog.header_line.isHidden() @@ -2562,14 +3048,12 @@ def test_PrintDialog__show_insert_usb_message(mocker): def test_PrintDialog__show_generic_error_message(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog.error_status = 'mock_error_status' dialog._show_generic_error_message() - assert dialog.header.text() == 'Unable to print' + assert dialog.header.text() == 'Printing failed' assert dialog.body.text() == 'mock_error_status: See your administrator for help.' assert not dialog.header.isHidden() assert not dialog.header_line.isHidden() @@ -2580,8 +3064,6 @@ def test_PrintDialog__show_generic_error_message(mocker): def test_PrintDialog__print_file(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog.close = mocker.MagicMock() @@ -2591,8 +3073,6 @@ def test_PrintDialog__print_file(mocker): def test_PrintDialog__on_preflight_success(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._print_file = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2606,8 +3086,6 @@ def test_PrintDialog__on_preflight_success(mocker): def test_PrintDialog__on_preflight_success_when_continue_enabled(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') dialog._print_file = mocker.MagicMock() dialog.continue_button.setEnabled(True) @@ -2618,8 +3096,6 @@ def test_PrintDialog__on_preflight_success_when_continue_enabled(mocker): def test_PrintDialog__on_preflight_success_enabled_after_preflight_success(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') assert not dialog.continue_button.isEnabled() dialog._on_preflight_success() @@ -2627,8 +3103,6 @@ def test_PrintDialog__on_preflight_success_enabled_after_preflight_success(mocke def test_PrintDialog__on_preflight_success_enabled_after_preflight_failure(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock.jpg') assert not dialog.continue_button.isEnabled() dialog._on_preflight_failure(mocker.MagicMock()) @@ -2636,8 +3110,6 @@ def test_PrintDialog__on_preflight_success_enabled_after_preflight_failure(mocke def test_PrintDialog__on_preflight_failure_when_status_is_PRINTER_NOT_FOUND(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_insert_usb_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2655,8 +3127,6 @@ def test_PrintDialog__on_preflight_failure_when_status_is_PRINTER_NOT_FOUND(mock def test_PrintDialog__on_preflight_failure_when_status_is_MISSING_PRINTER_URI(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_generic_error_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2677,8 +3147,6 @@ def test_PrintDialog__on_preflight_failure_when_status_is_MISSING_PRINTER_URI(mo def test_PrintDialog__on_preflight_failure_when_status_is_CALLED_PROCESS_ERROR(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_generic_error_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2699,8 +3167,6 @@ def test_PrintDialog__on_preflight_failure_when_status_is_CALLED_PROCESS_ERROR(m def test_PrintDialog__on_preflight_failure_when_status_is_unknown(mocker): - mocker.patch( - 'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow()) dialog = PrintDialog(mocker.MagicMock(), 'mock_uuid', 'mock_filename') dialog._show_generic_error_message = mocker.MagicMock() dialog.continue_button = mocker.MagicMock() @@ -2738,6 +3204,30 @@ def test_SourceConversationWrapper__on_source_deleted_wrong_uuid(mocker): assert scw.waiting_delete_confirmation.isHidden() +def test_SourceConversationWrapper__on_source_deletion_failed(mocker): + scw = SourceConversationWrapper(factory.Source(uuid='123'), mocker.MagicMock()) + scw._on_source_deleted('123') + + scw._on_source_deletion_failed('123') + + assert not scw.conversation_title_bar.isHidden() + assert not scw.conversation_view.isHidden() + assert not scw.reply_box.isHidden() + assert scw.waiting_delete_confirmation.isHidden() + + +def test_SourceConversationWrapper__on_source_deletion_failed_wrong_uuid(mocker): + scw = SourceConversationWrapper(factory.Source(uuid='123'), mocker.MagicMock()) + scw._on_source_deleted('123') + + scw._on_source_deletion_failed('321') + + assert scw.conversation_title_bar.isHidden() + assert scw.conversation_view.isHidden() + assert scw.reply_box.isHidden() + assert not scw.waiting_delete_confirmation.isHidden() + + def test_ConversationView_init(mocker, homedir): """ Ensure the conversation view has a layout to add widgets to. @@ -3362,7 +3852,7 @@ def test_ReplyBoxWidget_on_sync_source_deleted(mocker, source): controller = mocker.MagicMock() rb = ReplyBoxWidget(s, controller) - error_logger = mocker.patch('securedrop_client.gui.widgets.logger.error') + error_logger = mocker.patch('securedrop_client.gui.widgets.logger.debug') def pretend_source_was_deleted(self): raise sqlalchemy.orm.exc.ObjectDeletedError( @@ -3429,7 +3919,7 @@ def test_ReplyBoxWidget_on_authentication_changed_source_deleted(mocker, source) controller = mocker.MagicMock() rb = ReplyBoxWidget(s, controller) - error_logger = mocker.patch('securedrop_client.gui.widgets.logger.error') + error_logger = mocker.patch('securedrop_client.gui.widgets.logger.debug') def pretend_source_was_deleted(self): raise sqlalchemy.orm.exc.ObjectDeletedError( From ed8d38bd52f7053f03b43926de8b8b994693465c Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Wed, 25 Mar 2020 13:52:45 -0400 Subject: [PATCH 4/6] implementing requested changes from review --- securedrop_client/gui/widgets.py | 6 +----- securedrop_client/logic.py | 2 -- securedrop_client/storage.py | 6 +++--- tests/gui/test_widgets.py | 5 ++--- tests/test_storage.py | 4 +++- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index fb527b8bd..ccbe4d443 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -696,7 +696,6 @@ def setup(self, controller): """ self.controller = controller self.source_list.setup(controller) - self.source_list.delete_source_by_uuid.connect(self.delete_conversation) def show_sources(self, sources: List[Source]): """ @@ -909,8 +908,6 @@ class SourceList(QListWidget): } ''' - delete_source_by_uuid = pyqtSignal(str) - def __init__(self): super().__init__() @@ -989,7 +986,6 @@ def initial_update(self, sources: List[Source]): """ Initialise the list with the passed in list of sources. """ - sources.reverse() self.add_source(sources) def add_source(self, sources, slice_size=1): @@ -1010,7 +1006,7 @@ def schedule_source_management(slice_size=slice_size): list_item = QListWidgetItem(self) list_item.setSizeHint(new_source.sizeHint()) - self.addItem(list_item) + self.insertItem(0, list_item) self.setItemWidget(list_item, new_source) # ATTENTION! 32 is an arbitrary number arrived at via # experimentation. It adds plenty of sources, but doesn't block diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 38f79d837..694339a01 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -520,8 +520,6 @@ def update_sources(self): Display the updated list of sources with those found in local storage. """ sources = list(storage.get_local_sources(self.session)) - if sources: - sources.sort(key=lambda x: x.last_updated) self.gui.show_sources(sources) def on_update_star_success(self, source_uuid: str) -> None: diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index 8c1707933..667859faa 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -27,7 +27,7 @@ from dateutil.parser import parse from typing import List, Tuple, Type, Union -from sqlalchemy import and_, or_ +from sqlalchemy import and_, desc, or_ from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.session import Session @@ -44,9 +44,9 @@ def get_local_sources(session: Session) -> List[Source]: """ - Return all source objects from the local database. + Return all source objects from the local database, newest first. """ - return session.query(Source).all() + return session.query(Source).order_by(desc(Source.last_updated)).all() def delete_local_source_by_uuid(session: Session, uuid: str, data_dir: str) -> None: diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 8df68d823..f0666c0a9 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -480,7 +480,6 @@ def test_MainView_setup(mocker): assert mv.controller == controller mv.source_list.setup.assert_called_once_with(controller) - mv.source_list.delete_source_by_uuid.connect.assert_called_once_with(mv.delete_conversation) def test_MainView_show_sources_with_none_selected(mocker): @@ -1003,7 +1002,7 @@ def test_SourceList_add_source_closure_adds_sources(mocker): """ sl = SourceList() sl.controller = mocker.MagicMock() - sl.addItem = mocker.MagicMock() + sl.insertItem = mocker.MagicMock() sl.setItemWidget = mocker.MagicMock() sl.setCurrentItem = mocker.MagicMock() @@ -1024,7 +1023,7 @@ def test_SourceList_add_source_closure_adds_sources(mocker): inner_fn() assert mock_sw.call_count == 1 assert mock_lwi.call_count == 1 - assert sl.addItem.call_count == 1 + assert sl.insertItem.call_count == 1 assert sl.setItemWidget.call_count == 1 assert len(sl.source_widgets) == 1 assert sl.setCurrentItem.call_count == 0 diff --git a/tests/test_storage.py b/tests/test_storage.py index 7c8c55fe5..ab67307c5 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -203,7 +203,9 @@ def test_update_local_storage(homedir, mocker, session_maker): local_reply = mocker.MagicMock() mock_session.query().all = mocker.Mock() mock_session.query().all.side_effect = [ - [local_source], [local_file], [local_message], [local_reply]] + [local_file], [local_message], [local_reply]] + mock_session.query().order_by().all = mocker.Mock() + mock_session.query().order_by().all.side_effect = [[local_source]] src_fn = mocker.patch('securedrop_client.storage.update_sources') rpl_fn = mocker.patch('securedrop_client.storage.update_replies') file_fn = mocker.patch('securedrop_client.storage.update_files') From 434e548ff0229c2886b2dd8bd370b481811dee0c Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Wed, 25 Mar 2020 18:44:25 -0400 Subject: [PATCH 5/6] functional test: first source widget should be the first in the list --- tests/functional/test_download_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_download_file.py b/tests/functional/test_download_file.py index cba5409fe..9a5e02d98 100644 --- a/tests/functional/test_download_file.py +++ b/tests/functional/test_download_file.py @@ -28,7 +28,7 @@ def check_for_sources(): qtbot.waitUntil(check_for_sources, timeout=10000) source_ids = list(gui.main_view.source_list.source_widgets.keys()) - first_source_id = source_ids[2] + first_source_id = source_ids[0] first_source_widget = gui.main_view.source_list.source_widgets[first_source_id] qtbot.mouseClick(first_source_widget, Qt.LeftButton) From c1d41cb5117aa6b6e88c01ee57674668eee6f501 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Wed, 25 Mar 2020 18:54:19 -0400 Subject: [PATCH 6/6] functional test update for source list ordering changes --- tests/functional/test_export_dialog.py | 2 +- tests/functional/test_receive_message.py | 2 +- tests/functional/test_star_source.py | 2 +- tests/functional/test_unstar_source.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functional/test_export_dialog.py b/tests/functional/test_export_dialog.py index 7d49b6335..fb76d3e97 100644 --- a/tests/functional/test_export_dialog.py +++ b/tests/functional/test_export_dialog.py @@ -28,7 +28,7 @@ def check_for_sources(): qtbot.waitUntil(check_for_sources, timeout=10000) source_ids = list(gui.main_view.source_list.source_widgets.keys()) - first_source_id = source_ids[2] + first_source_id = source_ids[0] first_source_widget = gui.main_view.source_list.source_widgets[first_source_id] qtbot.mouseClick(first_source_widget, Qt.LeftButton) diff --git a/tests/functional/test_receive_message.py b/tests/functional/test_receive_message.py index 7520cf1e4..8014f2287 100644 --- a/tests/functional/test_receive_message.py +++ b/tests/functional/test_receive_message.py @@ -28,7 +28,7 @@ def check_for_sources(): qtbot.waitUntil(check_for_sources, timeout=10000) source_ids = list(gui.main_view.source_list.source_widgets.keys()) - first_source_id = source_ids[2] + first_source_id = source_ids[0] first_source_widget = gui.main_view.source_list.source_widgets[first_source_id] qtbot.mouseClick(first_source_widget, Qt.LeftButton) diff --git a/tests/functional/test_star_source.py b/tests/functional/test_star_source.py index 0e2c5235c..cd3cf711f 100644 --- a/tests/functional/test_star_source.py +++ b/tests/functional/test_star_source.py @@ -26,7 +26,7 @@ def check_for_sources(): qtbot.waitUntil(check_for_sources, timeout=10000) source_ids = list(gui.main_view.source_list.source_widgets.keys()) - first_source_id = source_ids[1] + first_source_id = source_ids[0] first_source_widget = gui.main_view.source_list.source_widgets[first_source_id] qtbot.mouseClick(first_source_widget, Qt.LeftButton) diff --git a/tests/functional/test_unstar_source.py b/tests/functional/test_unstar_source.py index b8fe81018..a700972e3 100644 --- a/tests/functional/test_unstar_source.py +++ b/tests/functional/test_unstar_source.py @@ -26,7 +26,7 @@ def check_for_sources(): qtbot.waitUntil(check_for_sources, timeout=10000) source_ids = list(gui.main_view.source_list.source_widgets.keys()) - first_source_id = source_ids[1] + first_source_id = source_ids[0] first_source_widget = gui.main_view.source_list.source_widgets[first_source_id] qtbot.mouseClick(first_source_widget, Qt.LeftButton)