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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add SendReplyJob and use it for sending replies to the general queue #401

Merged
merged 3 commits into from
Jun 6, 2019

Conversation

redshiftzero
Copy link
Contributor

Description

Fixes #390

Test Plan

@eloquence eloquence added this to Ready for review in SecureDrop Team Board Jun 5, 2019
Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

When testing this, I noticed that the ReplyWidget for a new reply now disappears after sending it.

STR: 1. send reply, 2. click on a different source and back to the original source.

I can see that replies are being successfully sent, but it's almost like the GUI no longer receives the reply_succeeded signal. I see it in the code being sent but something's up.

@redshiftzero
Copy link
Contributor Author

dang, good catch: it was a familiar culprit - b09774f#diff-16a175b4662541f3889b432ac9e7e0e8R1393

Copy link
Contributor

@sssoleileraaa sssoleileraaa left a comment

Choose a reason for hiding this comment

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

Now that you refresh the source db object you should no longer need to refresh each conversation_item. I think you can remove this line:

self.controller.session.refresh(conversation_item)

@eloquence eloquence moved this from Ready for review to In Development in SecureDrop Team Board Jun 6, 2019
two changes:
1. We should mark replies that we send as downloaded/decrypted
so that we don't keep redownloading them.
2. We should refresh the source so that we get all items
from the collection when we redraw it.
@redshiftzero
Copy link
Contributor Author

ah, good point @creviera - addressed and squashed into last commit

@redshiftzero redshiftzero moved this from In Development to Ready for review in SecureDrop Team Board Jun 6, 2019
@sssoleileraaa sssoleileraaa merged commit 9ef8177 into master Jun 6, 2019
SecureDrop Team Board automation moved this from Ready for review to Done Jun 6, 2019
@sssoleileraaa sssoleileraaa deleted the add-general-reply-queue branch June 6, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Send replies using general queue
2 participants