Refactor DeleteSourceDialog to allow it to accept a set of sources in…

…stead of a single source, and display an error message if the empty set iis provided. Add parameterized unit tests to test empty, n=1, and n>1 conditions.

client/securedrop_client/gui/actions.py

@@ -80,15 +80,17 @@ def __init__(
         source: Source,
         parent: QMenu,
         controller: Controller,
-        confirmation_dialog: Callable[[Source], QDialog],
+        confirmation_dialog: Callable[[set[Source]], QDialog],
     ) -> None:
         self.source = source
         self.controller = controller
         text = _("Delete Source Account")
 
         super().__init__(text, parent)
 
-        self._confirmation_dialog = confirmation_dialog(self.source)
+        # DeleteSource Dialog can accept more than one source (bulk delete),
+        # but when triggered from this menu, only applies to one source
+        self._confirmation_dialog = confirmation_dialog(set([self.source]))
         self._confirmation_dialog.accepted.connect(
             lambda: self.controller.delete_source(self.source)
         )
@@ -119,6 +121,7 @@ def __init__(
 
         super().__init__(text, parent)
 
+        # DeleteConversationDialog accepts only one source
         self._confirmation_dialog = confirmation_dialog(self.source)
         self._confirmation_dialog.accepted.connect(lambda: self._on_confirmation_dialog_accepted())
         self.triggered.connect(self.trigger)

client/securedrop_client/gui/source/delete/dialog.py

@@ -26,17 +26,27 @@
 class DeleteSourceDialog(ModalDialog):
     """Used to confirm deletion of source accounts."""
 
-    def __init__(self, source: Source) -> None:
+    def __init__(self, sources: set[Source]) -> None:
         super().__init__(show_header=False, dangerous=True)
+        self.sources = sources
 
-        self.source = source
+        # If the dialog is constructed with no sources, show a warning; otherwise,
+        # confirm the number and designation of the sources to be deleted
+        count = len(sources)
+        if count == 0:
+            self._show_warning_nothing_selected()
+        else:
+            if count == 1:
+                continue_text = _("YES, DELETE ENTIRE SOURCE ACCOUNT")
+            else:
+                continue_text = _("YES, DELETE %s SOURCE ACCOUNTS") % count
 
-        self.body.setText(self.make_body_text())
-        self.continue_button.setText(_("YES, DELETE ENTIRE SOURCE ACCOUNT"))
-        self.cancel_button.setDefault(True)
-        self.cancel_button.setFocus()
-        self.confirmation_label.setText(_("Are you sure this is what you want?"))
-        self.adjustSize()
+            self.body.setText(self.make_body_text())
+            self.continue_button.setText(continue_text)
+            self.cancel_button.setDefault(True)
+            self.cancel_button.setFocus()
+            self.confirmation_label.setText(_("Are you sure this is what you want?"))
+            self.adjustSize()
 
     def make_body_text(self) -> str:
         message_tuple = (
@@ -58,4 +68,23 @@ def make_body_text(self) -> str:
             "<p>&nbsp;</p>",
         )
 
-        return "".join(message_tuple).format(source=f"<b>{self.source.journalist_designation}</b>")
+        return "".join(message_tuple).format(
+            source=f"<b>{self._get_source_names(self.sources)}</b>"
+        )
+
+    def _get_source_names(self, sources: set[Source]) -> str:
+        """
+        Helper. Return a comma-separated list of journalist designations.
+        """
+        return ", ".join([s.journalist_designation for s in sources])
+
+    def _show_warning_nothing_selected(self) -> None:
+        """
+        Helper. Display warning if no sources are selected for deletion.
+        Disables "Continue" button so user must close or cancel dialog.
+        """
+        self.continue_button.setEnabled(False)
+        self.cancel_button.setFocus()
+        self.cancel_button.setDefault(True)
+        self.body.setText(_("No sources have been selected."))
+        self.adjustSize()

client/securedrop_client/locale/messages.pot

@@ -14,7 +14,7 @@ msgstr ""
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
-"Generated-By: Babel 2.15.0\n"
+"Generated-By: Babel 2.16.0\n"
 
 msgid "{application_name} is already running"
 msgstr ""
@@ -372,6 +372,10 @@ msgstr ""
 msgid "YES, DELETE ENTIRE SOURCE ACCOUNT"
 msgstr ""
 
+#, python-format
+msgid "YES, DELETE %s SOURCE ACCOUNTS"
+msgstr ""
+
 msgid "Are you sure this is what you want?"
 msgstr ""
 
@@ -387,3 +391,6 @@ msgstr ""
 msgid "All files and messages from that source will also be destroyed."
 msgstr ""
 
+msgid "No sources have been selected."
+msgstr ""
+

client/tests/gui/source/delete/test_dialog.py

@@ -1,22 +1,81 @@
-import unittest
+import pytest
 
-from securedrop_client.gui.source.delete import DeleteSourceDialog as Dialog
+from securedrop_client.gui.source.delete import DeleteSourceDialog
 from tests import factory
 
 
-class DeleteSourceDialogTest(unittest.TestCase):
-    def setUp(self):
-        self._source = factory.Source()
-        factory.File(source=self._source)
-        self.dialog = Dialog(self._source)
+@pytest.fixture(
+    params=[set(), set([factory.Source()]), set([factory.Source(), factory.Source()])],
+)
+def dialog(request):
+    """
+    Set up the dialog under various conditions: 0 sources, 1 source, >1 source.
+    Tests that target a specific configuration can use decorators to skip unwanted
+    conditions.
+    """
+    # Give the source(s) a submission
+    for source in request.param:
+        factory.File(source=source)
+    return DeleteSourceDialog(request.param)
 
-    def test_default_button_is_safer_choice(self):
+
+class TestDeleteSourceDialog:
+    def test_dialog_setup(self, dialog):
+        assert type(dialog) is DeleteSourceDialog
+        assert type(dialog.sources) is set
+        assert dialog.dangerous
+
+    def test_default_button_is_safer_choice(self, dialog):
         # This test does rely on an implementation detail (the buttons)
         # but I couldn't find a way to test this properly using key events.
-        assert not self.dialog.continue_button.isDefault()
-        assert self.dialog.cancel_button.isDefault()
+        assert not dialog.continue_button.isDefault()
+        assert dialog.cancel_button.isDefault()
+
+    def test_displays_important_information_when_shown(self, dialog):
+        if len(dialog.sources) < 1:
+            pytest.skip("Skip if no sources")
+        assert "not be able to send them replies" in dialog.text()
+        assert "source will not be able to log in" in dialog.text()
+        assert "files and messages from that source will also be destroyed" in dialog.text()
+
+    def test_dialog_continue_button_adapts_to_source_count(self, dialog):
+        count = len(dialog.sources)
+
+        if count < 1:
+            pytest.skip("Skip if no sources")
+
+        if count == 1:
+            assert dialog.continue_button.text() == "YES, DELETE ENTIRE SOURCE ACCOUNT"
+        elif count > 1:
+            assert dialog.continue_button.text() == f"YES, DELETE {count} SOURCE ACCOUNTS"
+        else:
+            # should be unreachable due to decorator
+            pytest.fail("Unreachable, or so we thought")
+
+    def test_no_sources_shows_error_text(self, dialog):
+        if len(dialog.sources) > 0:
+            pytest.skip("Skip if sources")
+
+        assert dialog.text() == "No sources have been selected."
+
+    def test_no_sources_continue_button_disabled(self, dialog):
+        if len(dialog.sources) > 0:
+            pytest.skip("Skip if sources")
+
+        assert not dialog.continue_button.isEnabled()
+
+    @pytest.mark.parametrize(
+        "names", [["source one"], ["source one", "source two", "source three"]]
+    )
+    def test_correct_format_body_text(self, dialog, names):
+        """
+        For n > 1 sources, ensure the warning text displays
+        all the journalist desginators.
+        """
+        sources = set()
+
+        for item in names:
+            source = factory.Source(journalist_designation=item)
+            sources.update([source])
 
-    def test_displays_important_information_when_shown(self):
-        assert "not be able to send them replies" in self.dialog.text()
-        assert "source will not be able to log in" in self.dialog.text()
-        assert "files and messages from that source will also be destroyed" in self.dialog.text()
+        assert all(n in dialog._get_source_names(sources) for n in names)

client/tests/gui/test_widgets.py

@@ -4353,7 +4353,7 @@ def test_ConversationView_add_not_downloaded_file(mocker, homedir, source, sessi
 def test_DeleteSource_from_source_menu_when_user_is_loggedout(mocker):
     mock_controller = mocker.MagicMock()
     mock_controller.api = None
-    mock_source = mocker.MagicMock()
+    mock_source = factory.Source()
     mock_delete_source_dialog_instance = mocker.MagicMock(DeleteSourceDialog)
     mock_delete_source_dialog = mocker.MagicMock()
     mock_delete_source_dialog.return_value = mock_delete_source_dialog_instance