Merge pull request #5905 from freedomofpress/single-file-message

Use definite article for single file error case; improve functional test output

securedrop/journalist_app/col.py

@@ -116,7 +116,7 @@ def download_single_file(filesystem_id: str, fn: str) -> werkzeug.Response:
         if not Path(file).is_file():
             flash(
                 gettext(
-                    "Your download failed because a file could not be found. An admin can find "
+                    "Your download failed because the file could not be found. An admin can find "
                     + "more information in the system and monitoring logs."
                 ),
                 "error"

securedrop/journalist_app/utils.py

@@ -205,9 +205,12 @@ def download(
         zf = current_app.storage.get_bulk_archive(submissions, zip_directory=zip_basename)
     except FileNotFoundError:
         flash(
-            gettext(
+            ngettext(
+                "Your download failed because the file could not be found. An admin can find "
+                + "more information in the system and monitoring logs.",
                 "Your download failed because a file could not be found. An admin can find "
-                + "more information in the system and monitoring logs."
+                + "more information in the system and monitoring logs.",
+                len(submissions)
             ),
             "error"
         )

securedrop/tests/__init__.py

@@ -1,8 +1,13 @@
 # -*- coding: utf-8 -*-
 from os.path import abspath, dirname, join, realpath
+import pytest
 import sys
 
 # The tests directory should be adjacent to the securedrop directory. By adding
 # the securedrop directory to sys.path here, all test modules are able to
 # directly import modules in the securedrop directory.
 sys.path.append(abspath(join(dirname(realpath(__file__)), '..', 'securedrop')))
+
+# This ensures we get pytest's more detailed assertion output in helper functions
+pytest.register_assert_rewrite('tests.functional.journalist_navigation_steps')
+pytest.register_assert_rewrite('tests.functional.source_navigation_steps')

securedrop/tests/functional/journalist_navigation_steps.py

@@ -1181,15 +1181,24 @@ def _journalist_uses_js_buttons_to_download_unread(self):
             classes = checkbox.get_attribute("class")
             assert "unread-cb" in classes
 
-    def _journalist_sees_missing_file_error_message(self):
+    def _journalist_sees_missing_file_error_message(self, single_file=False):
         notification = self.driver.find_element_by_css_selector(".error")
 
-        if self.accept_languages is None:
-            expected_text = (
+        # We use a definite article ("the" instead of "a") if a single file
+        # is downloaded directly.
+        if single_file:
+            error_msg = (
+                "Your download failed because the file could not be found. An admin can find "
+                + "more information in the system and monitoring logs."
+            )
+        else:
+            error_msg = (
                 "Your download failed because a file could not be found. An admin can find "
                 + "more information in the system and monitoring logs."
             )
-            assert expected_text == notification.text
+
+        if self.accept_languages is None:
+            assert notification.text == error_msg
 
     def _journalist_is_on_collection_page(self):
         return self.wait_for(

securedrop/tests/functional/test_journalist.py

@@ -133,23 +133,23 @@ def test_download_source_unread(self, missing_msg_file):
         journalist home page."""
         self._journalist_logs_in()
         self._journalist_clicks_source_unread()
-        self._journalist_sees_missing_file_error_message()
+        self._journalist_sees_missing_file_error_message(single_file=True)
         self._is_on_journalist_homepage()
 
     def test_select_source_and_download_all(self, missing_msg_file):
         """Test behavior when the journalist selects the source then clicks the "Download" button
         from the journalist home page."""
         self._journalist_logs_in()
         self._journalist_selects_first_source_then_download_all()
-        self._journalist_sees_missing_file_error_message()
+        self._journalist_sees_missing_file_error_message(single_file=True)
         self._is_on_journalist_homepage()
 
     def test_select_source_and_download_unread(self, missing_msg_file):
         """Test behavior when the journalist selects the source then clicks the "Download Unread"
         button from the journalist home page."""
         self._journalist_logs_in()
         self._journalist_selects_first_source_then_download_unread()
-        self._journalist_sees_missing_file_error_message()
+        self._journalist_sees_missing_file_error_message(single_file=True)
         self._is_on_journalist_homepage()
 
     def test_download_message(self, missing_msg_file):
@@ -158,7 +158,7 @@ def test_download_message(self, missing_msg_file):
         self._journalist_logs_in()
         self._journalist_checks_messages()
         self._journalist_downloads_message_missing_file()
-        self._journalist_sees_missing_file_error_message()
+        self._journalist_sees_missing_file_error_message(single_file=True)
         self._journalist_is_on_collection_page()
 
     def test_select_message_and_download_selected(self, missing_msg_file):
@@ -167,5 +167,5 @@ def test_select_message_and_download_selected(self, missing_msg_file):
         self._journalist_logs_in()
         self._journalist_selects_the_first_source()
         self._journalist_selects_message_then_download_selected()
-        self._journalist_sees_missing_file_error_message()
+        self._journalist_sees_missing_file_error_message(single_file=True)
         self._journalist_is_on_collection_page()