Skip to content

Commit

Permalink
Address deletion condition when accessing orm object attributes in co…
Browse files Browse the repository at this point in the history
…ntroller delete_sources. Ensure DeleteSourceDialog is shown even with no sources selected in widgets.py and improve comments.

Fix new FileWidget functional test.
  • Loading branch information
rocodes authored and cfm committed Oct 25, 2024
1 parent 41b4299 commit c267ce7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
5 changes: 4 additions & 1 deletion client/securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ 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()
if targets:
if targets is not None:
dialog = DeleteSourceDialog(targets)
dialog.accepted.connect(lambda: self.controller.delete_sources(targets))
dialog.exec()
Expand Down Expand Up @@ -904,6 +904,8 @@ def on_source_changed(self) -> None:
# One source selected; prepare the conversation widget
try:
source = self.source_list.get_selected_source()

# Avoid race between user selection and remote deletion
if not source:
return

Expand Down Expand Up @@ -1299,6 +1301,7 @@ def schedule_source_management(slice_size: int = slice_size) -> None:
QTimer.singleShot(1, schedule_source_management)

def get_selected_source(self) -> Optional[Source]:
# if len == 0, return None
if not self.selectedItems():
return None

Expand Down
12 changes: 11 additions & 1 deletion client/securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,17 @@ def delete_sources(self, sources: list[db.Source]) -> None:
the failure handler will display an error.
"""
for source in sources:
job = DeleteSourceJob(source.uuid)
try:
# Accessing source.uuid requires the source object to be
# present in the database.
# To avoid passing and referencing orm objects, a simplified
# ViewModel layer decoupled from the db that presents data to the API/GUI
# would be another possibility.
job = DeleteSourceJob(source.uuid)
except sqlalchemy.orm.exc.ObjectDeletedError:
logger.warning("DeleteSourceJob requested but source already deleted")
return

job.success_signal.connect(self.on_delete_source_success)
job.failure_signal.connect(self.on_delete_source_failure)

Expand Down
6 changes: 3 additions & 3 deletions client/tests/functional/test_deleted_file_filewidget.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ def check_for_sources():
qtbot.wait(TIME_CLICK_ACTION)

def conversation_with_file_is_rendered():
assert gui.main_view.view_layout.itemAt(0)
conversation = gui.main_view.view_layout.itemAt(0).widget()
assert gui.main_view.view_layout.currentIndex() == gui.main_view.CONVERSATION_INDEX
conversation = gui.main_view.view_layout.widget(gui.main_view.view_layout.currentIndex())
assert isinstance(conversation, SourceConversationWrapper)
file_id = list(conversation.conversation_view.current_messages.keys())[2]
file_widget = conversation.conversation_view.current_messages[file_id]
assert isinstance(file_widget, FileWidget)

# Get the selected source conversation that contains a file attachment
qtbot.waitUntil(conversation_with_file_is_rendered, timeout=TIME_RENDER_CONV_VIEW)
conversation = gui.main_view.view_layout.itemAt(0).widget()
conversation = gui.main_view.view_layout.widget(gui.main_view.view_layout.currentIndex())
file_id = list(conversation.conversation_view.current_messages.keys())[2]
file_widget = conversation.conversation_view.current_messages[file_id]

Expand Down

0 comments on commit c267ce7

Please sign in to comment.