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

Jump to bottom

no longer sync after sending a reply #722

Merged
merged 1 commit into from
Jan 30, 2020
Merged

no longer sync after sending a reply #722

merged 1 commit into from
Jan 30, 2020

Conversation

sssoleileraaa
Copy link
Contributor

Description

Resolves #660

Test Plan

  1. Send a reply
  2. See it updated in the gui

@sssoleileraaa sssoleileraaa added this to Ready for Review in SecureDrop Team Board Jan 22, 2020
redshiftzero
redshiftzero reviewed Jan 23, 2020
View reviewed changes
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

so if replies need to be reordered, this won't happen until the next successful regular sync (until #473 is implemented) with this change right? i think we need to emit some signal to let the UI (conversation view) know to update after the reply sends since this reordering function is running in the reply send job here

@redshiftzero redshiftzero moved this from Ready for Review to Under Review in SecureDrop Team Board Jan 23, 2020
@sssoleileraaa
Copy link
Contributor Author

so if replies need to be reordered, this won't happen until the next successful regular sync (until #473 is implemented) with this change right? i think we need to emit some signal to let the UI (conversation view) know to update after the reply sends since this reordering function is running in the reply send job here

i think i see what you mean. i think we should wait to merge this until #688 is merged, which adds an index to the messages and replies in the conversation view in order to update/reorder each time there is a new message or reply. With this change and continuous syncs, i think this pr is ready to merge right?

@sssoleileraaa
Copy link
Contributor Author

added blocked label until #688 is merged

@sssoleileraaa sssoleileraaa mentioned this pull request Jan 29, 2020
Merged
kushaldas
kushaldas approved these changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Works as expected, approved.

@kushaldas kushaldas merged commit 1436b6e into master Jan 30, 2020
SecureDrop Team Board automation moved this from Under Review to Done Jan 30, 2020
@kushaldas kushaldas deleted the no-sync-after-sending-reply branch January 30, 2020 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@redshiftzero redshiftzero redshiftzero left review comments

@kushaldas kushaldas kushaldas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
SecureDrop Team Board
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No longer sync on reply success
3 participants
@sssoleileraaa @kushaldas @redshiftzero