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.

Sign up for GitHub

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

Failed and pending message to end of conversation. #818

Closed
wants to merge 1 commit into from
Closed

Failed and pending message to end of conversation. #818

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 24 additions & 4 deletions securedrop_client/gui/widgets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1776,7 +1776,8 @@ def __init__(
message_failed_signal.connect(self._on_reply_failure)

# Set styles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this comment seems out of place (i know this wasn't introduced by your code but just noticed it above your new lines of code and was like, this isn't setting styles!)

self._set_reply_state(reply_status)
self.reply_status = reply_status
self._set_reply_state(self.reply_status)

def _set_reply_state(self, status: str) -> None:
if status == 'SUCCEEDED':
Expand All @@ -1795,7 +1796,8 @@ def _on_reply_success(self, source_id: str, message_id: str, content: str) -> No
signal matches the message_id of this widget.
"""
if message_id == self.message_id:
self._set_reply_state('SUCCEEDED')
self.reply_status = "SUCCEEDED"
self._set_reply_state(self.reply_status)

@pyqtSlot(str)
def _on_reply_failure(self, message_id: str) -> None:
Expand All @@ -1804,7 +1806,8 @@ def _on_reply_failure(self, message_id: str) -> None:
signal matches the message_id of this widget.
"""
if message_id == self.message_id:
self._set_reply_state('FAILED')
self.reply_status = "FAILED"
self._set_reply_state(self.reply_status)


class FileWidget(QWidget):
Expand Down Expand Up @@ -2751,11 +2754,23 @@ def update_conversation(self, collection: list) -> None:
# items corresponding to deleted items in the source collection.
current_conversation = self.current_messages.copy()

# To hold widgets that are in a failed or pending state which need to
# be appended to the end of the conversation until their state is
# updated from the server.
pending_items = []

for index, conversation_item in enumerate(collection):
item_widget = current_conversation.get(conversation_item.uuid)
if item_widget:
current_conversation.pop(conversation_item.uuid)
if item_widget.index != index:
if isinstance(item_widget, ReplyWidget) and item_widget.reply_status != "SUCCEEDED":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this if-elif block doesn't cover the case for reply widgets that are successful where item_widget.index == index

# This is a failed or pending reply widget, so simply
# remove it from its current position and add it to the
# pending_items list, to be added to the end of the
# conversation once this loop is finished.
self.conversation_layout.removeWidget(item_widget)
pending_items.append(item_widget)
elif item_widget.index != index:
# The existing widget is out of order.
# Remove / re-add it and update index details.
self.conversation_layout.removeWidget(item_widget)
Expand Down Expand Up @@ -2785,6 +2800,11 @@ def update_conversation(self, collection: list) -> None:
# source collection and should be removed.
for item_widget in current_conversation.values():
self.conversation_layout.removeWidget(item_widget)
# Ensure all widgets currently in a failed or pending state are added
# to the end of the conversation layout.
for item_widget in pending_items:
self.conversation_layout.addWidget(item_widget,
alignment=Qt.AlignRight)

def add_file(self, file: File, index):
"""
Expand Down