Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning when all sources are selected for deletion #2300

Merged
merged 5 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions client/securedrop_client/gui/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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])
)
Expand Down
37 changes: 26 additions & 11 deletions client/securedrop_client/gui/source/delete/dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! good catch.


# 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()
Expand Down Expand Up @@ -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>")
zenmonkeykstop marked this conversation as resolved.
Show resolved Hide resolved
else:
all_sources_text = ("", "", "")

message_text = (
"<p>",
_("Delete entire account for: {source_or_sources}?"),
"</p>",
Expand All @@ -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>"
)

Expand Down
3 changes: 2 additions & 1 deletion client/securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions client/securedrop_client/locale/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""

Expand Down
6 changes: 6 additions & 0 deletions client/securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
50 changes: 42 additions & 8 deletions client/tests/gui/source/delete/test_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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))
4 changes: 2 additions & 2 deletions client/tests/gui/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 36 additions & 9 deletions client/tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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())
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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"
Expand Down
Loading