TypeError that crashes the app when sending reply

[sssoleileraaa]

Description

On master branch, send a reply and you'll get a TypeError because add_reply takes one argument now, a Reply object, instead of a message uuid and a message string.

TypeError happening here -> securedrop-client/securedrop_client/gui/widgets.py", line 900, in send_reply self.conversation.add_reply(msg_uuid, message)

More info

We changed the add_reply signature here: #277. The solution should be to update send_reply to first send a reply, which should create a Reply object, send, and then add_reply to the ConversationView.

[sssoleileraaa]

Potential fix (fastest) is to revert the change from #277 so that add_reply(self, reply: Reply) -> add_reply(self, message_id: str, reply: str, files=None), but I think if I understand this right, this would mean that we would add ReplyWidgets to the conversation view before we create the Reply object in the database. If something goes wrong when creating the Reply object then we might get into a state of displaying replies that failed to be created in the db (perhaps sanitation fails or something).

Another solution would be to wait until the database object is created before displaying it in the conversation view. This would also allow us to keep the way we add replies the same as the way we add messages, see add_message(self, message: Message).

[heartsucker]

Because of the restrictions on the database (not null columns) we will not be able to reliably create the DB object. That can only happen after the response from the server. The widgets need to be created and allow to exist outside the DB. If we want to make it possible to store pending replies in the DB, we would need to alter the schema. This would allow someone to close the client and see pending replies when they open it again (before a sync occurs).

[sssoleileraaa]

What if instead of displaying pending replies we just wait to update the UI after all reply operations finish, including any user input checks, handling of server errors, responses, etc. To me, it seems better to wait until we create the Reply object before creating a widget with the user input that hasn't been validated yet.

[sssoleileraaa]

Per chat with @heartsucker another potential issue with what we're doing (generating uuids client-side and displaying reply content in th ui before it has been stored in a database) is that the content could be replaced or removed after a sync and this might be confusing to the user. Replies could also be lost because of refreshing the conversation in the gui.

[sssoleileraaa]

We can capture further discussion in the issue @heartsucker opened here: #294

And since this issue crashes the app, it makes sense to go with the fastest fix which is to just undo the change to the method signature. Thanks @heartsucker for pointing out that how we display and store and update replies is a larger change that needs to be scoped more!