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

Star/unstar sources using general queue #412

Closed
redshiftzero opened this issue Jun 10, 2019 · 2 comments · Fixed by #445
Closed

Star/unstar sources using general queue #412

redshiftzero opened this issue Jun 10, 2019 · 2 comments · Fixed by #445

Comments

@redshiftzero
Copy link
Contributor

Similar to #389: we should add StarSourceJob and UnstarSourceJob objects which are ran in the general queue
@redshiftzero redshiftzero mentioned this issue Jun 10, 2019
Closed
@redshiftzero redshiftzero added this to Nominated for next sprint (6/12-6/26) in SecureDrop Team Board Jun 11, 2019
@eloquence eloquence moved this from Current sprint nominations (in progress) to Current Sprint - 6/12-6/26 in SecureDrop Team Board Jun 12, 2019
@kushaldas
Copy link
Contributor

Currently we do a self.sync_api() call if the update star is successful. Should we keep doing that? Also, in my mind just having a single UpdateStarJob is simpler for this operation. Please let me know what do you think. I am also trying to figure out the tests.

@redshiftzero
Copy link
Contributor Author

For the purposes of this ticket which is just scoped to moving these starring operations to the queue, I think continuing to do the sync after success is best (while #69 also exists regarding changing that behavior, in the interest of keeping changes small, I think doing them as separate tickets is best).

If your idea is a single UpdateStarJob that we pass an arg specifying whether the job should star or unstar, that sounds like a reasonable approach to me.

@kushaldas kushaldas moved this from Current Sprint - 6/26-7/10 to In Development in SecureDrop Team Board Jun 27, 2019
kushaldas added a commit that referenced this issue Jul 1, 2019
@kushaldas
Fixes #412 moves update_star calls under queue
3fbc502 
Now we are using the same general queue for update_star
method on sources.
@redshiftzero redshiftzero moved this from In Development to Ready for review in SecureDrop Team Board Jul 1, 2019
@eloquence eloquence removed this from Ready for review in SecureDrop Team Board Jul 1, 2019
@kushaldas kushaldas mentioned this issue Jul 1, 2019
Merged
sssoleileraaa pushed a commit that referenced this issue Jul 1, 2019
@kushaldas @sssoleileraaa
Fixes #412 moves update_star calls under queue
2fe393a 
Now we are using the same general queue for update_star
method on sources.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Moves update_star calls under queue freedomofpress/securedrop-client
2 participants
@kushaldas @redshiftzero