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

app: ensure we do not keep references to a SourceWidget that has been deleted #887

Merged
merged 2 commits into from
Mar 9, 2020
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
3 changes: 3 additions & 0 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,10 @@ def update(self, sources: List[Source]):
current_source = self.get_current_source()
current_source_id = current_source and current_source.id

# When we call clear() to delete all SourceWidgets, we should
# also clear the source_widgets dict.
self.clear()
self.source_widgets = {}

for source in sources:
new_source = SourceWidget(source)
Expand Down
28 changes: 28 additions & 0 deletions tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,34 @@ def test_SourceList_update(mocker):
assert sl.setCurrentItem.call_count == 0


def test_SourceList_update_deleted_source(mocker):
"""
When the SourceList is updated after deleting a source, check that we
do not keep references to deleted source widgets on the source_widgets
dict.
"""
sl = SourceList()

sl.clear = mocker.MagicMock()
sl.addItem = mocker.MagicMock()
sl.setItemWidget = mocker.MagicMock()
sl.controller = mocker.MagicMock()
sl.setCurrentItem = mocker.MagicMock()

mock_sw = mocker.MagicMock()
mock_lwi = mocker.MagicMock()
mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw)
mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', mock_lwi)

sources = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock(), ]
sl.update(sources)

sources.pop()
sl.update(sources)

assert len(sl.source_widgets) == len(sources)


def test_SourceList_update_with_pre_selected_source(mocker):
"""
If there's a source already selected. It remains selected after update.
Expand Down