Merge pull request #817 from freedomofpress/no-waiting-to-sync-when-o…

…ffline

update sync method names and message

securedrop_client/gui/main.py

@@ -149,14 +149,14 @@ def show_sources(self, sources: List[Source]):
         """
         self.main_view.show_sources(sources)
 
-    def show_sync(self, updated_on):
+    def show_last_sync(self, updated_on):
         """
-        Display a message indicating the data-sync state.
+        Display a message indicating the time of last sync with the server.
         """
         if updated_on:
             self.update_activity_status(_('Last Refresh: {}').format(updated_on.humanize()))
         else:
-            self.update_activity_status(_('Waiting to refresh...'), 5000)
+            self.update_activity_status(_('Last Refresh: never'))
 
     def set_logged_in_as(self, db_user: User):
         """

securedrop_client/logic.py

@@ -50,8 +50,8 @@
 
 logger = logging.getLogger(__name__)
 
-
-SYNC_FREQUENCY = 30000  #: the number of milliseconds between sync updates.
+# 30 seconds between showing the time since the last sync
+TIME_BETWEEN_SHOWING_LAST_SYNC_MS = 1000 * 30
 
 
 def login_required(f):
@@ -229,8 +229,6 @@ def __init__(self, hostname: str, gui, session_maker: sessionmaker,
 
         self.export = Export()
 
-        self.sync_flag = os.path.join(home, 'sync_flag')
-
         # File data.
         self.data_dir = os.path.join(self.home, 'data')
 
@@ -240,9 +238,12 @@ def __init__(self, hostname: str, gui, session_maker: sessionmaker,
         self.api_sync.sync_success.connect(self.on_sync_success, type=Qt.QueuedConnection)
         self.api_sync.sync_failure.connect(self.on_sync_failure, type=Qt.QueuedConnection)
 
-        # Create a timer to check for sync status every SYNC_FREQUENCY seconds.
+        # Create a timer to show the time since the last sync
         self.show_last_sync_timer = QTimer()
-        self.show_last_sync_timer.timeout.connect(self.show_last_sync_time)
+        self.show_last_sync_timer.timeout.connect(self.show_last_sync)
+
+        # Path to the file containing the timestamp since the last sync with the server
+        self.last_sync_filepath = os.path.join(home, 'sync_flag')
 
     @property
     def is_authenticated(self) -> bool:
@@ -324,7 +325,7 @@ def on_queue_paused(self) -> None:
             _('The SecureDrop server cannot be reached.'),
             duration=0,
             retry=True)
-        self.show_last_sync_timer.start(SYNC_FREQUENCY)
+        self.show_last_sync_timer.start(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
     def resume_queues(self) -> None:
         self.api_job_queue.resume_queues()
@@ -403,8 +404,8 @@ def login_offline_mode(self):
         storage.mark_all_pending_drafts_as_failed(self.session)
         self.is_authenticated = False
         self.update_sources()
-        self.show_last_sync_time()
-        self.show_last_sync_timer.start(SYNC_FREQUENCY)
+        self.show_last_sync()
+        self.show_last_sync_timer.start(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
     def on_action_requiring_login(self):
         """
@@ -420,12 +421,12 @@ def authenticated(self):
         """
         return bool(self.api and self.api.token is not None)
 
-    def last_sync(self):
+    def get_last_sync(self):
         """
         Returns the time of last synchronisation with the remote SD server.
         """
         try:
-            with open(self.sync_flag) as f:
+            with open(self.last_sync_filepath) as f:
                 return arrow.get(f.read())
         except Exception:
             return None
@@ -442,7 +443,7 @@ def on_sync_success(self) -> None:
             * Download new messages and replies
             * Update missing files so that they can be re-downloaded
         """
-        with open(self.sync_flag, 'w') as f:
+        with open(self.last_sync_filepath, 'w') as f:
             f.write(arrow.now().format())
 
         storage.update_missing_files(self.data_dir, self.session)
@@ -465,11 +466,11 @@ def on_sync_failure(self, result: Exception) -> None:
             self.logout()
             self.gui.show_login(error=_('Your session expired. Please log in again.'))
 
-    def show_last_sync_time(self):
+    def show_last_sync(self):
         """
         Updates the UI to show human time of last sync.
         """
-        self.gui.show_sync(self.last_sync())
+        self.gui.show_last_sync(self.get_last_sync())
 
     def update_sources(self):
         """
@@ -522,8 +523,8 @@ def logout(self):
         self.api_job_queue.stop()
         self.gui.logout()
 
-        self.show_last_sync_timer.start(SYNC_FREQUENCY)
-        self.show_last_sync_time()
+        self.show_last_sync_timer.start(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
+        self.show_last_sync()
         self.is_authenticated = False
 
     def invalidate_token(self):

tests/gui/test_main.py

@@ -223,26 +223,26 @@ def test_clear_error_status(mocker):
     w.top_pane.clear_error_status.assert_called_once_with()
 
 
-def test_show_sync(mocker):
+def test_show_last_sync(mocker):
     """
     If there's a value display the result of its "humanize" method.humanize
     """
     w = Window()
     w.update_activity_status = mocker.MagicMock()
     updated_on = mocker.MagicMock()
-    w.show_sync(updated_on)
+    w.show_last_sync(updated_on)
     w.update_activity_status.assert_called_once_with(
         'Last Refresh: {}'.format(updated_on.humanize()))
 
 
-def test_show_sync_no_sync(mocker):
+def test_show_last_sync_no_sync(mocker):
     """
     If there's no value to display, default to a "waiting" message.
     """
     w = Window()
     w.update_activity_status = mocker.MagicMock()
-    w.show_sync(None)
-    w.update_activity_status.assert_called_once_with('Waiting to refresh...', 5000)
+    w.show_last_sync(None)
+    w.update_activity_status.assert_called_once_with('Last Refresh: never')
 
 
 def test_set_logged_in_as(mocker):

tests/test_logic.py

@@ -11,7 +11,7 @@
 from tests import factory
 
 from securedrop_client import db
-from securedrop_client.logic import APICallRunner, Controller, SYNC_FREQUENCY
+from securedrop_client.logic import APICallRunner, Controller, TIME_BETWEEN_SHOWING_LAST_SYNC_MS
 from securedrop_client.api_jobs.base import ApiInaccessibleError
 from securedrop_client.api_jobs.downloads import (
     DownloadChecksumMismatchException, DownloadDecryptionException, DownloadException
@@ -170,7 +170,7 @@ def test_Controller_login_offline_mode(homedir, config, mocker):
     co.gui.show_main_window.assert_called_once_with()
     co.gui.hide_login.assert_called_once_with()
     co.update_sources.assert_called_once_with()
-    co.show_last_sync_timer.start.assert_called_once_with(SYNC_FREQUENCY)
+    co.show_last_sync_timer.start.assert_called_once_with(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
 
 def test_Controller_on_authenticate_failure(homedir, config, mocker, session_maker):
@@ -202,7 +202,6 @@ def test_Controller_on_authenticate_success(homedir, config, mocker, session_mak
     user = factory.User()
     mock_gui = mocker.MagicMock()
     co = Controller('http://localhost', mock_gui, session_maker, homedir)
-    # co.api_sync = mocker.MagicMock()
     co.api_sync.start = mocker.MagicMock()
     co.api_job_queue.start = mocker.MagicMock()
     co.update_sources = mocker.MagicMock()
@@ -361,7 +360,7 @@ def test_Controller_last_sync_with_file(homedir, config, mocker, session_maker):
     timestamp = '2018-10-10 18:17:13+01:00'
     mocker.patch("builtins.open", mocker.mock_open(read_data=timestamp))
 
-    result = co.last_sync()
+    result = co.get_last_sync()
 
     assert isinstance(result, arrow.Arrow)
     assert result.format() == timestamp
@@ -377,7 +376,7 @@ def test_Controller_last_sync_no_file(homedir, config, mocker, session_maker):
     co = Controller('http://localhost', mock_gui, session_maker, homedir)
 
     mocker.patch("builtins.open", mocker.MagicMock(side_effect=Exception()))
-    assert co.last_sync() is None
+    assert co.get_last_sync() is None
 
 
 def test_Controller_on_sync_started(mocker, homedir):
@@ -454,19 +453,21 @@ def test_Controller_on_sync_success(homedir, config, mocker):
     co.resume_queues.assert_called_once_with()
 
 
-def test_Controller_show_last_sync_time(homedir, config, mocker, session_maker):
+def test_Controller_show_last_sync(homedir, config, mocker, session_maker):
     """
-    Cause the UI to update with the result of self.last_sync().
+    Ensure we get the last sync time when we show it.
     Using the `config` fixture to ensure the config is written to disk.
     This should only happen if the user isn't logged in or the API queues are
     paused (indicating network problems).
     """
     co = Controller('http://localhost', mocker.MagicMock(), session_maker, homedir)
-    co.last_sync = mocker.MagicMock()
+    co.get_last_sync = mocker.MagicMock()
     co.api = None
-    co.show_last_sync_time()
-    assert co.last_sync.call_count == 1
-    co.gui.show_sync.assert_called_once_with(co.last_sync())
+
+    co.show_last_sync()
+
+    assert co.get_last_sync.call_count == 1
+    co.gui.show_last_sync.assert_called_once_with(co.get_last_sync())
 
 
 def test_Controller_update_sources(homedir, config, mocker):
@@ -627,7 +628,7 @@ def test_Controller_logout_success(homedir, config, mocker, session_maker):
     msg = 'Client logout successful'
     info_logger.assert_called_once_with(msg)
     fail_draft_replies.called_once_with(co.session)
-    co.show_last_sync_timer.start.assert_called_once_with(SYNC_FREQUENCY)
+    co.show_last_sync_timer.start.assert_called_once_with(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
 
 def test_Controller_logout_failure(homedir, config, mocker, session_maker):
@@ -1472,7 +1473,7 @@ def test_Controller_on_queue_paused(homedir, config, mocker, session_maker):
     co.on_queue_paused()
     mock_gui.update_error_status.assert_called_once_with(
         'The SecureDrop server cannot be reached.', duration=0, retry=True)
-    co.show_last_sync_timer.start.assert_called_once_with(SYNC_FREQUENCY)
+    co.show_last_sync_timer.start.assert_called_once_with(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
 
 def test_Controller_call_update_star_success(homedir, config, mocker, session_maker, session):