Skip to content

Commit

Permalink
Remove old way of checking deletion timestamp
Browse files Browse the repository at this point in the history
Signed-off-by: Allie Crevier <[email protected]>
  • Loading branch information
Allie Crevier committed Mar 10, 2022
1 parent 4a4e672 commit 3aeef6e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 86 deletions.
38 changes: 12 additions & 26 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1329,11 +1329,6 @@ def set_snippet(
if self.deleting or self.deleting_conversation:
return

# If the sync started before the deletion finished, then the sync is stale and we do
# not want to update the source widget.
if self.sync_started_timestamp < self.deletion_scheduled_timestamp:
return

# If the source collection is empty yet the interaction_count is greater than zero, then we
# known that the conversation has been deleted.
if not self.source.server_collection:
Expand Down Expand Up @@ -2944,18 +2939,15 @@ def _on_conversation_deletion_successful(self, source_uuid: str, timestamp: date
logger.debug(f"Could not update ConversationView: {e}")

def update_deletion_markers(self) -> None:
try:
if self.source.collection:
self.scroll.show()
if self.source.collection[0].file_counter > 1:
self.deleted_conversation_marker.hide()
self.deleted_conversation_items_marker.show()
elif self.source.interaction_count > 0:
self.scroll.hide()
self.deleted_conversation_items_marker.hide()
self.deleted_conversation_marker.show()
except sqlalchemy.exc.InvalidRequestError as e:
logger.debug(f"Could not Update deletion markers in the ConversationView: {e}")
if self.source.collection:
self.scroll.show()
if self.source.collection[0].file_counter > 1:
self.deleted_conversation_marker.hide()
self.deleted_conversation_items_marker.show()
elif self.source.interaction_count > 0:
self.scroll.hide()
self.deleted_conversation_items_marker.hide()
self.deleted_conversation_marker.show()

def update_conversation(self, collection: list) -> None:
"""
Expand All @@ -2976,11 +2968,6 @@ def update_conversation(self, collection: list) -> None:
passed into this method in case of a mismatch between where the widget
has been and now is in terms of its index in the conversation.
"""
# If the sync started before the deletion finished, then the sync is stale and we do
# not want to update the conversation.
if self.sync_started_timestamp < self.deletion_scheduled_timestamp:
return

self.controller.session.refresh(self.source)

# Keep a temporary copy of the current conversation so we can delete any
Expand Down Expand Up @@ -3116,7 +3103,7 @@ def add_reply(self, reply: Union[DraftReply, Reply], sender: User, index: int) -
self.scroll.add_widget_to_conversation(index, conversation_item, Qt.AlignRight)
self.current_messages[reply.uuid] = conversation_item

def on_reply_sent(self, source_uuid: str, reply_uuid: str, reply_text: str) -> None:
def on_reply_sent(self, source_uuid: str) -> None:
"""
Add the reply text sent from ReplyBoxWidget to the conversation.
"""
Expand All @@ -3126,7 +3113,6 @@ def on_reply_sent(self, source_uuid: str, reply_uuid: str, reply_text: str) -> N
self.update_conversation(self.source.collection)
except sqlalchemy.exc.InvalidRequestError as e:
logger.debug(e)
self.update_deletion_markers()


class SourceConversationWrapper(QWidget):
Expand Down Expand Up @@ -3268,7 +3254,7 @@ class ReplyBoxWidget(QWidget):
A textbox where a journalist can enter a reply.
"""

reply_sent = pyqtSignal(str, str, str)
reply_sent = pyqtSignal(str)

def __init__(self, source: Source, controller: Controller) -> None:
super().__init__()
Expand Down Expand Up @@ -3365,7 +3351,7 @@ def send_reply(self) -> None:
self.text_edit.setText("")
reply_uuid = str(uuid4())
self.controller.send_reply(self.source.uuid, reply_uuid, reply_text)
self.reply_sent.emit(self.source.uuid, reply_uuid, reply_text)
self.reply_sent.emit(self.source.uuid)

@pyqtSlot(bool)
def _on_authentication_changed(self, authenticated: bool) -> None:
Expand Down
64 changes: 4 additions & 60 deletions tests/gui/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -1940,41 +1940,6 @@ def test_SourceWidget_set_snippet(mocker, session_maker, session, homedir):
sw.set_snippet(source_uuid, "mock_file_uuid", "something new")


def test_SourceWidget_set_snippet_does_not_update_if_sync_is_stale(mocker):
"""
Ensure that the snippet does not get updated when an update is based on a sync that started
before a deletion operation succeeded.
"""
controller = mocker.MagicMock()
source = mocker.MagicMock()
source.journalist_designation = "Testy McTestface"
source.collection = [factory.Message(content="abc123")]
source.server_collection = source.collection
sw = SourceWidget(controller, source, mocker.MagicMock(), mocker.MagicMock())
sw.update()
assert sw.preview.text() == "abc123"

# Make the sync stale
sw.sync_started_timestamp = datetime.now()
sw.deletion_scheduled_timestamp = datetime.now()

# Set up source data so that it appears that the conversation was deleted
source.collection = []
source.interaction_count = 1
source.server_collection = source.collection

# Assert the preview snippet does not get updated because the sync is stale
sw.set_snippet(source.uuid, "mock_file_uuid")
assert sw.preview.text() == "abc123"

# The sync is no longer stale so the preview should show that the are no more source
# conversation items because they've been deleted
sw.deletion_scheduled_timestamp = datetime.now()
sw.sync_started_timestamp = datetime.now()
sw.set_snippet(source.uuid, "mock_file_uuid")
assert sw.preview.text() == "— All files and messages deleted for this source —"


def test_SourceWidget_update_truncate_latest_msg(mocker):
"""
If the latest message in the conversation is longer than 150 characters,
Expand Down Expand Up @@ -4325,22 +4290,6 @@ def test_ConversationView__on_conversation_deletion_successful_does_not_hide_dra
assert not cv.current_messages[draft_reply.uuid].isHidden()


def test_ConversationView_update_conversation_skips_if_sync_is_stale(mocker):
"""
If the sync started before the source was scheduled for deletion, do not update the conversation
"""
cv = ConversationView(factory.Source(), mocker.MagicMock())
cv.update_deletion_markers = mocker.MagicMock()
cv.sync_started_timestamp = datetime.now()
cv.deletion_scheduled_timestamp = datetime.now()
cv.update_conversation([])
cv.update_deletion_markers.assert_not_called()
# Also test that a new message will not get added if the sync is stale
cv.update_conversation([factory.Message()])
assert not cv.current_messages
cv.update_deletion_markers.assert_not_called()


def test_ConversationView_update_conversation_position_follow(mocker, homedir):
"""
Check the signal handler sets the correct value for the scrollbar to be
Expand Down Expand Up @@ -4451,7 +4400,7 @@ def test_ConversationView_on_reply_sent(mocker):
cv.update_conversation = mocker.MagicMock()

assert cv.reply_flag is False
cv.on_reply_sent(source_uuid=source.uuid, reply_uuid="abc123", reply_text="test message")
cv.on_reply_sent(source_uuid=source.uuid)

cv.update_conversation.assert_called_with(source.collection)
assert cv.reply_flag is True
Expand All @@ -4467,7 +4416,7 @@ def test_ConversationView_on_reply_sent_does_not_add_message_intended_for_differ
cv = ConversationView(source, controller)
cv.add_reply = mocker.MagicMock()

cv.on_reply_sent("different_source_id", "mock", "mock")
cv.on_reply_sent("different_source_id")

assert not cv.add_reply.called

Expand All @@ -4484,18 +4433,13 @@ def test_ConversationView_on_reply_sent_with_deleted_source(mocker):
def collection_error():
raise sqlalchemy.exc.InvalidRequestError()

debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
mock_collection = mocker.patch(
"securedrop_client.db.Source.collection", new_callable=PropertyMock
)
ire = sqlalchemy.exc.InvalidRequestError()
mock_collection.side_effect = ire

cv.on_reply_sent(source.uuid, "mock", "mock")

debug_logger.assert_any_call(
f"Could not Update deletion markers in the ConversationView: {ire}"
)
cv.on_reply_sent(source.uuid)


def test_ConversationView_add_reply(mocker, homedir, session, session_maker):
Expand Down Expand Up @@ -4772,7 +4716,7 @@ def test_ReplyBoxWidget_send_reply(mocker):

scw.reply_box.send_reply()

scw.reply_box.reply_sent.emit.assert_called_once_with("abc123", "456xyz", "Alles für Alle")
scw.reply_box.reply_sent.emit.assert_called_once_with("abc123")
scw.reply_box.text_edit.setText.assert_called_once_with("")
controller.send_reply.assert_called_once_with("abc123", "456xyz", "Alles für Alle")

Expand Down

0 comments on commit 3aeef6e

Please sign in to comment.