Merge pull request #2300 from freedomofpress/2298-warn-on-select-all

Add warning when all sources are selected for deletion

client/securedrop_client/gui/actions.py

@@ -81,7 +81,7 @@ def __init__(
         source: Source,
         parent: QMenu,
         controller: Controller,
-        confirmation_dialog: Callable[[list[Source]], QDialog],
+        confirmation_dialog: Callable[[list[Source], int], QDialog],
     ) -> None:
         self.source = source
         self.controller = controller
@@ -91,7 +91,10 @@ def __init__(
 
         # 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([self.source])
+        self._confirmation_dialog = confirmation_dialog(
+            [self.source],
+            self.controller.get_source_count(),
+        )
         self._confirmation_dialog.accepted.connect(
             lambda: self.controller.delete_sources([self.source])
         )

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

@@ -37,23 +37,32 @@
 class DeleteSourceDialog(ModalDialog):
     """Used to confirm deletion of source accounts."""
 
-    def __init__(self, sources: list[Source]) -> None:
+    def __init__(self, sources: list[Source], source_total: int) -> None:
         super().__init__(show_header=False, dangerous=True)
         self.sources = sources
+        self.source_total = source_total
+        self.continue_text = "CONTINUE"
 
         # If the dialog is constructed with no sources, show a warning; otherwise,
         # confirm the number and designation of the sources to be deleted
         num_sources = len(sources)
         if num_sources == 0:
             self._show_warning_nothing_selected()
         else:
-            self.continue_text = ngettext(
-                "YES, DELETE ENTIRE SOURCE ACCOUNT",
-                "YES, DELETE {number} SOURCE ACCOUNTS",
-                num_sources,
-            ).format(number=num_sources)
-
-            self.body.setText(self.make_body_text(self.sources))
+            if num_sources < source_total:
+                self.continue_text = ngettext(
+                    "YES, DELETE ENTIRE SOURCE ACCOUNT",
+                    "YES, DELETE {number} SOURCE ACCOUNTS",
+                    num_sources,
+                ).format(number=num_sources)
+            else:
+                self.continue_text = ngettext(
+                    "YES, DELETE ENTIRE SOURCE ACCOUNT",  # in this case, all 1 accounts.
+                    "YES, DELETE ALL {number} SOURCE ACCOUNTS",
+                    num_sources,
+                ).format(number=num_sources)
+
+            self.body.setText(self.make_body_text(self.sources, self.source_total))
             self.continue_button.setText(self.continue_text)
             self.cancel_button.setDefault(True)
             self.cancel_button.setFocus()
@@ -96,8 +105,13 @@ def update_continue_button(self, initial: bool = False) -> None:
             self.continue_button.setText(self.continue_text)
             self.continue_button.setEnabled(True)
 
-    def make_body_text(self, sources: list[Source]) -> str:
-        message_tuple = (
+    def make_body_text(self, sources: list[Source], source_total: int) -> str:
+        if len(sources) == source_total:
+            all_sources_text = ("<p><b>", _("Notice: All sources have been selected!"), "</p></b>")
+        else:
+            all_sources_text = ("", "", "")
+
+        message_text = (
             "<p>",
             _("Delete entire account for: {source_or_sources}?"),
             "</p>",
@@ -116,7 +130,8 @@ def make_body_text(self, sources: list[Source]) -> str:
             "<p>&nbsp;</p>",
         )
 
-        return "".join(message_tuple).format(
+        full_text = all_sources_text + message_text
+        return "".join(full_text).format(
             source_or_sources=f"<b>{self._get_source_names_truncated(sources, LOTS_OF_SOURCES)}</b>"
         )
 

client/securedrop_client/gui/widgets.py

@@ -553,8 +553,9 @@ def on_action_triggered(self) -> None:
             # The current source selection is continuously received by the controller
             # as the user selects and deselects; here we retrieve the selection
             targets = self.controller.get_selected_sources()
+            source_count = self.controller.get_source_count()
             if targets is not None:
-                dialog = DeleteSourceDialog(targets)
+                dialog = DeleteSourceDialog(targets, source_count)
                 self._last_dialog = dialog  # FIXME: workaround for #2273
                 dialog.accepted.connect(lambda: self.controller.delete_sources(targets))
                 dialog.open()

client/securedrop_client/locale/messages.pot

@@ -398,6 +398,9 @@ msgstr ""
 msgid "{text} (wait {delay} sec)"
 msgstr ""
 
+msgid "Notice: All sources have been selected!"
+msgstr ""
+
 msgid "Delete entire account for: {source_or_sources}?"
 msgstr ""
 

client/securedrop_client/logic.py

@@ -1215,3 +1215,9 @@ def on_receive_selected_sources(self, sources: list[db.Source]) -> None:
 
     def get_selected_sources(self) -> list[db.Source] | None:
         return self._selected_sources
+
+    def get_source_count(self) -> int:
+        """
+        Return total sources in local storage.
+        """
+        return self.session.query(db.Source).count()

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

@@ -16,7 +16,7 @@ def dialog(request):
     # Give the source(s) a submission
     for source in request.param:
         factory.File(source=source)
-    return DeleteSourceDialog(request.param)
+    return DeleteSourceDialog(request.param, len(request.param) + 1)
 
 
 class TestDeleteSourceDialog:
@@ -68,24 +68,58 @@ def test_no_sources_continue_button_not_shown(self, dialog):
     def test_correct_format_body_text(self):
         """
         For n > 1 sources, ensure the warning text includes
-        all the journalist desginators.
+        all the journalist designators.
         """
         sources = []
         names = [
             "source one",
             "source two",
             "source three",
-            "source four",
-            "source five",
-            "source six",
-            "source seven",
         ]
 
         for item in names:
             source = factory.Source(journalist_designation=item)
             sources.append(source)
+            # pretend we've selected all but one source
+            fake_total = len(sources) + 1
 
-        dialog = DeleteSourceDialog(sources)
+        dialog = DeleteSourceDialog(sources, fake_total)
+        dialog_text = dialog.make_body_text(sources, fake_total)
 
+        assert "All sources have been selected" not in dialog_text
         for n in names:
-            assert n in dialog.make_body_text(sources)
+            assert n in dialog_text
+
+    def test_correct_format_body_text_all_selected(self):
+        """
+        Ensure that warning has been added when all sources selected
+        """
+        sources = []
+        names = [
+            "source one",
+            "source two",
+            "source three",
+        ]
+
+        for item in names:
+            source = factory.Source(journalist_designation=item)
+            sources.append(source)
+
+        dialog = DeleteSourceDialog(sources, len(sources))
+
+        assert "All sources have been selected" in dialog.make_body_text(sources, len(sources))
+
+    def test_correct_format_body_text_truncated(self):
+        """
+        Ensure that source list is truncated correctly when over display limit
+        """
+        sources = []
+        names = [f"source_{i}" for i in range(1, 46)]
+
+        for item in names:
+            source = factory.Source(journalist_designation=item)
+            sources.append(source)
+
+        dialog = DeleteSourceDialog(sources, len(sources))
+
+        assert "plus 15 additional sources" in dialog.make_body_text(sources, len(sources))

client/tests/gui/test_actions.py

@@ -45,7 +45,7 @@ def test_deletes_conversation_when_dialog_accepted(self):
 
         self.action.trigger()
 
-        self._controller.delete_conversation.assert_called_once_with(self._source)
+        (self._controller.delete_conversation.assert_called_once_with(self._source),)
         self._app_state.remove_conversation_files.assert_called_once_with(
             state.ConversationId("some_conversation")
         )
@@ -91,7 +91,7 @@ def setUp(self):
         self._controller = MagicMock(Controller, api=True)
         self._dialog = QDialog()
 
-        def _dialog_constructor(source: Source) -> QDialog:
+        def _dialog_constructor(source: Source, source_total: int) -> QDialog:
             return self._dialog
 
         self.action = DeleteSourceAction(self._source, _menu, self._controller, _dialog_constructor)

client/tests/gui/test_widgets.py

@@ -606,7 +606,9 @@ def test_MainView_delete_conversation_when_conv_wrapper_exists(mocker):
     Ensure SourceConversationWrapper is deleted if it exists.
     """
     source = factory.Source(uuid="123")
-    conversation_wrapper = SourceConversationWrapper(source, mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 2
+    conversation_wrapper = SourceConversationWrapper(source, controller)
     conversation_wrapper.deleteLater = mocker.MagicMock()
     mv = MainView(None)
     mv.source_conversations = {}
@@ -644,6 +646,7 @@ def test_MainView_on_source_changed(mocker):
     mv.source_list.selectedItems = mocker.MagicMock(return_value=[source])
     mv.source_list.count = mocker.MagicMock(return_value=3)
     mv.controller = mocker.MagicMock(is_authenticated=True)
+    mv.controller.get_source_count.return_value = 3
     mocker.patch("securedrop_client.gui.widgets.source_exists", return_value=True)
     mv.on_source_changed()
 
@@ -755,6 +758,7 @@ def test_MainView_on_source_changed_updates_conversation_view(mocker, session):
     mv.source_list.selectedItems = mocker.MagicMock(return_value=[source])
     mv.source_list.get_selected_source = mocker.MagicMock(return_value=source)
     mv.controller = mocker.MagicMock(is_authenticated=True)
+    mv.controller.get_source_count.return_value = 1
     session.add(source)
     file = factory.File(source=source, filename="0-mock-doc.gpg")
     message = factory.Message(source=source, filename="0-mock-msg.gpg")
@@ -811,6 +815,8 @@ def test_MainView_on_source_changed_SourceConversationWrapper_is_preserved(mocke
 
     mv.controller = mocker.MagicMock(is_authenticated=True)
 
+    mv.controller.get_source_count.return_value = 2
+
     mv.on_source_changed()
     assert mv.set_conversation.call_count == 1
     assert source.uuid in mv.source_conversations
@@ -990,7 +996,9 @@ def test_MainView_set_conversation(mocker):
     """
     mv = MainView(None)
 
-    scw = SourceConversationWrapper(factory.Source(), mocker.MagicMock())
+    mv_controller = mocker.MagicMock()
+    mv_controller.get_source_count.return_value = 2
+    scw = SourceConversationWrapper(factory.Source(), mv_controller)
     mv.set_conversation(scw)
 
     assert mv.view_layout.widget(mv.CONVERSATION_INDEX) == scw
@@ -3856,6 +3864,7 @@ def test_SourceConversationWrapper_on_conversation_updated(mocker, qtbot):
 
     get_file = mocker.MagicMock(return_value=file)
     controller = mocker.MagicMock(get_file=get_file)
+    controller.get_source_count.return_value = 1
 
     scw = SourceConversationWrapper(source, controller, None)
     scw.conversation_title_bar.updated.setText("CANARY")
@@ -3876,6 +3885,7 @@ def test_SourceConversationWrapper_on_source_deleted(mocker):
     mv.source_list = mocker.MagicMock()
     mv.source_list.get_selected_source = mocker.MagicMock(return_value=source)
     mv.controller = mocker.MagicMock(is_authenticated=True)
+    mv.controller.get_source_count.return_value = 1
 
     # Detached sourceconversationwrapper, just for unit testing
     scw = SourceConversationWrapper(source, mv.controller, None)
@@ -3892,7 +3902,9 @@ def test_SourceConversationWrapper_on_source_deleted(mocker):
 
 
 def test_SourceConversationWrapper_on_source_deleted_wrong_uuid(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_source_deleted("321")
     assert not scw.conversation_title_bar.isHidden()
     assert not scw.conversation_view.isHidden()
@@ -3901,7 +3913,9 @@ def test_SourceConversationWrapper_on_source_deleted_wrong_uuid(mocker):
 
 
 def test_SourceConversationWrapper_on_source_deletion_failed(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_source_deleted("123")
 
     scw.on_source_deletion_failed("123")
@@ -3913,7 +3927,9 @@ def test_SourceConversationWrapper_on_source_deletion_failed(mocker):
 
 
 def test_SourceConversationWrapper_on_source_deletion_failed_wrong_uuid(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_source_deleted("123")
 
     scw.on_source_deletion_failed("321")
@@ -3930,6 +3946,7 @@ def test_SourceConversationWrapper_on_conversation_deleted(mocker):
     mv.source_list = mocker.MagicMock()
     mv.source_list.get_selected_source = mocker.MagicMock(return_value=source)
     mv.controller = mocker.MagicMock(is_authenticated=True)
+    mv.controller.get_source_count.return_value = 1
     mocker.patch("securedrop_client.gui.widgets.source_exists", return_value=True)
     mv.show()
     scw = SourceConversationWrapper(source, mv.controller, None)
@@ -3948,7 +3965,9 @@ def test_SourceConversationWrapper_on_conversation_deleted(mocker):
 
 
 def test_SourceConversationWrapper_on_conversation_deleted_wrong_uuid(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_conversation_deleted("321")
     assert not scw.conversation_title_bar.isHidden()
     assert not scw.conversation_view.isHidden()
@@ -3958,7 +3977,9 @@ def test_SourceConversationWrapper_on_conversation_deleted_wrong_uuid(mocker):
 
 
 def test_SourceConversationWrapper__on_conversation_deletion_successful(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_conversation_deleted("123")
 
     scw._on_conversation_deletion_successful("123", datetime.now())
@@ -3971,7 +3992,9 @@ def test_SourceConversationWrapper__on_conversation_deletion_successful(mocker):
 
 
 def test_SourceConversationWrapper_on_conversation_deletion_failed(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_conversation_deleted("123")
 
     scw.on_conversation_deletion_failed("123")
@@ -3984,7 +4007,9 @@ def test_SourceConversationWrapper_on_conversation_deletion_failed(mocker):
 
 
 def test_SourceConversationWrapper_on_conversation_deletion_failed_wrong_uuid(mocker):
-    scw = SourceConversationWrapper(factory.Source(uuid="123"), mocker.MagicMock())
+    controller = mocker.MagicMock()
+    controller.get_source_count.return_value = 1
+    scw = SourceConversationWrapper(factory.Source(uuid="123"), controller)
     scw.on_conversation_deleted("123")
 
     scw.on_conversation_deletion_failed("321")
@@ -4453,6 +4478,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_controller.get_source_count.return_value = 1
     mock_source = factory.Source()
     mock_delete_source_dialog_instance = mocker.MagicMock(DeleteSourceDialog)
     mock_delete_source_dialog = mocker.MagicMock()
@@ -5326,6 +5352,7 @@ def test_SourceProfileShortWidget_update_timestamp(mocker):
     instance with the last_updated value from the source..
     """
     mock_controller = mocker.MagicMock()
+    mock_controller.get_source_count.return_value = 1
     mock_source = mocker.MagicMock()
     mock_source.last_updated = datetime.now()
     mock_source.journalist_designation = "wimple horse knackered unittest"