diff --git a/securedrop_client/api_jobs/downloads.py b/securedrop_client/api_jobs/downloads.py index cca93dfae..8c22f7fa7 100644 --- a/securedrop_client/api_jobs/downloads.py +++ b/securedrop_client/api_jobs/downloads.py @@ -36,8 +36,10 @@ class MetadataSyncJob(ApiJob): Update source metadata such that new download jobs can be added to the queue. ''' + NUMBER_OF_TIMES_TO_RETRY_AN_API_CALL = 15 + def __init__(self, data_dir: str, gpg: GpgHelper) -> None: - super().__init__() + super().__init__(remaining_attempts=self.NUMBER_OF_TIMES_TO_RETRY_AN_API_CALL) self.data_dir = data_dir self.gpg = gpg diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 9be4e78a1..5ed25e14a 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -76,9 +76,8 @@ def __init__(self): layout.setContentsMargins(10, 0, 0, 0) layout.setSpacing(0) - # Refresh button - self.refresh = RefreshButton() - self.refresh.disable() + # Sync icon + self.sync_icon = SyncIcon() # Activity status bar self.activity_status_bar = ActivityStatusBar() @@ -89,35 +88,35 @@ def __init__(self): # Create space the size of the status bar to keep the error status bar centered spacer = QWidget() - # Create space ths size of the refresh button to keep the error status bar centered + # Create space ths size of the sync icon to keep the error status bar centered spacer2 = QWidget() spacer2.setFixedWidth(42) # Set height of top pane to 42 pixels self.setFixedHeight(42) - self.refresh.setFixedHeight(42) + self.sync_icon.setFixedHeight(42) self.activity_status_bar.setFixedHeight(42) self.error_status_bar.setFixedHeight(42) spacer.setFixedHeight(42) spacer2.setFixedHeight(42) # Add widgets to layout - layout.addWidget(self.refresh, 1) + layout.addWidget(self.sync_icon, 1) layout.addWidget(self.activity_status_bar, 1) layout.addWidget(self.error_status_bar, 1) layout.addWidget(spacer, 1) layout.addWidget(spacer2, 1) def setup(self, controller): - self.refresh.setup(controller) + self.sync_icon.setup(controller) self.error_status_bar.setup(controller) def set_logged_in(self): - self.refresh.enable() + self.sync_icon.enable() self.setPalette(self.online_palette) def set_logged_out(self): - self.refresh.disable() + self.sync_icon.disable() self.setPalette(self.offline_palette) def update_activity_status(self, message: str, duration: int): @@ -192,13 +191,13 @@ def set_logged_out(self): self.logo.setPalette(self.offline_palette) -class RefreshButton(SvgPushButton): +class SyncIcon(QLabel): """ - A button that shows an icon for different refresh states. + An icon that shows sync state. """ CSS = ''' - #refresh_button { + #sync_icon { border: none; color: #fff; } @@ -206,60 +205,40 @@ class RefreshButton(SvgPushButton): def __init__(self): # Add svg images to button - super().__init__( - normal='refresh.svg', - disabled='refresh_offline.svg', - active='refresh_active.svg', - selected='refresh.svg', - svg_size=QSize(16, 16)) - - self.active = False - - # Set css id - self.setObjectName('refresh_button') - - # Set styles + super().__init__() + self.setObjectName('sync_icon') self.setStyleSheet(self.CSS) - self.setFixedSize(QSize(20, 20)) - - # Click event handler - self.clicked.connect(self._on_clicked) + self.setFixedSize(QSize(24, 20)) + self.sync_animation = load_movie("sync_disabled.gif") + self.sync_animation.setScaledSize(QSize(24, 20)) + self.setMovie(self.sync_animation) + self.sync_animation.start() def setup(self, controller): """ Assign a controller object (containing the application logic). """ self.controller = controller - self.controller.sync_events.connect(self._on_refresh_complete) - - def _on_clicked(self): - if self.active: - return + self.controller.sync_events.connect(self._on_sync) - self.controller.sync_api(manual_refresh=True) - - # This is a temporary solution for showing the icon as active for the entire duration of a - # refresh, rather than for just the duration of a click. The icon image will be replaced - # when the controller tells us the refresh has finished. A cleaner solution would be to - # store and update our own icon mode so we don't have to reload any images. - self.setIcon(load_icon(normal='refresh_active.svg', disabled='refresh_offline.svg')) - self.active = True - - def _on_refresh_complete(self, data): - if (data == 'synced'): - self.setIcon(load_icon( - normal='refresh.svg', - disabled='refresh_offline.svg', - active='refresh_active.svg', - selected='refresh.svg')) - self.active = False + def _on_sync(self, data): + if data == 'syncing': + self.sync_animation = load_movie("sync_active.gif") + self.sync_animation.setScaledSize(QSize(24, 20)) + self.setMovie(self.sync_animation) + self.sync_animation.start() def enable(self): - self.setEnabled(True) - self.active is False + self.sync_animation = load_movie("sync.gif") + self.sync_animation.setScaledSize(QSize(24, 20)) + self.setMovie(self.sync_animation) + self.sync_animation.start() def disable(self): - self.setEnabled(False) + self.sync_animation = load_movie("sync_disabled.gif") + self.sync_animation.setScaledSize(QSize(24, 20)) + self.setMovie(self.sync_animation) + self.sync_animation.start() class ActivityStatusBar(QStatusBar): diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index ccc91ff22..0302ff1f1 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -369,30 +369,18 @@ def authenticated(self): """ return bool(self.api and self.api.token is not None) - def sync_api(self, manual_refresh: bool = False): + def sync_api(self): """ Grab data from the remote SecureDrop API in a non-blocking manner. """ logger.debug("In sync_api on thread {}".format(self.thread().currentThreadId())) - self.sync_events.emit('syncing') - if self.authenticated(): + self.sync_events.emit('syncing') logger.debug("You are authenticated, going to make your call") job = MetadataSyncJob(self.data_dir, self.gpg) job.success_signal.connect(self.on_sync_success, type=Qt.QueuedConnection) - - # If the sync did not originate from a manual refrsh, increase the number of - # retry attempts (remaining_attempts) to 15, otherwise use the default so that a user - # finds out quicker whether or not their refresh-attempt failed. - # - # Set up failure-handling depending on whether or not the sync originated from a manual - # refresh. - if manual_refresh: - job.failure_signal.connect(self.on_refresh_failure, type=Qt.QueuedConnection) - else: - job.remaining_attempts = 15 - job.failure_signal.connect(self.on_sync_failure, type=Qt.QueuedConnection) + job.failure_signal.connect(self.on_sync_failure, type=Qt.QueuedConnection) self.api_job_queue.enqueue(job) @@ -441,16 +429,6 @@ def on_sync_failure(self, result: Exception) -> None: duration=0, retry=True) - def on_refresh_failure(self, result: Exception) -> None: - """ - Called when syncronisation of data via the API fails after a user manual clicks refresh. - """ - logger.debug('The SecureDrop server cannot be reached due to Error: {}'.format(result)) - self.gui.update_error_status( - _('The SecureDrop server cannot be reached.'), - duration=0, - retry=True) - def update_sync(self): """ Updates the UI to show human time of last sync. diff --git a/securedrop_client/resources/images/sync.gif b/securedrop_client/resources/images/sync.gif new file mode 100644 index 000000000..d42843800 Binary files /dev/null and b/securedrop_client/resources/images/sync.gif differ diff --git a/securedrop_client/resources/images/sync.svg b/securedrop_client/resources/images/sync.svg new file mode 100644 index 000000000..27c350875 --- /dev/null +++ b/securedrop_client/resources/images/sync.svg @@ -0,0 +1 @@ + Signal Icon Created with Sketch. \ No newline at end of file diff --git a/securedrop_client/resources/images/sync_active.gif b/securedrop_client/resources/images/sync_active.gif new file mode 100644 index 000000000..0397ee052 Binary files /dev/null and b/securedrop_client/resources/images/sync_active.gif differ diff --git a/securedrop_client/resources/images/sync_disabled.gif b/securedrop_client/resources/images/sync_disabled.gif new file mode 100644 index 000000000..eb8414334 Binary files /dev/null and b/securedrop_client/resources/images/sync_disabled.gif differ diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 58452ddf4..443590b21 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -4,7 +4,7 @@ import pytest from PyQt5.QtCore import Qt, QEvent -from PyQt5.QtGui import QFocusEvent +from PyQt5.QtGui import QFocusEvent, QMovie from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \ QLineEdit from sqlalchemy.orm import scoped_session, sessionmaker @@ -13,7 +13,7 @@ from securedrop_client.export import ExportError, ExportStatus from securedrop_client.gui.widgets import MainView, SourceList, SourceWidget, LoginDialog, \ SpeechBubble, MessageWidget, ReplyWidget, FileWidget, ConversationView, \ - DeleteSourceMessageBox, DeleteSourceAction, SourceMenu, TopPane, LeftPane, RefreshButton, \ + DeleteSourceMessageBox, DeleteSourceAction, SourceMenu, TopPane, LeftPane, SyncIcon, \ ErrorStatusBar, ActivityStatusBar, UserProfile, UserButton, UserMenu, LoginButton, \ ReplyBoxWidget, ReplyTextEdit, SourceConversationWrapper, StarToggleButton, LoginOfflineLink, \ LoginErrorBar, EmptyConversationView, ExportDialog, PrintDialog, PasswordEdit, SecureQLabel @@ -28,20 +28,22 @@ def test_TopPane_init(mocker): Ensure the TopPane instance is correctly set up. """ tp = TopPane() - assert not tp.refresh.isEnabled() + file_path = tp.sync_icon.sync_animation.fileName() + filename = file_path[file_path.rfind('/') + 1:] + assert filename == 'sync_disabled.gif' def test_TopPane_setup(mocker): """ - Calling setup calls setup for RefreshButton. + Calling setup calls setup for SyncIcon. """ tp = TopPane() - tp.refresh = mocker.MagicMock() + tp.sync_icon = mocker.MagicMock() mock_controller = mocker.MagicMock() tp.setup(mock_controller) - tp.refresh.setup.assert_called_once_with(mock_controller) + tp.sync_icon.setup.assert_called_once_with(mock_controller) def test_TopPane_set_logged_in(mocker): @@ -49,23 +51,23 @@ def test_TopPane_set_logged_in(mocker): Calling set_logged_in calls enable on TopPane. """ tp = TopPane() - tp.refresh = mocker.MagicMock() + tp.sync_icon = mocker.MagicMock() tp.set_logged_in() - tp.refresh.enable.assert_called_once_with() + tp.sync_icon.enable.assert_called_once_with() def test_TopPane_set_logged_out(mocker): """ - Calling set_logged_out calls disable on RefreshButton. + Calling set_logged_out calls disable on SyncIcon. """ tp = TopPane() - tp.refresh = mocker.MagicMock() + tp.sync_icon = mocker.MagicMock() tp.set_logged_out() - tp.refresh.disable.assert_called_once_with() + tp.sync_icon.disable.assert_called_once_with() def test_TopPane_update_activity_status(mocker): @@ -94,7 +96,7 @@ def test_TopPane_update_error_status(mocker): def test_TopPane_clear_error_status(mocker): """ - Calling clear_error_status calls clear_message on RefreshButton. + Calling clear_error_status calls clear_message. """ tp = TopPane() tp.error_status_bar = mocker.MagicMock() @@ -153,79 +155,112 @@ def test_LeftPane_set_logged_out(mocker): lp.user_profile.hide.assert_called_once_with() -def test_RefreshButton_setup(mocker): +def test_SyncIcon_init(mocker): + sync_icon = SyncIcon() + file_path = sync_icon.sync_animation.fileName() + filename = file_path[file_path.rfind('/') + 1:] + assert filename == 'sync_disabled.gif' + + +def test_SyncIcon_init_starts_animiation(mocker): + movie = QMovie() + movie.start = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.load_movie', return_value=movie) + + sync_icon = SyncIcon() + + sync_icon.sync_animation.start.assert_called_once_with() + + +def test_SyncIcon_setup(mocker): """ - Calling setup stores reference to controller, which will later be used to update button icon on - sync event. + Calling setup stores reference to controller, which will later be used to update sync icon on + syncing event. """ - rb = RefreshButton() + sync_icon = SyncIcon() controller = mocker.MagicMock() - rb.setup(controller) + sync_icon.setup(controller) - assert rb.controller == controller + assert sync_icon.controller == controller -def test_RefreshButton_on_clicked(mocker): - """ - When refresh button is clicked, sync_api should be called with manual_refresh set to True. - """ - rb = RefreshButton() - rb.controller = mocker.MagicMock() +def test_SyncIcon_enable(mocker): + sync_icon = SyncIcon() - rb._on_clicked() + sync_icon.enable() - rb.controller.sync_api.assert_called_once_with(manual_refresh=True) + file_path = sync_icon.sync_animation.fileName() + filename = file_path[file_path.rfind('/') + 1:] + assert filename == 'sync.gif' -def test_RefreshButton_on_clicked_while_active(mocker): - """ - When refresh button is clicked while active, sync_api should not be called. - """ - rb = RefreshButton() - rb.active = True - rb.controller = mocker.MagicMock() +def test_SyncIcon_enable_starts_animiation(mocker): + movie = QMovie() + movie.start = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.load_movie', return_value=movie) - rb._on_clicked() + sync_icon = SyncIcon() + sync_icon.enable() - rb.controller.sync_api.assert_not_called() + sync_icon.sync_animation.start.assert_called_with() -def test_RefreshButton_on_refresh_complete(mocker): - """ - Make sure we are enabled after a refresh completes. - """ - rb = RefreshButton() - rb._on_refresh_complete('synced') - assert rb.isEnabled() - assert rb.active is False +def test_SyncIcon_disable(mocker): + sync_icon = SyncIcon() + sync_icon.disable() -def test_RefreshButton_enable(mocker): - rb = RefreshButton() - rb.enable() - assert rb.isEnabled() - assert rb.active is False + file_path = sync_icon.sync_animation.fileName() + filename = file_path[file_path.rfind('/') + 1:] + assert filename == 'sync_disabled.gif' -def test_RefreshButton_disable(mocker): - rb = RefreshButton() - rb.disable() - assert not rb.isEnabled() - assert rb.active is False +def test_SyncIcon_disable_starts_animiation(mocker): + movie = QMovie() + movie.start = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.load_movie', return_value=movie) + sync_icon = SyncIcon() + sync_icon.disable() -def test_RefreshButton_disable_while_active(mocker): - rb = RefreshButton() - rb.active = True - rb.disable() - assert not rb.isEnabled() - assert rb.active is True + sync_icon.sync_animation.start.assert_called_with() + + +def test_SyncIcon__on_sync(mocker): + ''' + Sync icon becomes active when it receives the syncing sync signal. + ''' + sync_icon = SyncIcon() + + sync_icon._on_sync('syncing') + + file_path = sync_icon.sync_animation.fileName() + filename = file_path[file_path.rfind('/') + 1:] + assert filename == 'sync_active.gif' + + +def test_SyncIcon___on_sync_with_data_not_equal_to_syncing(mocker): + ''' + Sync does not because active when the sync signal's data is something other than 'syncing' + ''' + movie = QMovie() + movie.start = mocker.MagicMock() + mocker.patch('securedrop_client.gui.widgets.load_movie', return_value=movie) + sync_icon = SyncIcon() + + # assert that start call count has only been called once + sync_icon.sync_animation.start.assert_called_once_with() + + sync_icon._on_sync('something other than syncing') + + # assert that _on_sync doesn't increase start call count from one + sync_icon.sync_animation.start.assert_called_once_with() def test_ErrorStatusBar_clear_error_status(mocker): """ - Calling clear_error_status calls clear_message on RefreshButton. + Calling clear_error_status calls clear_message. """ esb = ErrorStatusBar() esb.status_bar = mocker.MagicMock() diff --git a/tests/test_logic.py b/tests/test_logic.py index ecbaf364e..ae15036f9 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -376,23 +376,6 @@ def test_Controller_sync_api(homedir, config, mocker, session_maker): co.api_job_queue.enqueue.call_count == 1 -def test_Controller_sync_api_manual_refresh(homedir, config, mocker, session_maker): - """ - Syncing from a manual refresh also enqueues a job. - """ - mock_gui = mocker.MagicMock() - - co = Controller('http://localhost', mock_gui, session_maker, homedir) - - co.authenticated = mocker.MagicMock(return_value=True) - co.api_job_queue = mocker.MagicMock() - co.api_job_queue.enqueue = mocker.MagicMock() - - co.sync_api(manual_refresh=True) - - co.api_job_queue.enqueue.call_count == 1 - - def test_Controller_last_sync_with_file(homedir, config, mocker, session_maker): """ The flag indicating the time of the last sync with the API is stored in a @@ -443,24 +426,6 @@ def test_Controller_on_sync_failure(homedir, config, mocker, session_maker): assert mock_storage.update_local_storage.call_count == 0 -def test_Controller_on_refresh_failure(homedir, config, mocker, session_maker): - """ - If there's no result to syncing, then don't attempt to update local storage - and perhaps implement some as-yet-undefined UI update. - Using the `config` fixture to ensure the config is written to disk. - """ - gui = mocker.MagicMock() - co = Controller('http://localhost', gui, session_maker, homedir) - exception = Exception('mock') # Not the expected tuple. - mock_storage = mocker.patch('securedrop_client.logic.storage') - - co.on_refresh_failure(exception) - - assert mock_storage.update_local_storage.call_count == 0 - gui.update_error_status.assert_called_once_with( - 'The SecureDrop server cannot be reached.', duration=0, retry=True) - - def test_Controller_on_sync_success(homedir, config, mocker): """ If there's a result to syncing, then update local storage.