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.
  • Loading branch information
rocodes committed Oct 25, 2024
1 parent 01e3fad commit 30b6544
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 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 @@ -1035,7 +1035,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

0 comments on commit 30b6544

Please sign in to comment.