Integration tests and unit test fixes

client/securedrop_client/export_status.py

@@ -31,8 +31,9 @@ class ExportStatus(Enum):
     SUCCESS_EXPORT = "SUCCESS_EXPORT"
     ERROR_EXPORT = "ERROR_EXPORT"  # Could not write to disk
 
-    # Export succeeds but drives were not properly unmounted
+    # Export succeeds but drives were not properly closed
     ERROR_EXPORT_CLEANUP = "ERROR_EXPORT_CLEANUP"
+    ERROR_UNMOUNT_VOLUME_BUSY = "ERROR_UNMOUNT_VOLUME_BUSY"
 
     DEVICE_ERROR = "DEVICE_ERROR"  # Something went wrong while trying to check the device
 

client/securedrop_client/gui/actions.py

@@ -15,8 +15,8 @@
 from securedrop_client import state
 from securedrop_client.conversation import Transcript as ConversationTranscript
 from securedrop_client.db import Source
+from securedrop_client.export import Export
 from securedrop_client.gui.base import ModalDialog
-from securedrop_client.gui.conversation import ExportDevice
 from securedrop_client.gui.conversation import (
     PrintTranscriptDialog as PrintConversationTranscriptDialog,
 )
@@ -184,7 +184,7 @@ def _on_triggered(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with open(file_path, "r") as f:
-            export = ExportDevice()
+            export = Export()
             dialog = PrintConversationTranscriptDialog(
                 export, TRANSCRIPT_FILENAME, [str(file_path)]
             )
@@ -234,7 +234,7 @@ def _on_triggered(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with open(file_path, "r") as f:
-            export_device = ExportDevice()
+            export_device = Export()
             wizard = ExportWizard(export_device, TRANSCRIPT_FILENAME, [str(file_path)])
             wizard.exec()
 
@@ -320,7 +320,7 @@ def _prepare_to_export(self) -> None:
         # out of scope, any pending file removal will be performed
         # by the operating system.
         with ExitStack() as stack:
-            export_device = ExportDevice()
+            export_device = Export()
             files = [
                 stack.enter_context(open(file_location, "r")) for file_location in file_locations
             ]

client/securedrop_client/gui/conversation/__init__.py

@@ -3,7 +3,6 @@
 """
 # Import classes here to make possible to import them from securedrop_client.gui.conversation
 from .delete import DeleteConversationDialog  # noqa: F401
-from .export import Export as ExportDevice  # noqa: F401
 from .export import ExportWizard as ExportWizard  # noqa: F401
-from .export import PrintDialog as PrintFileDialog  # noqa: F401
+from .export import PrintDialog  # noqa: F401
 from .export import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/__init__.py

@@ -1,4 +1,3 @@
-from ....export import Export  # noqa: F401
 from .export_wizard import ExportWizard  # noqa: F401
 from .print_dialog import PrintDialog  # noqa: F401
 from .print_transcript_dialog import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/export_wizard.py

@@ -199,6 +199,7 @@ def on_status_received(self, status: ExportStatus) -> None:
         # Release the page (page was held during "next" button click event)
         page = self.currentPage()
         if isinstance(page, ExportWizardPage):
+            logger.debug(f"page {page} received {status.value}, release page_complete")
             page.set_complete(True)
         self._stop_animate_activestate()
 

client/securedrop_client/gui/conversation/export/export_wizard_page.py

@@ -85,6 +85,7 @@ def _build_layout(self) -> QVBoxLayout:
         """
         Create parent layout, draw elements, return parent layout
         """
+        self.setObjectName("QWizard_export_page")
         self.setStyleSheet(self.WIZARD_CSS)
         parent_layout = QVBoxLayout(self)
         parent_layout.setContentsMargins(self.MARGIN, self.MARGIN, self.MARGIN, self.MARGIN)
@@ -236,12 +237,13 @@ def nextId(self) -> int:
 
     @pyqtSlot(object)
     def on_status_received(self, status: ExportStatus) -> None:
-        self.stop_animate_header()
-        header = _("Ready to export:<br />" '<span style="font-weight:normal">{}</span>').format(
-            self.summary
-        )
-        self.header.setText(header)
         self.status = status
+        if isinstance(self.wizard().currentPage(), PreflightPage):
+            self.stop_animate_header()
+            header = _(
+                "Ready to export:<br />" '<span style="font-weight:normal">{}</span>'
+            ).format(self.summary)
+            self.header.setText(header)
 
 
 class ErrorPage(ExportWizardPage):
@@ -271,29 +273,26 @@ def __init__(self, export: Export, summary: str) -> None:
 
     @pyqtSlot(object)
     def on_status_received(self, status: ExportStatus) -> None:
-        logger.debug(f"InsertUSB received {status.value}")
-        should_show_hint = status in (
-            ExportStatus.MULTI_DEVICE_DETECTED,
-            ExportStatus.INVALID_DEVICE_DETECTED,
-        ) or (
-            self.status == status == ExportStatus.NO_DEVICE_DETECTED
-            and isinstance(self.wizard().currentPage, InsertUSBPage)
-        )
-        self.update_content(status, should_show_hint)
         self.status = status
-        self.completeChanged.emit()
-        if status in (ExportStatus.DEVICE_LOCKED, ExportStatus.DEVICE_WRITABLE) and isinstance(
-            self.wizard().currentPage(), InsertUSBPage
-        ):
-            logger.debug("Device detected - advance the wizard")
-            self.wizard().next()
+        if isinstance(self.wizard().currentPage(), InsertUSBPage):
+            logger.debug(f"InsertUSB received {status.value}")
+            should_show_hint = status in (
+                ExportStatus.MULTI_DEVICE_DETECTED,
+                ExportStatus.INVALID_DEVICE_DETECTED,
+                ExportStatus.NO_DEVICE_DETECTED,
+            )
+            self.update_content(status, should_show_hint)
+            if status in (ExportStatus.DEVICE_LOCKED, ExportStatus.DEVICE_WRITABLE):
+                logger.debug("Device detected - advance the wizard")
+                self.wizard().next()
 
     def validatePage(self) -> bool:
         """
         Override method to implement custom validation logic, which
         shows an error-specific hint to the user.
         """
         if self.status in (ExportStatus.DEVICE_WRITABLE, ExportStatus.DEVICE_LOCKED):
+            logger.debug("InsertUSBPage: Hide error details")
             self.error_details.hide()
             return True
         else:
@@ -305,6 +304,7 @@ def validatePage(self) -> bool:
                 ExportStatus.NO_DEVICE_DETECTED,
                 ExportStatus.INVALID_DEVICE_DETECTED,
             ):
+                logger.debug("InsertUSBPage: Show error details")
                 self.update_content(self.status, should_show_hint=True)
                 return False
             else:
@@ -341,8 +341,8 @@ def __init__(self, export: Export) -> None:
     @pyqtSlot(object)
     def on_status_received(self, status: ExportStatus) -> None:
         logger.debug(f"Final page received status {status}")
-        self.update_content(status)
         self.status = status
+        self.update_content(status)
 
     def update_content(self, status: ExportStatus, should_show_hint: bool = False) -> None:
         header = None
@@ -354,7 +354,7 @@ def update_content(self, status: ExportStatus, should_show_hint: bool = False) -
                 "outside of your Workstation machine."
             )
         elif status == ExportStatus.ERROR_EXPORT_CLEANUP:
-            header = header = _("Export sucessful, but drive was not locked")
+            header = _("Export sucessful, but drive was not locked")
             body = STATUS_MESSAGES.get(ExportStatus.ERROR_EXPORT_CLEANUP)
 
         else:
@@ -412,19 +412,20 @@ def _build_layout(self) -> QVBoxLayout:
 
     @pyqtSlot(object)
     def on_status_received(self, status: ExportStatus) -> None:
-        logger.debug(f"Passphrase page rececived {status.value}")
-        should_show_hint = status in (
-            ExportStatus.ERROR_UNLOCK_LUKS,
-            ExportStatus.ERROR_UNLOCK_GENERIC,
-        )
-        self.update_content(status, should_show_hint)
         self.status = status
-        self.completeChanged.emit()
-        if status in (
-            ExportStatus.SUCCESS_EXPORT,
-            ExportStatus.ERROR_EXPORT_CLEANUP,
-        ) and isinstance(self.wizard().currentPage(), PassphraseWizardPage):
-            self.wizard().next()
+        if isinstance(self.wizard().currentPage(), PassphraseWizardPage):
+            logger.debug(f"Passphrase page rececived {status.value}")
+            if status in (
+                ExportStatus.SUCCESS_EXPORT,
+                ExportStatus.ERROR_EXPORT_CLEANUP,
+            ):
+                self.set_complete(True)
+                self.wizard().next()
+            should_show_hint = status in (
+                ExportStatus.ERROR_UNLOCK_LUKS,
+                ExportStatus.ERROR_UNLOCK_GENERIC,
+            )
+            self.update_content(status, should_show_hint)
 
     def validatePage(self) -> bool:
         # Also to add: DEVICE_BUSY for unmounting.

client/securedrop_client/gui/conversation/export/print_dialog.py

@@ -97,7 +97,7 @@ def _print_file(self) -> None:
         self._device.print(self.filepaths)
         self.close()
 
-    @pyqtSlot()
+    @pyqtSlot(object)
     def _on_print_preflight_check_succeeded(self, status: ExportStatus) -> None:
         # We don't use the ExportStatus for now for "success" status,
         # but in future work we will migrate towards a wizard-style dialog, where

client/securedrop_client/gui/conversation/export/wizard.css

@@ -1,9 +1,13 @@
 #QWizard_export {
     min-width: 800px;
     max-width: 800px;
-    min-height: 300px;
+    min-height: 500px;
     max-height: 800px;
-    background-color: #fff;
+    background: #ffffff;
+}
+
+#QWizard_export_page {
+    background: #ffffff;
 }
 
 #QWizard_header_icon, #QWizard_header_spinner {

client/securedrop_client/gui/widgets.py

@@ -70,6 +70,7 @@
     Source,
     User,
 )
+from securedrop_client.export import Export
 from securedrop_client.gui import conversation
 from securedrop_client.gui.actions import (
     DeleteConversationAction,
@@ -81,7 +82,6 @@
 )
 from securedrop_client.gui.base import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton
 from securedrop_client.gui.conversation import DeleteConversationDialog
-from securedrop_client.gui.conversation.export import ExportWizard
 from securedrop_client.gui.datetime_helpers import format_datetime_local
 from securedrop_client.gui.source import DeleteSourceDialog
 from securedrop_client.logic import Controller
@@ -2460,9 +2460,11 @@ def _on_export_clicked(self) -> None:
             logger.debug("Clicked export but file not downloaded")
             return
 
-        export_device = conversation.ExportDevice()
+        export_device = Export()
 
-        self.export_wizard = ExportWizard(export_device, self.file.filename, [file_location])
+        self.export_wizard = conversation.ExportWizard(
+            export_device, self.file.filename, [file_location]
+        )
         self.export_wizard.show()
 
     @pyqtSlot()
@@ -2476,9 +2478,9 @@ def _on_print_clicked(self) -> None:
 
         filepath = self.file.location(self.controller.data_dir)
 
-        export_device = conversation.ExportDevice()
+        export_device = Export()
 
-        dialog = conversation.PrintFileDialog(export_device, self.file.filename, [filepath])
+        dialog = conversation.PrintDialog(export_device, self.file.filename, [filepath])
         dialog.exec()
 
     def _on_left_click(self) -> None:

client/tests/conftest.py

@@ -23,6 +23,7 @@
     Source,
     make_session_maker,
 )
+from securedrop_client.export import Export
 from securedrop_client.export_status import ExportStatus
 from securedrop_client.gui import conversation
 from securedrop_client.gui.main import Window
@@ -78,9 +79,9 @@ def lang(request):
 def print_dialog(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
-    dialog = conversation.PrintFileDialog(export_device, "file123.jpg", ["/mock/path/to/file"])
+    dialog = conversation.PrintDialog(export_device, "file123.jpg", ["/mock/path/to/file"])
 
     yield dialog
 
@@ -89,7 +90,7 @@ def print_dialog(mocker, homedir):
 def print_transcript_dialog(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.PrintTranscriptDialog(
         export_device, "transcript.txt", ["some/path/transcript.txt"]
@@ -102,7 +103,7 @@ def print_transcript_dialog(mocker, homedir):
 def export_wizard_multifile(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     wizard = conversation.ExportWizard(
         export_device,
@@ -117,7 +118,7 @@ def export_wizard_multifile(mocker, homedir):
 def export_wizard(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.ExportWizard(export_device, "file123.jpg", ["/mock/path/to/file"])
 
@@ -128,7 +129,7 @@ def export_wizard(mocker, homedir):
 def export_transcript_wizard(mocker, homedir):
     mocker.patch("PyQt5.QtWidgets.QApplication.activeWindow", return_value=QMainWindow())
 
-    export_device = mocker.MagicMock(spec=conversation.ExportDevice)
+    export_device = mocker.MagicMock(spec=Export)
 
     dialog = conversation.ExportWizard(
         export_device, "transcript.txt", ["/some/path/transcript.txt"]
@@ -177,7 +178,7 @@ def mock_export_locked():
         * "Export" clicked, export wizard launched
         * Passphrase successfully entered on first attempt (and export suceeeds)
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.NO_DEVICE_DETECTED
@@ -201,7 +202,7 @@ def mock_export_unlocked():
         * Export wizard launched
         * Export succeeds
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.DEVICE_WRITABLE
@@ -225,7 +226,7 @@ def mock_export_no_usb_then_bad_passphrase():
         * Correct passphrase
         * Export succeeds
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.NO_DEVICE_DETECTED
@@ -256,7 +257,7 @@ def mock_export_fail_early():
         * Unrecoverable error before export happens
           (eg, mount error)
     """
-    device = conversation.ExportDevice()
+    device = mock.MagicMock(spec=Export)
 
     device.run_export_preflight_checks = lambda: device.export_state_changed.emit(
         ExportStatus.DEVICE_LOCKED