Fixes #412 moves update_star calls under queue

Now we are using the same general queue for update_star
method on sources.

securedrop_client/api_jobs/updatestar.py

@@ -0,0 +1,42 @@
+import logging
+import sdclientapi
+
+from sdclientapi import API
+from sqlalchemy.orm.session import Session
+
+from securedrop_client.api_jobs.base import ApiJob
+
+logger = logging.getLogger(__name__)
+
+
+class UpdateStarJob(ApiJob):
+    def __init__(self, source_uuid: str, star_status: bool) -> None:
+        super().__init__()
+        self.source_uuid = source_uuid
+        self.star_status = star_status
+
+    def call_api(self, api_client: API, session: Session) -> str:
+        '''
+        Override ApiJob.
+
+        Star or Unstar an user on the server
+        '''
+        try:
+            source_sdk_object = sdclientapi.Source(uuid=self.source_uuid)
+
+            if self.star_status:
+                api_client.remove_star(source_sdk_object)
+            else:
+                api_client.add_star(source_sdk_object)
+
+            return self.source_uuid
+        except Exception as e:
+            error_message = "Failed to update star on source {uuid} due to {exception}".format(
+                uuid=self.source_uuid, exception=repr(e))
+            raise UpdateStarJobException(error_message, self.source_uuid)
+
+
+class UpdateStarJobException(Exception):
+    def __init__(self, message: str, source_uuid: str):
+        super().__init__(message)
+        self.source_uuid = source_uuid

securedrop_client/logic.py

@@ -34,6 +34,7 @@
 from securedrop_client.api_jobs.downloads import FileDownloadJob, MessageDownloadJob, \
     ReplyDownloadJob
 from securedrop_client.api_jobs.uploads import SendReplyJob, SendReplyJobException
+from securedrop_client.api_jobs.updatestar import UpdateStarJob, UpdateStarJobException
 from securedrop_client.crypto import GpgHelper, CryptoError
 from securedrop_client.queue import ApiJobQueue
 from securedrop_client.utils import check_dir_permissions
@@ -420,7 +421,7 @@ def on_update_star_success(self, result) -> None:
         self.sync_api()  # Syncing the API also updates the source list UI
         self.gui.clear_error_status()
 
-    def on_update_star_failure(self, result: Exception) -> None:
+    def on_update_star_failure(self, result: UpdateStarJobException) -> None:
         """
         After we unstar a source, we should sync the API such that the local database is updated.
         """
@@ -439,18 +440,14 @@ def update_star(self, source_db_object):
         else:  # Clear the error status bar
             self.gui.clear_error_status()
 
-        source_sdk_object = sdclientapi.Source(uuid=source_db_object.uuid)
+        job = UpdateStarJob(
+                source_db_object.uuid,
+                source_db_object.is_starred
+        )
+        job.success_signal.connect(self.on_update_star_success, type=Qt.QueuedConnection)
+        job.failure_signal.connect(self.on_update_star_failure, type=Qt.QueuedConnection)
 
-        if source_db_object.is_starred:
-            self.call_api(self.api.remove_star,
-                          self.on_update_star_success,
-                          self.on_update_star_failure,
-                          source_sdk_object)
-        else:
-            self.call_api(self.api.add_star,
-                          self.on_update_star_success,
-                          self.on_update_star_failure,
-                          source_sdk_object)
+        self.api_job_queue.enqueue(job)
 
     def logout(self):
         """

tests/api_jobs/test_updatestar.py

@@ -0,0 +1,84 @@
+import pytest
+
+from securedrop_client.api_jobs.updatestar import UpdateStarJob, UpdateStarJobException
+from tests import factory
+
+
+def test_star_if_unstar(homedir, mocker, session, session_maker):
+    '''
+    Check if we call add_star method if a source is not stared.
+    '''
+    source = factory.Source()
+    session.add(source)
+    session.commit()
+
+    api_client = mocker.MagicMock()
+
+    api_client.add_star = mocker.MagicMock()
+
+    mock_sdk_source = mocker.Mock()
+    mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source',
+                                    return_value=mock_sdk_source)
+
+    job = UpdateStarJob(
+        source.uuid,
+        source.is_starred
+    )
+
+    job.call_api(api_client, session)
+
+    # ensure we call add_star with right uuid for source
+    mock_source_init.assert_called_once_with(uuid=source.uuid)
+    api_client.add_star.assert_called_once_with(mock_sdk_source)
+
+
+def test_unstar_if_star(homedir, mocker, session, session_maker):
+    '''
+    Check if we call remove_star method if a source is stared.
+    '''
+    source = factory.Source()
+    source.is_starred = True
+    session.add(source)
+    session.commit()
+
+    api_client = mocker.MagicMock()
+
+    api_client.remove_star = mocker.MagicMock()
+
+    mock_sdk_source = mocker.Mock()
+    mock_source_init = mocker.patch('securedrop_client.logic.sdclientapi.Source',
+                                    return_value=mock_sdk_source)
+
+    job = UpdateStarJob(
+        source.uuid,
+        source.is_starred
+    )
+
+    job.call_api(api_client, session)
+
+    # ensure we call remove start wtih right source uuid
+    mock_source_init.assert_called_once_with(uuid=source.uuid)
+    api_client.remove_star.assert_called_once_with(mock_sdk_source)
+
+
+def test_failure_to_star(homedir, mocker, session, session_maker):
+    '''
+    Check if we call remove_star method if a source is stared.
+    '''
+    source = factory.Source()
+    source.is_starred = True
+    session.add(source)
+    session.commit()
+
+    api_client = mocker.MagicMock()
+
+    api_client.remove_star = mocker.MagicMock()
+    api_client.remove_star.side_effect = Exception
+
+    job = UpdateStarJob(
+        source.uuid,
+        source.is_starred
+    )
+
+    with pytest.raises(UpdateStarJobException):
+        job.call_api(api_client, session)

tests/test_logic.py

@@ -559,70 +559,6 @@ def test_Controller_update_sources(homedir, config, mocker):
     mock_gui.show_sources.assert_called_once_with(source_list)
 
 
-def test_Controller_unstars_a_source_if_starred(homedir, config, mocker, session_maker):
-    """
-    Ensure that the client unstars a source if it is starred.
-    Using the `config` fixture to ensure the config is written to disk.
-    """
-    mock_gui = mocker.MagicMock()
-    co = Controller('http://localhost', mock_gui, session_maker, homedir)
-
-    source_db_object = mocker.MagicMock()
-    source_db_object.uuid = mocker.MagicMock()
-    source_db_object.is_starred = True
-
-    co.call_api = mocker.MagicMock()
-    co.api = mocker.MagicMock()
-    co.api.remove_star = mocker.MagicMock()
-    co.on_update_star_success = mocker.MagicMock()
-    co.on_update_star_failure = mocker.MagicMock()
-
-    source_sdk_object = mocker.MagicMock()
-    mock_source = mocker.patch('sdclientapi.Source')
-    mock_source.return_value = source_sdk_object
-    co.update_star(source_db_object)
-
-    co.call_api.assert_called_once_with(
-        co.api.remove_star,
-        co.on_update_star_success,
-        co.on_update_star_failure,
-        source_sdk_object,
-    )
-    mock_gui.clear_error_status.assert_called_once_with()
-
-
-def test_Controller_unstars_a_source_if_unstarred(homedir, config, mocker, session_maker):
-    """
-    Ensure that the client stars a source if it is unstarred.
-    Using the `config` fixture to ensure the config is written to disk.
-    """
-    mock_gui = mocker.MagicMock()
-    co = Controller('http://localhost', mock_gui, session_maker, homedir)
-
-    source_db_object = mocker.MagicMock()
-    source_db_object.uuid = mocker.MagicMock()
-    source_db_object.is_starred = False
-
-    co.call_api = mocker.MagicMock()
-    co.api = mocker.MagicMock()
-    co.api.add_star = mocker.MagicMock()
-    co.on_update_star_success = mocker.MagicMock()
-    co.on_update_star_failure = mocker.MagicMock()
-
-    source_sdk_object = mocker.MagicMock()
-    mock_source = mocker.patch('sdclientapi.Source')
-    mock_source.return_value = source_sdk_object
-    co.update_star(source_db_object)
-
-    co.call_api.assert_called_once_with(
-        co.api.add_star,
-        co.on_update_star_success,
-        co.on_update_star_failure,
-        source_sdk_object,
-    )
-    mock_gui.clear_error_status.assert_called_once_with()
-
-
 def test_Controller_update_star_not_logged_in(homedir, config, mocker, session_maker):
     """
     Ensure that starring/unstarring a source when not logged in calls
@@ -1247,3 +1183,38 @@ def test_Controller_api_call_timeout(homedir, config, mocker, session_maker):
     co.on_api_timeout()
     mock_gui.update_error_status.assert_called_once_with(
         'The connection to the SecureDrop server timed out. Please try again.')
+
+
+def test_Controller_call_update_star_success(homedir, config, mocker, session_maker, session):
+    '''
+    Check that a UpdateStar is submitted to the queue when update_star is called.
+    '''
+    mock_gui = mocker.MagicMock()
+    co = Controller('http://localhost', mock_gui, session_maker, homedir)
+    co.call_api = mocker.MagicMock()
+    co.api = mocker.MagicMock()
+
+    mock_success_signal = mocker.MagicMock()
+    mock_failure_signal = mocker.MagicMock()
+    mock_job = mocker.MagicMock(success_signal=mock_success_signal,
+                                failure_signal=mock_failure_signal)
+    mock_job_cls = mocker.patch(
+        "securedrop_client.logic.UpdateStarJob", return_value=mock_job)
+    mock_queue = mocker.patch.object(co, 'api_job_queue')
+
+    source = factory.Source()
+    session.add(source)
+    session.commit()
+
+    co.update_star(source)
+
+    mock_job_cls.assert_called_once_with(
+        source.uuid,
+        source.is_starred
+    )
+
+    mock_queue.enqueue.assert_called_once_with(mock_job)
+    mock_success_signal.connect.assert_called_once_with(
+        co.on_update_star_success, type=Qt.QueuedConnection)
+    mock_failure_signal.connect.assert_called_once_with(
+        co.on_update_star_failure, type=Qt.QueuedConnection)