From c5721a1789995446f635fbea57516f23674f1dae Mon Sep 17 00:00:00 2001 From: Tim Ruffles Date: Sun, 11 Nov 2018 16:25:35 -0800 Subject: [PATCH 1/4] Show a summarised snippet of the latest reply from each thread --- securedrop_client/gui/widgets.py | 4 ++++ securedrop_client/models.py | 22 ++++++++++++++++++++-- tests/gui/test_widgets.py | 18 +++++++++++------- tests/test_logic.py | 3 +-- tests/test_models.py | 22 ++++++++++++++++++++++ 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 12b95f5dd..5500d3d91 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -206,10 +206,12 @@ def __init__(self, parent, source): self.attached = load_svg('paperclip.svg') self.attached.setMaximumSize(16, 16) self.name = QLabel() + self.last_content = QLabel() self.summary_layout.addWidget(self.name) self.summary_layout.addStretch() self.summary_layout.addWidget(self.attached) layout.addWidget(self.summary) + layout.addWidget(self.last_content) self.updated = QLabel() layout.addWidget(self.updated) self.update() @@ -248,6 +250,8 @@ def update(self): if self.source.document_count == 0: self.attached.hide() + self.last_content.setText(self.source.last_activity_summary_text) + def toggle_star(self, event): """ Called when the star is clicked. diff --git a/securedrop_client/models.py b/securedrop_client/models.py index 911a14c60..a7f7720a7 100644 --- a/securedrop_client/models.py +++ b/securedrop_client/models.py @@ -26,9 +26,11 @@ def make_engine(home: str): class WithContent(): + data = None + @property def content(self): - if self.is_downloaded: + if self.is_downloaded and self.data: fn_no_ext, _ = os.path.splitext(self.filename) return self.data.get(fn_no_ext) else: @@ -74,6 +76,23 @@ def collection(self): collection.sort(key=lambda x: int(x.filename.split('-')[0])) return collection + @property + def last_activity_summary_text(self): + if len(self.collection) == 0: + return '' + + def ellipsis(content, n): + if len(content) <= n: + return content + else: + return '{}…'.format(content[:n]) + + last = self.collection[-1] + + prefix = '↳' if isinstance(last, Submission) else '' + content = last.content or '' + return '{}{}'.format(prefix, ellipsis(content, 100)) + class Submission(Base, WithContent): __tablename__ = 'submissions' @@ -98,7 +117,6 @@ class Submission(Base, WithContent): def __init__(self, source, uuid, size, filename, download_url): # ORM event catching _should_ have already initialized `self.data` - self.source_id = source.id self.uuid = uuid self.size = size diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index d70a3461d..e7279885b 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -166,11 +166,15 @@ def test_SourceList_update(): assert sl.setItemWidget.call_count == len(sources) +def MockSource(): + return mock.MagicMock(last_activity_summary_text='') + + def test_SourceWidget_init(): """ The source widget is initialised with the passed-in source. """ - mock_source = mock.MagicMock() + mock_source = MockSource() mock_source.journalist_designation = 'foo bar baz' sw = SourceWidget(None, mock_source) assert sw.source == mock_source @@ -181,7 +185,7 @@ def test_SourceWidget_setup(): The setup method adds the controller as an attribute on the SourceWidget. """ mock_controller = mock.MagicMock() - mock_source = mock.MagicMock() + mock_source = MockSource() sw = SourceWidget(None, mock_source) sw.setup(mock_controller) assert sw.controller == mock_controller @@ -191,7 +195,7 @@ def test_SourceWidget_update_starred(): """ Ensure the widget displays the expected details from the source. """ - mock_source = mock.MagicMock() + mock_source = MockSource() mock_source.journalist_designation = 'foo bar baz' mock_source.is_starred = True sw = SourceWidget(None, mock_source) @@ -207,7 +211,7 @@ def test_SourceWidget_update_unstarred(): """ Ensure the widget displays the expected details from the source. """ - mock_source = mock.MagicMock() + mock_source = MockSource() mock_source.journalist_designation = 'foo bar baz' mock_source.is_starred = False sw = SourceWidget(None, mock_source) @@ -240,7 +244,7 @@ def test_SourceWidget_toggle_star(): The toggle_star method should call self.controller.update_star """ mock_controller = mock.MagicMock() - mock_source = mock.MagicMock() + mock_source = MockSource() event = mock.MagicMock() sw = SourceWidget(None, mock_source) sw.controller = mock_controller @@ -530,7 +534,7 @@ def test_ConversationView_add_downloaded_file(): cv = ConversationView(None) cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() - mock_source = mock.MagicMock() + mock_source = MockSource() mock_file = mock.MagicMock() mock_file.is_downloaded = True with mock.patch('securedrop_client.gui.widgets.QLabel') as mock_label, \ @@ -551,7 +555,7 @@ def test_ConversationView_add_not_downloaded_file(): cv = ConversationView(None) cv.controller = mock.MagicMock() cv.conversation_layout = mock.MagicMock() - mock_source = mock.MagicMock() + mock_source = MockSource() mock_file = mock.MagicMock() mock_file.is_downloaded = False mock_file.size = 123 diff --git a/tests/test_logic.py b/tests/test_logic.py index 1cd72171b..6b6996b37 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -807,8 +807,7 @@ def test_Client_on_file_download_user_not_signed_in(safe_tmpdir): mock_gui = mock.MagicMock() mock_session = mock.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir)) - source = models.Source('source-uuid', 'testy-mctestface', False, - 'mah pub key', 1, False, datetime.now()) + source = factory.Source() submission = models.Submission(source, 'submission-uuid', 1234, 'myfile.doc.gpg', 'http://myserver/myfile') cl.on_action_requiring_login = mock.MagicMock() diff --git a/tests/test_models.py b/tests/test_models.py index 24bed8087..93dc2a700 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -82,3 +82,25 @@ def test_source_collection(): # Now these items should be in the source collection in the proper order assert source.collection[0] == reply assert source.collection[1] == submission + + +def test_last_activity_summary_text(): + source = factory.Source() + submission = Submission(source=source, uuid="test", size=123, + filename="2-test.doc.gpg", + download_url='http://test/test') + user = User('hehe') + reply = Reply(source=source, journalist=user, filename="1-reply.gpg", + size=1234, uuid='test') + + reply_with_content = mock.MagicMock(wrap=reply, content='reply content') + submission_content = mock.MagicMock(wrap=submission, content='submission content') + + source.submissions = [reply_with_content] + source.replies = [submission_content] + + assert source.last_activity_summary_text == 'submission content' + + submission_content.content = 'extremely long content' * 50 + assert source.last_activity_summary_text[-1] == '…' + assert len(source.last_activity_summary_text) < len(submission_content.content) From f60ef61145217e529fb262c1fb356136a21593c9 Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Tue, 13 Nov 2018 13:16:03 -0800 Subject: [PATCH 2/4] Escape HTML in summary --- securedrop_client/gui/widgets.py | 4 +++- securedrop_client/models.py | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index 6f05e6ad0..f23793102 100644 --- a/securedrop_client/gui/widgets.py +++ b/securedrop_client/gui/widgets.py @@ -258,7 +258,9 @@ def update(self): if self.source.document_count == 0: self.attached.hide() - self.last_content.setText(self.source.last_activity_summary_text) + self.last_content.setText( + "{}".format(html.escape(self.source.last_activity_summary_text)) + ) def toggle_star(self, event): """ diff --git a/securedrop_client/models.py b/securedrop_client/models.py index a7f7720a7..c3cdf4ae8 100644 --- a/securedrop_client/models.py +++ b/securedrop_client/models.py @@ -1,3 +1,5 @@ +# coding: utf-8 + import os from sqlalchemy import Boolean, Column, create_engine, DateTime, ForeignKey, Integer, String, \ @@ -91,6 +93,7 @@ def ellipsis(content, n): prefix = '↳' if isinstance(last, Submission) else '' content = last.content or '' + return '{}{}'.format(prefix, ellipsis(content, 100)) From 13999b40ccb3cd5cea2833dbf8f26d74cdcb846a Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Mon, 10 Dec 2018 21:33:38 -0800 Subject: [PATCH 3/4] fixes tests --- tests/gui/test_widgets.py | 42 +++++++++------------------------------ tests/test_models.py | 6 +++--- 2 files changed, 12 insertions(+), 36 deletions(-) diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index b7f22a368..1562b45d9 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -149,15 +149,15 @@ def test_SourceList_update(mocker): each passed-in source is created along with an associated QListWidgetItem. """ sl = SourceList(None) - sl.clear = mock.MagicMock() - sl.addItem = mock.MagicMock() - sl.setItemWidget = mock.MagicMock() - sl.controller = mock.MagicMock() - mock_sw = mock.MagicMock() - mock_lwi = mock.MagicMock() - with mock.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw), \ - mock.patch('securedrop_client.gui.widgets.QListWidgetItem', - mock_lwi): + sl.clear = mocker.MagicMock() + sl.addItem = mocker.MagicMock() + sl.setItemWidget = mocker.MagicMock() + sl.controller = mocker.MagicMock() + mock_sw = mocker.MagicMock() + mock_lwi = mocker.MagicMock() + with mocker.patch('securedrop_client.gui.widgets.SourceWidget', mock_sw), \ + mocker.patch('securedrop_client.gui.widgets.QListWidgetItem', + mock_lwi): sources = ['a', 'b', 'c', ] sl.update(sources) sl.clear.assert_called_once_with() @@ -167,30 +167,6 @@ def test_SourceList_update(mocker): assert sl.setItemWidget.call_count == len(sources) -def MockSource(): - return mock.MagicMock(last_activity_summary_text='') - - -def test_SourceList_maintains_selection(): - sl.clear = mocker.MagicMock() - sl.addItem = mocker.MagicMock() - sl.setItemWidget = mocker.MagicMock() - sl.controller = 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 = ['a', 'b', 'c', ] - sl.update(sources) - sl.clear.assert_called_once_with() - assert mock_sw.call_count == len(sources) - assert mock_lwi.call_count == len(sources) - assert sl.addItem.call_count == len(sources) - assert sl.setItemWidget.call_count == len(sources) - - def test_SourceList_maintains_selection(mocker): """ Maintains the selected item if present in new list diff --git a/tests/test_models.py b/tests/test_models.py index f3ffe50f2..3b4b6a88b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -86,7 +86,7 @@ def test_source_collection(): assert source.collection[1] == submission -def test_last_activity_summary_text(): +def test_last_activity_summary_text(mocker): source = factory.Source() submission = Submission(source=source, uuid="test", size=123, filename="2-test.doc.gpg", @@ -95,8 +95,8 @@ def test_last_activity_summary_text(): reply = Reply(source=source, journalist=user, filename="1-reply.gpg", size=1234, uuid='test') - reply_with_content = mock.MagicMock(wrap=reply, content='reply content') - submission_content = mock.MagicMock(wrap=submission, content='submission content') + reply_with_content = mocker.MagicMock(wrap=reply, content='reply content') + submission_content = mocker.MagicMock(wrap=submission, content='submission content') source.submissions = [reply_with_content] source.replies = [submission_content] From e0ac81887aa4478a6f90ad4b35a24c85834fbfae Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Fri, 14 Dec 2018 10:15:24 -0800 Subject: [PATCH 4/4] WIP update source view --- securedrop_client/db.py | 4 +++- securedrop_client/logic.py | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/securedrop_client/db.py b/securedrop_client/db.py index 688497d04..ec53da2ef 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -79,8 +79,10 @@ def collection(self): collection.sort(key=lambda x: int(x.filename.split('-')[0])) return collection + # XXX jt you are here, fix these. @property def last_activity_summary_text(self): + print("Last activity summary text!", self.collection) if len(self.collection) == 0: return '' @@ -91,7 +93,7 @@ def ellipsis(content, n): return '{}…'.format(content[:n]) last = self.collection[-1] - + print("Last", last, "content", last.content) prefix = '↳' if isinstance(last, Submission) else '' content = last.content or '' diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index b49bf1500..20c11a64d 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -144,8 +144,8 @@ def setup(self): # that as downloads/decryption occur, the messages and replies # populate the view. self.conv_view_update = QTimer() - self.conv_view_update.timeout.connect( - self.update_conversation_view) + self.conv_view_update.timeout.connect(self.update_views) + self.conv_view_update.start(1000 * 60 * 0.10) # every 6 seconds event.listen(db.Submission, 'load', self.on_object_loaded) @@ -153,6 +153,10 @@ def setup(self): event.listen(db.Reply, 'load', self.on_object_loaded) event.listen(db.Reply, 'init', self.on_object_instantiated) + def update_views(self): + self.update_sources() + self.update_conversation_view() + def on_object_instantiated(self, target, args, kwargs): target.data = Data(self.data_dir) return target @@ -581,6 +585,7 @@ def on_file_downloaded(self, result, current_object): # Refresh the current source conversation, bearing in mind # that the user may have navigated to another source. self.gui.show_conversation_for(self.gui.current_source) + # self.update_sources() self.set_status( 'Finished downloading {}'.format(current_object.filename)) else: # The file did not download properly.