-
-
Notifications
You must be signed in to change notification settings - Fork 35
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split up question worker to send to an individual user rather than to all followers #1442
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1442 +/- ##
==========================================
+ Coverage 95.41% 95.70% +0.29%
==========================================
Files 174 174
Lines 2702 2699 -3
==========================================
+ Hits 2578 2583 +5
+ Misses 124 116 -8 ☔ View full report in Codecov by Sentry. |
1500880
to
b193ad2
Compare
app/workers/question_worker.rb
Outdated
user = User.find(user_id) | ||
question = Question.find(question_id) | ||
def perform(follower_id, question_id) | ||
follower = User.includes(:web_push_subscriptions, :mute_rules, :muted_users).find(follower_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one minor concern i have is that during deployment there may be still some QuestionWorker
jobs that expect the first parameter to be the user id who asked the question, which will eventually end up as them asking the question themselves.
perhaps we can keep the old QuestionWorker
as a fallback, and rename this worker to something else (QuestionJob
maybe -- Sidekiq nowadays prefers to use *Job
instead of *Worker
https://github.com/sidekiq/sidekiq/blob/main/lib/sidekiq/worker_compatibility_alias.rb)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good overall though
Co-authored-by: Georg Gadinger <[email protected]>
Closes #810